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

use DOM API instead of attribute for "selected" option in dropdowns #1576

Merged
merged 1 commit into from
Jul 10, 2017

Conversation

alexkrolick
Copy link
Contributor

@alexkrolick alexkrolick commented Jul 9, 2017

https://developer.mozilla.org/en-US/docs/Web/API/HTMLOptionElement

The Option element exposes a "selected" getter that works even when the "selected" attribute is not set, such as when the element is created by React.

Another way to do this is to provide an alternate data attribute to identify the default.

Codepen link demonstrating what happens when an initial "selected" attribute is not present on any of custom option tags for ql-align: https://codepen.io/alexkrolick/pen/qjMrEN?editors=0010

Fixes zenoamaro/react-quill#226.

https://developer.mozilla.org/en-US/docs/Web/API/HTMLOptionElement

The Option element exposes a "selected" getter that works even when the "selected" attribute is not set, such as when the element is created by React or manipulated. Fixes react-quill#175.
@alexkrolick alexkrolick changed the title use DOM API instead of attribute for "selected" use DOM API instead of attribute for "selected" option in dropdowns Jul 9, 2017
@alexkrolick
Copy link
Contributor Author

alexkrolick commented Jul 9, 2017

Demo of the change:

https://codepen.io/alexkrolick/pen/awRxWz?editors=0010

Note that the highlighted values in the toolbar don't reset to default with the Snow theme in React-Quill, which is another bug (zenoamaro/react-quill#175).

@jhchen
Copy link
Member

jhchen commented Jul 10, 2017

Thanks!

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