Repository navigation
DRYD-1294: Use Native Date Input - #29
Conversation
8425c46 to
448a24c
Compare
|
Regarding which version to use in the CHANGELOG, I set it to |
| data-name={name} | ||
| className={className} | ||
| value={normalizedValue} | ||
| onChange={handleChange} |
There was a problem hiding this comment.
The native date input (at least in Chrome) fires a change event per keystroke inside a segment. Typing the year 2025 over an existing 2025-12-01 produces "0002-12-01", "0020-12-01", "0202-12-01" and then "2025-12-01". Each one is committed. Clearing a segment commits "" as well. (As a side effect, the round trip through normalizeDateString shifts those (before 1000 year) years back a day, so the field visibly jumps around while typing.)
Could we use the changeable and committable enhancers instead, as the other text inputs do, so the value being edited is held locally and only committed on blur or enter? A BaseDateInput here can hold the native <input type="date">. This component can render committable(changeable(BaseDateInput)). handleChange and the getPath import can then go away.
| parentPath: pathPropType, | ||
| subpath: pathPropType, | ||
| value: PropTypes.string, | ||
| locale: PropTypes.string, |
There was a problem hiding this comment.
| locale: PropTypes.string, |
| parentPath: undefined, | ||
| subpath: undefined, | ||
| value: undefined, | ||
| locale: 'en-US', |
There was a problem hiding this comment.
| locale: 'en-US', |
| onCommit, | ||
| asText, | ||
| embedded, | ||
| locale, |
| } = props; | ||
|
|
||
| const className = embedded ? styles.embedded : styles.normal; | ||
| const normalizedValue = normalizeDateString(value, locale) || ''; |
There was a problem hiding this comment.
| const normalizedValue = normalizeDateString(value, locale) || ''; |
| <LineInput | ||
| id={id} | ||
| asText={asText} | ||
| value={normalizedValue} |
There was a problem hiding this comment.
| value={normalizedValue} | |
| value={value} |
| type="date" | ||
| data-name={name} | ||
| className={className} | ||
| value={normalizedValue} |
There was a problem hiding this comment.
| value={normalizedValue} | |
| value={value} |
| } = props; | ||
|
|
||
| const className = embedded ? styles.embedded : styles.normal; | ||
| const normalizedValue = normalizeDateString(value, locale) || ''; |
There was a problem hiding this comment.
I still can see the jump around while typing. The root cause is calling the normalizer here. This results to normalizing values typed on every keystroke. Let's follow the pattern used before switching to native. Having a wrapper that calls the normalizer once. I had to apply the fix locally to test it, so I also add the relevant suggested changes.
There was a problem hiding this comment.
I wonder if I need to change my locale to see the jumping behavior (which makes sense with normalize being called so often). Let me see if I can do that then I'll pull in your changes.
There was a problem hiding this comment.
Oh nevermind, I see it in chrome as well. I must have only tested onCommit being called from the calendar input the other day.
This wraps the BaseDateInput to allow normalization to happen only when a date is committed and not while being edited. Co-authored-by: Spiros Dimopulos <spiros@wantedpixel.com>
What does this do?
Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1294
The current implementation of the DateInput uses a Dropdown which opens the react-calendar widget, and was flagged in the accessibility audit. This switches to using the browser's native date input as it is generally made to be accessible.
A few notes on some of the changes:
normalizeDateStringas it calls bothnormalizeISO8601DateStringandnormalizeNaturalLanguageDateString. I don't expect we'll have anything with natural language, but it seemed harmless.How should this be tested? Do these changes have associated tests?
"cspace-input": "file:../cspace-input.js",
annotationDatefor CollectionObjectsDependencies for merging? Releasing to production?
The CHANGELOG still needs to be updated. 2.4.0? 2.3.1? I can do it here once we decide.
The tooltip was removed so there's likely some dead code paths from cspace-ui.js now. I don't believe any of the other input components use a tooltip, probably worth checking.
Has the application documentation been updated for these changes?
If there's documentation about the date input it will need to be. I'll search.
Did someone actually run this code to verify it works?
@mikejritter tested locally