-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
feat(datetime): isDateEnabled to enable/disable specific days #24898
Conversation
@@ -170,6 +170,60 @@ console.log(formattedString); // Jun 4, 2021 | |||
|
|||
See https://date-fns.org/docs/format for a list of all the valid format tokens. | |||
|
|||
## Disabling Dates |
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.
We will need framework-specific code as well. You can add examples in the usage
directory.
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.
Added per-framework examples. Let me know if it would make sense to rip out the code snippets at this level. The goal was to document a few common use cases. Possibly those should be handled per each framework though. Playground would be ideal here.
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.
Couple more small tweaks and then this LGTM 👍 Great work on this! I love how elegant the actual component change was.
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.
One behavior I don't think we accounted for is how to skip over dates when using the keyboard.
If you focus day 11 on the first datetime in: http://localhost:3333/src/components/datetime/test/disable-dates, it's not possible to use the arrow keys to navigate backwards anymore to reach day 9.
When using the arrow keys, should it skip over the date entirely? Maybe it could highlight it but indicate that it is disabled? We may need to do some additional research here.
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
@liamdebeasi I don't know if we should be doing anything custom with the keyboard navigation here. This aligns with the browser behavior of sibling disabled buttons. https://codepen.io/sean-perkins/pen/XWVryXw Users should be able to tab-over disabled buttons, but not use left/right keyboard navigation to access them. With a screen reader/voice over, they would be able to focus that node, hear that it is disabled and focus the next/previous node. Let me know if you feel different or if we have a native example that operates differently. |
Ah good point 👍 I checked MUI and they have the same behavior: https://codesandbox.io/s/basicdatepicker-material-demo-forked-lc6zdw?file=/demo.js. I'm fine leaving things the way they are. |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally and any changes were pushednpm run lint
) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Developers do not have the ability to disable specific days, ranges of dates, holidays, weekends, etc.
Issue URL: #24209
What is the new behavior?
Developers now have full access to each rendered calendar day, allowing them to write custom logic to enable or disable dates. Developers can use this property to disable a specific date, date range, weekends, etc.
isDateEnabled
toion-datetime
isDateEnabled
will default to enabling all dates (min/max rules will overrule this behavior)isDateEnabled
accepts an ISO 8601 date string for each rendered calendar dayDoes this introduce a breaking change?
Other information