feat: CalendarPreview range selection - #897
Open
Shreyag02 wants to merge 2 commits into
Open
Conversation
PR 4 of 7. `selection='range'` on the root, `field="start" | "end"` on
`.Input`, and the from/to machine that ties them together.
BREAKING CHANGE: a range emits only once it is complete. `onSelect` used
to fire on every step with a partial `{ from?, to? }`, and the docs told
consumers to gate on `range.to`. `onValueChange` now fires with both
edges or not at all, `to` stops being nullable, and the gate-on-range.to
idiom retires. Anyone reading the first-click event loses it. No type
error will find this: the old partial satisfies the new shape whenever
`to` happened to be set, so the failure is a callback that stops firing
rather than one that stops compiling.
The half-built range stays internal. It is on the root context so the
grid can draw the track between endpoints, and it is never emitted. The
restart case leaves the consumer's value at the previous complete range
until the new one completes; Escape or closing drops the draft.
The machine is the shipped one, branch for branch: an empty range takes
the first click as `from` and moves focus to the end field; a later
second click completes and closes; an earlier one becomes the new
`from`; a click on a complete range restarts.
Completing writes to open state, which the grid must not do directly, so
it routes through the root's `setOpen` — a consumer controlling `open`
keeps it open and only sees the request.
`lock` is replaced by `readOnly` on one `.Input`. The endpoint registers
itself, because `readOnly` is the input's prop and the grid is the thing
that has to refuse the write. A read-only endpoint with no value makes
the range unsatisfiable — the free endpoint sets, nothing ever completes
— so it needs a value; the docs say so.
Selection arms are discriminated on `selection`, so a single-day
consumer keeps a `Date | null` callback rather than both arms widening
to a union. The implementation stays shared, with one cast at the seam.
Range styling from the frames: endpoints accent-filled and pill-rounded
on their outer edges, the days between on one continuous band rather
than three cell backgrounds. react-day-picker marks every day of a range
`selected`, so the days on the track needed the single-day white text
undone — caught by rendering it, not by reading the CSS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Completing a range closes the popover, the browser hands focus back to the trigger, and the focus handler opened it again — the same shape as the Escape reopen, but the auto-close reason was not in the blocked set so the guard let it through. `closePress` joins `escapeKey` and `triggerPress` there. Emitting `itemPress` was also wrong: it is not in the popover's reason union, so the close was carrying a reason Base UI's own type says cannot occur. `closePress` is in the union and is what this is. jsdom passed throughout, because it does not restore focus to the trigger the way a browser does. Found by driving real Chrome over CDP. The regression test asserts the guard rather than the symptom, since the symptom is not reproducible in jsdom. Real browser, trusted input, after the fix: 10 after 2nd click emit: 10-20 10 auto-closed: true 10 range open log: true:trigger-press,false:close-press Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 tasks
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.
Summary
PR 4 of 7 in the RFC 005 stack, on top of #896.
Adds
selection='range'. Not an export — the same parts, withfieldon the inputs.Warning
Breaking, and no type error will find it. A range now emits only when complete.
{ from?, to? }torange.togateThe old partial satisfies the new shape whenever
tohappened to be set — so this is a callback that stops firing, not one that stops compiling.selection='range',.Inputfield, the from/to machine, range stylingselection='multiple'(cut from v1), scale views (PR 5)Changes
selection='range'—value/defaultValueare{ from: Date; to: Date }, no nulls on either edge..Inputgainsfield="start" | "end"— two addressable inputs, not astartProps/endPropsbag.lock→readOnlyon one.Input— the endpoint registers itself; the grid refuses to rewrite it.The click machine — the shipped one, branch for branch
from, moves focus to the end fieldfromonly, later dayfromonly, earlier dayfromTechnical Details
setOpenopenkeeps it open and only sees the requestselectionDate | nullcallback widens to a union. Implementation stays shared, one cast at the seamreadOnlyendpoint registersreadOnlyis the input's prop; the grid is what refuses the writeCalendarPreviewContextValue<Value>is already generic.CalendarPreviewDaysProvideris view-scoped and the wrong homeRange styling — endpoints accent-filled and pill-rounded on their outer edges; the days between on one continuous band, not three cell backgrounds. react-day-picker marks every day of a range
selected, so the track inherited the single-day white text and rendered grey-on-grey. Caught by rendering it, not by reading the CSS.Review notes
mode="multiple"— consumer search done, zero usersshowOutsideDays: falsedefault rather than reproduced heremode="multiple"consumer searchCalendar mode="multiple"is genuinely shipped API —CalendarProps = DayPickerProps & …types it, andmode='single'sits before{...props}incalendar.tsx, so a consumer'smodeoverrides it and reaches react-day-picker. Searched both orgs (private repos included) for every quote and brace form:mode="multiple",mode='multiple',mode: "multiple",mode={'multiple'},mode={"multiple"}pixxelhq/ff_mission_dashboard— AG Grid'sconfigure_selection(selection_mode="multiple")in Python. UnrelatedEvery real Apsara
Calendarusage found is explicitly single:pixxelhq/studioEditTaskEndDatePopup.tsxmode='single'pixxelhq/studioEndDateCell.tsxmode='single'pixxelhq/studioDateRangePicker.tsxDatePicker, nomodeNothing to migrate for
multiple. One caveat on scope: the search covers theraystackandpixxelhqorgs, not consumers of the published npm package elsewhere.Noted in passing, for phase 6 rather than for this PR —
studio/DateRangePicker.tsxis a full runtime consumer ofDatePicker(popoverProps,dateFormat,onSelect,inputProps,calendarProps) and also derives types from it viaComponentProps<typeof Calendar>['disabled']. It's already on the phase-6 migration list on the runtime usage alone, so the type coupling adds no separate work — worth knowing only because those three prop bags are exactly theslotPropsshapeCalendarPreviewdrops.Test Plan
range.test.tsxcalendar-preview/totalbiome check/tsc --noEmitbuild:apsara/ docs buildCovered — every machine branch including restart; no emit on the first click; one emit with both edges on completion; focus advances to the end field;
data-range-start/-middle/-endon the right cells; a controlled range renders without a click; each endpoint shows in its own field; a read-only endpoint is never rewritten by a grid click; auto-close fires throughonOpenChange, and a controlledopenwins.Real browser, trusted input — CDP-driven, because jsdom does not restore focus to the trigger after a close:
10-20, auto-closes and stays closedThis caught a bug the 3104 jsdom tests did not: completing a range closed the popover, the browser handed focus back to the trigger, and the focus handler reopened it —
true:trigger-press, false:item-press, true:trigger-focus.closePressnow joinsescapeKeyandtriggerPressin the blocked-reason set. Fixing it also surfaced thatitemPressis not in the popover's reason union at all, so the close had been carrying a reason Base UI's own type says cannot occur.Visual — two-month range rendered against the Figma frame: endpoints pill-rounded outward, one connected band between them, today's dot below the number and independent of the range.
SQL Safety (if your PR touches
*_repository.goorgoqu.*)Not applicable — TypeScript and CSS only. No Go files, no database access.