feat: add datetime-picker component #1370
Conversation
…or forward and back month arrows
✅ Deploy Preview for astro-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for astro-stencil ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment was marked as resolved.
This comment was marked as resolved.
|
…te in calendar when iso is given by default
… in favor of only passing ISO to calendar
|
Functionality wise - couldn't find a thing wrong when testing them all! Very impressed 👏🏼 |
… copy/paste tests
…ter rendered with a given value
brothhammer
left a comment
brothhammer
left a comment
There was a problem hiding this comment.
Wow! this is really impressive and myself coming in at the last minute is struggling to understand how you have kept everything straight for so long.
I have only dug into rux-datetime-picker.tsx. There are places where we are making fairly expensive calculations in the render method that would be good to memoize or in some cases move to a constants file (like the static regex). I also took a shot at code splitting this file on a seperate branch #1408
For maintainability our future selves and other developers I think we need to continue to explore ways to break up rux-datetime-picker.tsx and rux-calendar.tsx. In my view it is okay to have very long files when that file is doing the same thing over and over again (like a testing file or routing file). I'm not married to how I split things and was making some pretty quick decisions.
Additionally given the size of this PR I would greatly appreciate a code walk through meeting to better understand.
Again this is very impressive and I believe the functionality is there. Just want to move it in a direction that can be more easily understood. Happy to pair up or look further into how memoization works in Stencil to keep this moving.
| this.isCalendarOpen = !this.isCalendarOpen | ||
| } | ||
|
|
||
| determineMinMax(type: PartKey, isMicro: boolean, isJulian?: boolean) { |
There was a problem hiding this comment.
This is a very expensive calculation in the render method. We should look into a way to memoize in Stencil. This method is called multiple times per render cycle but the result only depends on minYear, maxYear and JulianFormat - that don't change often. We want to avoid frequent recalculations of the same min/max values.
| <input | ||
| key={i} | ||
| class={{ | ||
| part: true, |
There was a problem hiding this comment.
This class object is in the render method and recreated with each render. It only needs to be recreated if type or precision changes.
| //check if incoming value is in an Oridnal ISO format. If so, convert it to gregorian since we're not in julianFormat. | ||
| const ordinalFormatMatch = value.match( | ||
| /^([0-9]{4})(?:-([0-9]{1,3}))?(?:T([0-9]{2})(?::([0-9]{2}))?(?::([0-9]{2})(?:\.([0-9]{1,6}))?)?Z?)?$/ | ||
| ) |
There was a problem hiding this comment.
Regex compilation is expensive and this is static. We should move this so it doesn't need to get recompiled for every call of handleInitialValue
Brief Description
Adds the
rux-datetime-pickercomponent. This component uses the newrux-calendarandrux-daycomponents.General Notes
Props
<rux-datetime-picker>precision: Controls the granularity of the time (hour, min, sec, ms)julian-format: Controls whether or not the datepicker is in julian modemax-year: The max year value the datepicker can havemin-year: The min year value the datepicker can havehelp-text: The help text. Same as ourrux-inputerror-text: The error text. Same as ourrux-inputdisabled: renders the datepicker as disabled.label: Controls the label text for the datepickername: Controls the name of the datepicker inputvalue: The value of the datepicker. Must be a valid ISO or Julian ISO stringsize: The size of the datepicker. small, medium or large<rux-calendar>and<rux-day>both have their own props which are passed down the component tree originating from<rux-datetime-picker>. These props are internal for the time being, until the need for a standalone calendar arises.Events
<rux-datetime-picker>Motivation and Context
Adds an astro component for handling datetime selection. Provides users with a julian formatted datetime picker.
Issues and Limitations
Types of changes
Checklist