-
Notifications
You must be signed in to change notification settings - Fork 127
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
New DateInput component #2645
New DateInput component #2645
Conversation
🦋 Changeset detectedLatest commit: 804f19f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2645 +/- ##
==========================================
+ Coverage 87.49% 87.82% +0.32%
==========================================
Files 213 218 +5
Lines 11926 12615 +689
Branches 1556 1695 +139
==========================================
+ Hits 10435 11079 +644
- Misses 1441 1486 +45
Partials 50 50
|
3188d3b
to
e01db7f
Compare
Size Change: +7.08 kB (+1.11%) Total Size: 647 kB
ℹ️ View Unchanged
|
60dd522
to
21a3279
Compare
21a3279
to
35dcf3a
Compare
35dcf3a
to
2629de4
Compare
2629de4
to
a236a84
Compare
c18223d
to
a52c8c0
Compare
I've refactored the internals significantly:
|
f283524
to
a075ae0
Compare
} | ||
|
||
// Don't allow editing the value when the input is disabled or read-only | ||
if (input.disabled || input.readOnly) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to be explicit about this if the input is disabled ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, the onKeyDown
event handler is still called when the input
disabled. Surprised me as well 😳
let newValue: number; | ||
|
||
const getValue = (offset: number) => | ||
value ? shiftInRange(value, offset, min, max) : defaultValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
month: { | ||
value: values.month, | ||
'aria-valuetext': values.month | ||
? [values.month, getMonthName(values.month, locale)].join(', ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
@@ -45,7 +45,7 @@ import { | |||
type SidePanelContextProps, | |||
} from './SidePanelContext.js'; | |||
|
|||
vi.mock('../../hooks/useMedia'); | |||
vi.mock('../../hooks/useMedia/index.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why so explicit ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The final ES module spec requires full file paths (ref). I'm surprised that this worked before; I assume Vite does some magic under the hood to support ambiguous file paths.
Addresses DSYS-656.
Purpose
input[type="date"]
is a rare example of a native element that doesn't meet accessibility requirements and warrants a custom component. The native component also lacks customizability to disable specific dates or allow the selection of a date range.Approach and changes
Definition of done