-
Notifications
You must be signed in to change notification settings - Fork 8
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(Input): handle textual changes to date picker #1672
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deanterm
requested review from
atlwendy,
benjamincharity and
shani-terminus
as code owners
August 29, 2019 15:03
deanterm
force-pushed
the
datepicker-input-changes
branch
from
August 29, 2019 16:42
d91b145
to
fab7cea
Compare
Codecov Report
@@ Coverage Diff @@
## release #1672 +/- ##
===========================================
+ Coverage 95.53% 95.54% +<.01%
===========================================
Files 127 127
Lines 4843 4850 +7
Branches 842 843 +1
===========================================
+ Hits 4627 4634 +7
Misses 211 211
Partials 5 5
Continue to review full report at Codecov.
|
benjamincharity
suggested changes
Aug 30, 2019
author dean <dean.jones@terminus.com> 1567090130 -0400 committer dean <dean.jones@terminus.com> 1567169046 -0400 parent fb7c386 author dean <dean.jones@terminus.com> 1567090130 -0400 committer dean <dean.jones@terminus.com> 1567169006 -0400 fix(Input): handle textual changes to date picker The datepicker functionality has an issue when a user changes the text value of the input after a date has been selected. Example of this: Input value is set to: 08-22-2019 User decides they want change the date to: 08-23-2019 So, they put the cursor after the 22 and change the last 2 to a 3. During this editing, if we update the value that the datepicker looks at, it gets confused and the value ends up looking like: 8-232-019 This value cannot be parsed by the datepicker and we end up with a null Date() value. However, the value inside the input looks correct. The emitted value from the input, however, ends up being null. We tried a few workarounds here. 1. Tried setting this.value = new Date(textualValue) This was a problem because the moment you removed the 2 in the example above, the value ended up looking like: 08-22-019. The 2 from 2019 moved into the day section of the mask and that value is actually a valid date! The date turns out to parse into 08-22-2019, which is what we already had. This, in turn, would fire off an event on the datepicker which would re-update its internal stuff and kick the cursor to the end of the line. Making it literally impossible to change the date manually unless you deleted every character and reentered the whole date by hand. 2. Tried creating a hidden input element that the datepicker was tied to and then manually update the shown input based on changes. This caused a lot of issues where we had to manually update the mask on every event and make sure we updated both values on the hidden and shown input. This became messy quickly. 3. The solution that's being committed here. We don't fire off a selected event on input changes. We instead just update the masked value and store the masked value in a separate store. Then, on blur events, the change event will look to see if the date is null (due to the description of the original issue above) and if the textual value is not null. If so, the textual value wins and we create a new date object based on that info. Otherwise, the datepicker value wins. This prevents the input text changing while the user is typing and throws out an event on blur with the updated value.
deanterm
force-pushed
the
datepicker-input-changes
branch
from
August 30, 2019 12:45
b2e7b2b
to
8df7b34
Compare
The datepicker functionality has an issue when a user changes the text value of the input after a date has been selected. Example of this: Input value is set to: 08-22-2019 User decides they want change the date to: 08-23-2019 So, they put the cursor after the 22 and change the last 2 to a 3. During this editing, if we update the value that the datepicker looks at, it gets confused and the value ends up looking like: 8-232-019 This value cannot be parsed by the datepicker and we end up with a null Date() value. However, the value inside the input looks correct. The emitted value from the input, however, ends up being null. We tried a few workarounds here. Tried setting this.value = new Date(textualValue) This was a problem because the moment you removed the 2 in the example above, the value ended up looking like: 08-22-019. The 2 from 2019 moved into the day section of the mask and that value is actually a valid date! The date turns out to parse into 08-22-2019, which is what we already had. This, in turn, would fire off an event on the datepicker which would re-update its internal stuff and kick the cursor to the end of the line. Making it literally impossible to change the date manually unless you deleted every character and reentered the whole date by hand. Tried creating a hidden input element that the datepicker was tied to and then manually update the shown input based on changes. This caused a lot of issues where we had to manually update the mask on every event and make sure we updated both values on the hidden and shown input. This became messy quickly. The solution that's being committed here. We don't fire off a selected event on input changes. We instead just update the masked value and store the masked value in a separate store. Then, on blur events, the change event will look to see if the date is null (due to the description of the original issue above) and if the textual value is not null. If so, the textual value wins and we create a new date object based on that info. Otherwise, the datepicker value wins. This prevents the input text changing while the user is typing and throws out an event on blur with the updated value.
deanterm
force-pushed
the
datepicker-input-changes
branch
from
August 30, 2019 12:57
51528f5
to
ccc60a7
Compare
benjamincharity
approved these changes
Aug 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The datepicker functionality has an issue when a user changes the text
value of the input after a date has been selected. Example of this:
Input value is set to:
08-22-2019
User decides they want change the date to:
08-23-2019
So, they put the cursor after the 22 and change the last 2 to a 3.
During this editing, if we update the value that the datepicker looks
at, it gets confused and the value ends up looking like:
8-232-019
This value cannot be parsed by the datepicker and we end up with a null
Date() value. However, the value inside the input looks correct. The
emitted value from the input, however, ends up being null.
We tried a few workarounds here.
This was a problem because the moment you removed the 2 in the
example above, the value ended up looking like: 08-22-019. The
2 from 2019 moved into the day section of the mask and that value
is actually a valid date! The date turns out to parse into
08-22-2019, which is what we already had. This, in turn, would
fire off an event on the datepicker which would re-update its
internal stuff and kick the cursor to the end of the line.
Making it literally impossible to change the date manually
unless you deleted every character and reentered the whole
date by hand.
and then manually update the shown input based on changes. This
caused a lot of issues where we had to manually update the mask on
every event and make sure we updated both values on the hidden and
shown input. This became messy quickly.
selected event on input changes. We instead just update the masked
value and store the masked value in a separate store. Then, on blur
events, the change event will look to see if the date is null (due to
the description of the original issue above) and if the textual value
is not null. If so, the textual value wins and we create a new date
object based on that info. Otherwise, the datepicker value wins.
This prevents the input text changing while the user is typing and
throws out an event on blur with the updated value.