Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates test message and adds default case #728

Merged
merged 3 commits into from
Apr 12, 2017
Merged

Updates test message and adds default case #728

merged 3 commits into from
Apr 12, 2017

Conversation

jbalsas
Copy link
Contributor

@jbalsas jbalsas commented Apr 11, 2017

Hey @ipeychev, I've rebased and fixed the conflicts. Turns out the test file was conflicting with some previously added ones... I think it's the first time we run into problems by writing too many tests 😂

@ipeychev
Copy link
Contributor

Just started reviewing :)

:octocat: Sent from GH.

@ipeychev
Copy link
Contributor

Umm, let me see if I understood correctly the issue:
If the user passes explicitly 'ae_dragresize in extraPlugins property, then the drag in IE11 will stop working. Is that correct?

var content = new AlloyEditor.editable('myContent', {extraPlugins: 'ae_dragresize'})

And why do we do this, user's preferences always have precedence?

@jbalsas
Copy link
Contributor Author

jbalsas commented Apr 12, 2017

Hey Iliyan!

Yeah, on top of user preferences, I think we always want our swap to prevail. We added that so users would only need to add ae_dragresize (always) and we'd take care of using the right plugin depending on the browser.

Also, in some cases like server-side generated config, they won't even be able to decide which one should they use. That's why we always need to do the replace at the end.

And yeah, what you say is exactly what happens... which we meant to fix originally by replacing the default plugins ourselves.

@ipeychev ipeychev merged commit f72f6e1 into master Apr 12, 2017
@ipeychev
Copy link
Contributor

Thank you, pull request merged! See changes here.

:octocat: Sent from GH.

@ipeychev
Copy link
Contributor

Okay... then we can merge this one for now. Thanks!

@jbalsas jbalsas deleted the pr-722 branch August 28, 2017 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants