feat(core): add DateTimeInput timeOptionInterval for a preset-time dropdown - #4837
Open
AKnassa wants to merge 2 commits into
Open
feat(core): add DateTimeInput timeOptionInterval for a preset-time dropdown#4837AKnassa wants to merge 2 commits into
AKnassa wants to merge 2 commits into
Conversation
Adds an opt-in `timeOptionInterval` prop that turns the time field into an
APG combobox over a listbox of preset times at a 5/10/15/30/60-minute
cadence. Click or Alt+ArrowDown opens it, arrows move the active option,
Enter commits, Escape closes, and typing moves the highlight to the closest
option without filtering the list, so free-form entry keeps working.
min/max trim the options on the boundary date.
The dropdown is opt-in rather than default because the field's ArrowUp and
ArrowDown already step the value by timeIncrement, and because a second
always-on combobox would break every getByRole('combobox') query that
resolves to the date input. With the prop omitted the time field is
byte-identical to today: no combobox role, no listbox, no new attributes.
With the list closed, arrows keep stepping.
Guards three failure modes the popover stack creates: the calendar's onHide
no longer steals focus when the time list evicts it, options mount only
while the list is open, and the selected marker normalises seconds so an
external value of 14:00:00 still matches a 14:00 option.
Closes facebook#2727
|
@AKnassa is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
August 8, 2026 22:16
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsDateTimeInput (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
Adds 26 edge-case tests and fixes the nine defects they found. Real bugs: - Clicking the option already marked selected fired a change, because the commit compared the raw value while the marker compared a normalised one. - A min/max window narrower than the cadence opened an empty list, and one that emptied under an open list left it open, where the gated keyboard switch let the arrows silently step the value instead. - Opening on a field with a date but no time left no active option, so ArrowUp and Enter did nothing at all. - Enter discarded a typed off-cadence time and committed the option below it; it now prefers typed text only while the highlight still follows the typing, and honours the active option once the user arrows away. - A stale highlight survived the list shrinking, leaving no visible active option and arrow keys that looked dead. - Blur never closed the list, stranding it open in the top layer on a programmatic focus move. Now mirrors BaseTypeahead's relatedTarget guard. - A pointer press that never became a click wedged the light-dismiss deferral flag and held later keyboard opens hostage. - The listbox was named from `label` while its own input was named from `timeLabel`, so renaming the field left the list announcing the old name. - Option rows ignored `size`, leaving an sm field with md-sized rows. Part of facebook#2727
AKnassa
marked this pull request as ready for review
August 8, 2026 23:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Adds an optional dropdown of preset times to the time half of
DateTimeInput. SettimeOptionIntervalto5,10,15,30, or60and the time field offers a clickable list of times at that spacing.60gives the 12 AM to 11 PM list the issue asks for.Why
Today, setting a specific time means typing it or holding an arrow key. For "pick a meeting time" flows that is slow. Opening a list and clicking is the expected way to do this.
What changed
timeOptionIntervalprop onDateTimeInput. Leave it off and nothing about the component changes.minandmaxtrim the list on the boundary date, so only valid times are offered.date-time-input-time-listboxanddate-time-input-time-option.Why it is opt-in rather than always on
The issue suggests the list appear on focus and follow
timeIncrement. Two things in the current component make that a breaking change rather than an addition:timeIncrement, which is documented and tested. A list that is always open would have to take those keys. Instead the list opens on Alt+Down, and plain arrows keep stepping whenever the list is closed.DateTimeInput.test.tsxalone has 33 assertions using the singulargetByRole('combobox')to reach the date input, and those throw the moment a second one exists. The same breakage would hit consumers silently on upgrade.timeIncrementalso defaults to1, which would mean a 1440 row list, and its type cannot express60. So the list gets its own prop. Making it default-on later is a one line change if that is preferred.How to see it
Storybook: Core / DateTimeInput / With Time Options and With Hourly Time Options.
Notes for review
Four things surfaced while building that are worth a look:
popover="auto", so opening the list evicts the calendar and fires itsonHide, which refocused the date input. That is now guarded, and dismissing the calendar normally still restores date focus.14:00:00) withhasSecondsoff never matched a14:00option, so nothing showed as selected. Normalized before comparing.TimeInputdeliberately does not get this. It has real consumers that render it inside a popover, and it renders a different tree inside anInputGroup. Happy to file that as a follow-up.Closes #2727