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

Reset of select ignores defaultValue #11010

Closed
ZeHiro opened this issue Oct 1, 2017 · 9 comments · Fixed by #11057
Closed

Reset of select ignores defaultValue #11010

ZeHiro opened this issue Oct 1, 2017 · 9 comments · Fixed by #11057

Comments

@ZeHiro
Copy link

ZeHiro commented Oct 1, 2017

Do you want to request a feature or report a bug?
I report a bug.
What is the current behavior?
When resetting a form which contains a select with a defaultValue, the selected option becomes the first option element in the list, not the one with value=defaultValue.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/ebsrpraL/).
I have put a piece of code here :
https://codepen.io/zehiro/pen/YrxZWw?editors=1111#0

Click on the reset button, which just does a form.reset() on the form.
What is the expected behavior?
I expect the select to select the option with value=defaultValue (like applying a form.reset() on a <input/>
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React-DOM 15.6.2.
It seems to be browser independent (tried with Chromium 61 and firefox 55 for Fedora).
I didn't try this on prior version of React.

@gaearon
Copy link
Collaborator

gaearon commented Oct 1, 2017

@aweary

@aweary aweary self-assigned this Oct 1, 2017
@aweary
Copy link
Contributor

aweary commented Oct 2, 2017

@ZeHiro I'm seeing that if I click the button to reset the form without changing the value at all, it sets the value of the select element to "First". If I first change the select's value to "First" and then click reset, nothing happens (what you're reporting). Can you verify that you're seeing the same behavior? Thanks!

@ZeHiro
Copy link
Author

ZeHiro commented Oct 2, 2017

@aweary I have exaclty thie behaviour. I would expect for both situations that after clicking the reset button, the "Default" option is selected.

@aweary
Copy link
Contributor

aweary commented Oct 2, 2017

Thanks for confirming @ZeHiro!

According to the HTML spec, select elements should do the following when a reset event happens:

The reset algorithm for select elements is to go through all the option elements in the element's list of options, set their selectedness to true if the option element has a selected attribute, and false otherwise, set their dirtiness to false, and then have the option elements ask for a reset.

So it looks like it depends on the selected attribute being preset. We set the selected property on the option elements, but we never set the attribute, so we hit this case for the option reset algorithm:

If the select element's display size is 1, and no option elements in the select element's list of options have their selectedness set to true

Set the selectedness of the first option element in the list of options in tree order that is not disabled, if any, to true.

I think what we need to do is utilize the defaultSelected attribute on the option element(s) and keep it in sync with defaultValue. The spec doesn't really say how the defaultSelected attribute works with the reset algorithm, but some quick testing shows that it should help us resolve this issue.

@ZeHiro
Copy link
Author

ZeHiro commented Oct 2, 2017

Thanks for analysing more deeply the subject. I cannot help you to chose the most appropriate solution though :-)

@aweary
Copy link
Contributor

aweary commented Oct 2, 2017

No worries, I'm mostly just documenting it for future reference and for the rest of the team 😉 Thanks again for the report!

@ZeHiro
Copy link
Author

ZeHiro commented Oct 2, 2017

Thanks for adressing it.

@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

@sebmarkbage I remember you had opinions about reset() in the past, do they affect this issue?

@aweary aweary removed their assignment Oct 10, 2017
@gaearon
Copy link
Collaborator

gaearon commented Nov 3, 2017

React 16.1.0-beta has been released. Please update react, react-dom, and react-test-renderer (if you use it) to this version and let us know if it solved the issue! We’d appreciate if you could test before Monday when we plan to get 16.1.0 out.

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

Successfully merging a pull request may close this issue.

3 participants