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

Fix [required] fields not working in Microsoft Edge with Selectize.js #1222

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Fix [required] fields not working in Microsoft Edge with Selectize.js #1222

merged 1 commit into from
Sep 26, 2017

Conversation

TomOne
Copy link
Contributor

@TomOne TomOne commented Sep 13, 2017

This is achieved by adding a Selectize plugin that overrides the bugged refreshValidityState method in the Selectize library. This commit should be reverted if selectize/selectize.js/pull/1320 is ever merged and a new version of Selectize gets released.

MS Edge is sometimes the only browser people can use in corporate environments. Even if supporting MS Edge is sometimes cumbersome it would be great if the Grav Admin Plugin would work with Edge without problems. This PR is a step towards it.

I’m aware of the fact that overriding things in external libraries is generally a bad idea. Something could break when the library is updated. But I think this is a special case because Selectize has been looking for new maintainers since 2015 (see selectize/selectize.js/issues/752) and receives only minimal attention from the original maintainer since then. Therefore it’s unlikely that this issue is fixed in Selectize itself (selectize/selectize.js/pull/1320), leaving the Admin plugin forever with this issue.

I tested the changes from this PR in Chrome and in MS Edge without problems.

Fixes #1061

This is achieved by adding a Selectize plugin that overrides the bugged refreshValidityState method in the Selectize library. This commit should be reverted if selectize/selectize.js#1320 is ever merged and a new version of Selectize gets released.

Fixes #1061
Copy link
Member

@rhukster rhukster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me, but as this is JS stuff, I would like @w00fz to review and ensure it's ok.

Copy link
Member

@w00fz w00fz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, requires a recompilation of the JS once this is merged. Please ping me in this PR when it happens.

@rhukster rhukster merged commit bca7f7b into getgrav:develop Sep 26, 2017
@rhukster
Copy link
Member

merged @w00fz

@TomOne TomOne deleted the selectize-required-fix branch September 26, 2017 20:13
w00fz added a commit that referenced this pull request Sep 27, 2017
@w00fz
Copy link
Member

w00fz commented Sep 27, 2017

Cheers, recompiled.

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.

Adding new page in Edge
3 participants