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

Dropdown default values not working with custom JSX toolbar #175

Open
1 task done
kamas06 opened this issue Mar 23, 2017 · 10 comments
Open
1 task done

Dropdown default values not working with custom JSX toolbar #175

kamas06 opened this issue Mar 23, 2017 · 10 comments

Comments

@kamas06
Copy link

kamas06 commented Mar 23, 2017

Expected:
When you select a text with "normal" font size, the dropdown should reflect that selection.

Actual:
Dropdown shows "large" or "small" but never goes back to "normal". It looks to be only happening with manually created HTML toolbar and only with React. Tired modding official Quill codepen to manual toolbar and it works there with plain HTML.

image
image

I assume this is because of the way React handles select tags - https://facebook.github.io/react/docs/forms.html#the-select-tag

I have tried using defaultValue on select etc. but no luck.

http://codepen.io/kamas06/pen/EWLpKm

Quill version:

  • 1.0.0-beta-3
@alexkrolick
Copy link
Collaborator

alexkrolick commented Mar 23, 2017

I'm seeing this for your Codepen on macOS Sierra, Google Chrome. What are you using?

EDIT: Ok I see there is an issue here. Dropping a link to the relevant Quill modules for investigation: https://github.com/quilljs/quill/blob/develop/modules/toolbar.js

@alexkrolick
Copy link
Collaborator

Here's a side-by-side of the different toolbar options: https://codepen.io/alexkrolick/pen/BWVNYx?editors=0010

The JSX custom toolbar option is definitely doing something a little differently than the uncontrolled way - note extra bottom border between the elements.

@kamas06
Copy link
Author

kamas06 commented Mar 24, 2017

@alexkrolick Nice side-by-side, shows the problem really well. I got hit by #147 too at some point but moved away to JSX toolbar which sorted it. Had to move anyway to get more control over it as I am placing some custom markers/icons inside it.

@alexkrolick
Copy link
Collaborator

Note that is possible to have some control over the appearance of the icons Quill creates with the toolbar prop. Use CSS to target ql-<format>: https://codepen.io/alexkrolick/pen/qryGQd

Will continue looking into the issue.

@alexkrolick alexkrolick changed the title Font size dropdown not defaulting to "normal" with manual HTML toolbar Dropdown default values not working with custom JSX toolbar Jul 7, 2017
@alexkrolick
Copy link
Collaborator

alexkrolick commented Jul 9, 2017

Some additional debugging notes:

This line requires one of the <option> tags to have the selected attribute: https://github.com/quilljs/quill/blob/develop/ui/picker.js#L46

If it is missing, the default item will not get the ql-selected class here: https://github.com/quilljs/quill/blob/develop/ui/picker.js#L73

And finally, the defaultItem will not exist here: https://github.com/quilljs/quill/blob/develop/ui/icon-picker.js#L11-L18

screen shot 2017-07-08 at 11 19 43 pm

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

Note that setting THEME to false at the top actually works better when it comes to dropdowns than the themes that extend base (ie Snow and Bubble) because it only uses HTML selects, rather than the "fake" select from icon-picker.js

So... it seems the main thing to do is figure out a way to 1) get React to stop swallowing the selected attribute on options in selects or 2) add an alternate identifier for Quill to use to determine the selected state, that still emits the right change events

NOTE: React doesn't allow setting selected manually for each tag (instead it is set by the select's value): https://stackoverflow.com/questions/21733847/react-jsx-selecting-selected-on-selected-select-option#comment33279210_21736116

@jxmai
Copy link

jxmai commented Oct 12, 2017

So... it seems the main thing to do is figure out a way to 1) get React to stop swallowing the selected attribute on options in selects or 2) add an alternate identifier for Quill to use to determine the selected state, that still emits the right change events

@alexkrolick I am coming from another project (https://www.primefaces.org/) which is heavily used by enterprise since your change seems broke our code (primefaces/primefaces#2802), and I have raised another ticket in quill slab/quill#1762

@alexkrolick
Copy link
Collaborator

alexkrolick commented Feb 26, 2018

💯 I think this can be fixed by adding some hints for React's event system:

<select defaultValue={""} onChange={e => e.persist()}>

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

@coxandrew
Copy link

Hi @alexkrolick - I'm trying to wrap Quill in a React component for my own custom implementation and I cannot cannot figure out why react-quill is working with a defaultValue, but mine is not (the onChange doesn't appear to be necessary in your example above).

Here's a streamlined example demonstrating the bug: https://codepen.io/coxandrew/pen/xjxXmy?editors=1011

I've tried with and without jsx toolbar components and about every combination of selected or value={""} within the option tag and cannot get it to work with my simple example.

I have another CodePen that uses a forked version of Quill with this attempted fix, but that doesn't work with my React version either: https://codepen.io/coxandrew/pen/YLzvwJ?editors=0011

Any ideas on what might be different about react-quill's implementation?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Apr 18, 2018

@coxandrew seems to work when I upgrade your Pen to React 16: https://codepen.io/alexkrolick/pen/KRKYLw?editors=1011

@coxandrew
Copy link

@alexkrolick Aha! Thanks for the quick response! We may not be able to upgrade to React 16 immediately, but at least we have a path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants