Skip to content

Fix <input> with type date/time/etc. issue on mobile browsers #7397

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.

Sign up for GitHub

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

Merged
keyz merged 2 commits into from
Aug 2, 2016
Merged

Fix <input> with type date/time/etc. issue on mobile browsers #7397

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cf33a5
fixed <input> with type date/time/etc. issue on iOS safari
Aug 1, 2016
c6f8c32
supported color type and improved comments
Aug 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
supported color type and improved comments
  • Loading branch information
Keyan Zhang committed Aug 2, 2016
commit c6f8c3226b556fe04eceb1884bcefa24df70b9d9
4 changes: 3 additions & 1 deletion src/renderers/dom/client/wrappers/ReactDOMInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,15 @@ var ReactDOMInput = {
case 'submit':
case 'reset':
break;
case 'color':
case 'date':
Copy link
Collaborator

@sophiebits sophiebits Aug 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add 'color'? Broken in Android 4.4.4 browser.

case 'datetime':
case 'datetime-local':
case 'month':
case 'time':
case 'week':
// this fixes the no-show issue on iOS Safari: https://github.com/facebook/react/issues/7233
// This fixes the no-show issue on iOS Safari and Android Chrome:
// https://github.com/facebook/react/issues/7233
node.value = '';
Copy link

@kellycampbell kellycampbell Aug 3, 2016

Choose a reason for hiding this comment

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

What's the reason for this assignment since the next line reassigns it to defaultValue? Also, line 249 looks like a no-op as well.

Copy link
Collaborator

@sophiebits sophiebits Aug 3, 2016

Choose a reason for hiding this comment

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

It fixes a bug where iOS Safari and Android Chrome do not show the value. The one on line 249 detaches .value from .defaultValue (by default, changing .defaultValue affects .value but they work independently as soon as .value is ever set, including to its existing value).

Copy link
Contributor

@nhunzaker nhunzaker Aug 3, 2016

Choose a reason for hiding this comment

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

Hmm. Neat. I need to check to see if this influences my Chrome backspace issue PR (#7359)

Copy link
Contributor

@nhunzaker nhunzaker Aug 3, 2016

Choose a reason for hiding this comment

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

Looks like no. At least from my test case:

https://gist.github.com/nhunzaker/d31817ffd48279b4bf1f7513dd84f400

I don't want to make you all linger too much on this. I wish I understood how the detachment works better. Is it possible that this case is display-only?

Copy link
Collaborator

@sophiebits sophiebits Aug 3, 2016

Choose a reason for hiding this comment

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

Sorry, do you have a specific question? The correct browser behavior is:

.value always reflects the current displayed value of a field and what would be used if the form is submitted.

.defaultValue is always the same as the HTML value attribute (and get/setAttribute 'value'). When a form "reset" button is clicked, .value reverts to .defaultValue.

Initially, they are linked so that changing the value attribute or setting .defaultValue also changes value. Once value changes (either due to .value = or the user interacting), they function independently.

Copy link
Contributor

@nhunzaker nhunzaker Aug 3, 2016

Choose a reason for hiding this comment

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

Yes, though you've just now answered it. I am sorry to have forced a quick lecture on the DOM :).

I wanted to figure out if the delinking here eliminated the need for the fix I added to #7359 where defaultValue is only reassigned if it is different.

From a quick test outside of React, I thought this PR would make it unnecessary. However it isn't, and I became confused. It looks like, no matter what, value and checked get assigned in ReactDOMInput.getHostProps, and subsequently get sent out as DOM mutations. Even for uncontrolled inputs.

So that's what I'm working through now. No need to continue talking about it here, but I'd like to avoid being able to do the comparison check and wanted to better understand the solution in this PR.

node.value = node.defaultValue;
break;
Expand Down