✨ Add Bento Date Picker Component #37385
Conversation
In an attempt to share defaults and make it less confusing which props are coming from the provider and which props are getting passed into the component, this moves all default props to the provider and creates more type safety for the date picker components that get data from this provider. A potential downside of this change is that it makes it more difficult to use the SingleDatePicker component and RangeDatePicker in isolation if that is something we want to do later.
|
Can we obviate the need for some or all of |
|
@kvchari While it's defintely possible to write this logic on our own, I still think it's worth including the library, especially considering that we're replacing |
| @@ -0,0 +1,25 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Hi, thanks for your contribution.
Seems like the PR is missing a few validation related files, specifically a .protoascii file and a /test/validator-*.out file.
The .protoascii file should describe the validation rules, the .html file is a test sample, and .out is the expected output of validation.
Please add those files. In the .html file consider adding a few blocks that will fail validation in all possible ways worth testing
See https://github.com/ampproject/amphtml/blob/main/docs/component-validator-rules.md for details.
Feel free to browse other extensions for examples of validation rules and tests: https://github.com/ampproject/amphtml/tree/main/extensions.
There was a problem hiding this comment.
Hi @MichaelRybak, I just checked in and confirmed with @jridgewell that these changes aren't necessary for this PR. The work in this PR supports Preact and Bento mode, but not AMP mode for right now.
There was a problem hiding this comment.
@becca-bailey Got it. What about the test/validator*.html file? Is it used in some way in this PR or should it be part of the AMP-related PR?
There was a problem hiding this comment.
At the moment it sounds like there are some open questions about the future of these auto-generated AMP files. The current advice I have received is to leave them alone for now, and they will most likely be removed in a future PR once we know for sure that we will not be supporting AMP mode for bento components.
There was a problem hiding this comment.
Any thoughts here @dethstrobe ?
There was a problem hiding this comment.
Removing it sounds good to me.
There was a problem hiding this comment.
This file was causing some issues in CI. so I just deleted it for now.
| components={{Day: DayButton}} | ||
| disabled={[isDisabled]} | ||
| formatters={{ | ||
| formatCaption: formatMonth, | ||
| formatWeekdayName: formatWeekday, | ||
| }} |
There was a problem hiding this comment.
Something for us to maybe worry about later if performance becomes a slight issue would be to memoize these objects between renders. But I feel like that's an optimization we can make later, if we need it.
This is Part 1 of two PRs that will be put up for Bento Date Picker. Because this component had a large list of behavior to replicate, I wanted to divide up this work to keep the size of this PR a little more manageable. This set of work includes:
react-dates+momenttoreact-day-picker+date-fnsPart 2 will include:
react-day-picker)Notes on the migration to
react-day-pickerI made the decision to migrate away from
react-datesand use the beta version ofreact-day-pickerin order to reduce the bundle size and remove the dependency on moment.js. React day picker is under active development, uses modern react patterns, and was generally easier to use than react dates.Because of this, the new component doesn't replicate the behavior and styling of the original 100%. I would appreciate any feedback on whether there are important parts of the original component that need to be replicated exactly.
Bento date picker 1.0
Amp date picker 0.1
Issue: #37771