feat: CalendarPreview scale-aware selection - #898
Open
Shreyag02 wants to merge 6 commits into
Open
Conversation
PR 5 of 7. `scales` and `trailingValue` on the root, and eight parts:
`.Picker`, `.Label`, `.Scales`, `.Scale`, `.Separator`, `.Panel`, and the
four period views.
This is the surface that forced the value contract. A `Date` cannot say
whether it means "August 2026" or "1 August 2026", so beyond day scale
the value is a `ScaleValue` — `{ date: 'YYYY-MM-DD', scale }` — and the
scale travels with it rather than with a prop.
Every date computation goes through `lib/scale.ts`: `periodOf`,
`anchorOf`, `convertScale` and `isAvailable`. Nothing here does period
maths, and no component imports date-fns.
Availability tests the date a period would PRODUCE, not the period, so
the same period answers differently at each end of a pair. With a bound
of 15 July 2026, Q3 2026 is disabled for a start field (emits 1 July)
and available for an end field (emits 30 September). That is the RFC's
table, and it is the fixture.
A scale switch moves the view and sets a draft; it emits nothing. A cell
click or Enter commits. Escape drops the draft AND restores the scale
the value carries — without that the input still reads "Q3 2026" for a
day value, which the test caught.
`.Days` becomes a sibling view that gates on the day scale, the way the
four period views do, so `.Panel` can mount all five and a consumer can
mount `.Quarters` alone. That is a behaviour change for `.Days` and is
why the day-only default matters: at `scales='day'` the scale is always
'day', so an inline calendar is unaffected.
The period lists are one scrolling column with year headings inside it,
and open scrolled to the active year — a twenty-year list otherwise
opens on 2016, which the tests found first.
Open Item 1, the `scales` discriminator: TypeScript cannot test an
array's contents, so the arms discriminate on the SHAPE of `scales`.
Omitted or the literal 'day' keeps `Date`; any other scale, or any
array, moves to `ScaleValue`. The wart is that `scales={['day']}` takes
the scale-aware arm where `scales='day'` does not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (24)
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 |
commit: |
Open Item 2 in the RFC, settled. `.Picker` overloaded the old `DatePicker` vocabulary for what is just the popup body, and `.Field` would have collided with Apsara's `Field`. Renames the part, its props type, its display name and its `data-slot`. The slot moves from `calendar-preview-picker` to `calendar-preview-body`, which is semver-covered surface — it has never shipped, so this costs nobody, but it is the last chance to make it free. While here: the eight parts added in the previous commit were registered on the root but their props types were never exported. They are now, from both barrels, so a consumer can type a wrapper around any of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Settles the last open item. The RFC set the default day format to DD/MM/YYYY; the frames and the shipped `DatePicker`'s own `dateFormat` both render `15 Aug 2026`. Going with the frames. `formatDayLabel` was day-first for a stated reason — a rendered value could be typed straight back into the field, because `lib/parse.ts` accepted exactly what it produced. Changing the format alone would have broken that: `parseScaleInput` had no pattern for a day with a month name, so selecting all and retyping `15 Aug 2026` verbatim came back unparseable. So the parser learns the form the formatter renders. `15 Aug 2026` and `15 August 2026` now parse at day scale, and `31 Feb 2026` is still rejected, because `dayKeyFromParts` validates against the real calendar rather than rolling forward. Every input form that worked before still works — the slashed and ISO shapes are untouched, they are simply no longer what gets rendered. The multi-scale placeholder advertises the new form too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`toDate()` was already right for the scale arm — `selectPeriod` passes the produced date as the occasion, so it hands back the period edge that `trailingValue` chose, as a method rather than a field. `period` was not. It was computed against the root's current `scale` state, which is the scale on SCREEN, not the one being committed. On a click those agree, because switching the view is what put the cells there. On a typed commit they do not: "Q4 2026" typed while the view is still on days committed a quarter but reported a single day as its period. It now derives the scale from the value being emitted, so the two cannot drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR 5 shipped ten parts with no docs, so the scale surface was invisible on the docs site — which is where it was noticed. Adds the API entries for `.Body`, `.Scales`, `.Scale`, `.Panel`, the four period views, `.Label` and `.Separator`, the eleven slots they render, and a section covering the pieces that are not guessable from the props: that the value carries its own scale, that switching drafts rather than emits, what `trailingValue` does to the value, and the availability table that falls out of it. Two things the section has to say out loud, because both have already caused confusion: `ScaleValue.date` is stored as `YYYY-MM-DD` and is never what renders — `formatValue` puts `DD MMM YYYY` on screen and `toDate()` hands back a `Date`; and a start/end pair is two independent roots, not `selection='range'`, because the two ends can hold different scales. The first demo tab is the inline body rather than the popover form. The popover renders as the words "Add start date" until you click it, which is exactly why the preview looked missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The list claimed to open on the active year and never did — a real browser showed `scrollTop: 0` with the 2026 group 540px down a 320px viewport. Every scale switch landed the user twenty years early, on 2016, and clicking what looked like "Q3" committed Q3 2016. Two causes, both invisible to jsdom. The effect ran on mount, but `.Panel` mounts all five views at once and a view still runs its hooks while it returns null. So the effect fired with an empty ref, and a mount effect never fires again when the view later becomes visible. It now runs when the view becomes active. `scrollIntoView` was also the wrong instrument: it walks every scrollable ancestor, so it would move the popover along with the list. Scrolling the container directly touches nothing else. Separately, and found by the same probe: `switchScale` and the period list both anchored on `today` rather than on `month`. A consumer opening on 2030, or a user who navigated there in the day grid, was thrown back to this year by switching scale. Both now follow the month on screen — which already falls back to today when nothing else set it. jsdom cannot see any of this: it has no layout, so `scrollTop` is always 0 and `getBoundingClientRect` is always zeroes. The tests cover the anchor, which is observable; the scroll is verified in a browser. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 5 of 7 in the RFC 005 stack, on top of #897.
Selection at scales coarser than a day. This is the surface that forced the value contract — a
Datecannot say whether it means "August 2026" or "1 August 2026", so beyond day scale the value carries its own scale.scales,trailingValue, 10 parts, and docsselection='multiple', presets, time-of-day, any pre-composed exportChanges
.Body.Scales/.ScaleTabs. Renders nothing at one scale.Panel.Months/.Quarters/.HalfYears/.Years.Label/.Separatorlib/scale.ts—periodOf,anchorOf,convertScale,isAvailable. Zero date-fns imports outside the adapter..Inputparses every offered scale and moves the scale to match."Q4"in a day-only field is a typo, not a quarter.trailingValuechanges the value, not the formatting — a start field emits the period's first day, an end field its last.scalesandtrailingValue. "1 Aug 2026 → Q3 2026" is not expressible as one range value.Storage vs display
Two layers, easy to conflate:
ScaleValue.date2026-08-15<and>and no parsing. Also dodgesnew Date('2026-08-01')rendering as 31 July at negative offsets — the RFC review's reason for itformatValue15 Aug 2026No
YYYY-MM-DDreaches a screen.details.toDate()hands back aDate.Availability depends on which end you are
The reason
isAvailabletakestrailing. The RFC's table, bounded at 15 July 2026:Every period starts before the bound. Only the produced date separates them.
Technical Details
.Daysgates on the day scale.Panelmounts all five and.Quarterscan stand alone. Atscales='day'the scale is always'day', so an inline calendar is unaffected"Q3 2026"— the input reformats at the drafted scalemonth, nottodaymonthalready falls back to todayscrollIntoViewwould have dragged the popover with itperiodderives from the committed value's scalescalestate — the scale on screen. Typing"Q4 2026"committed a quarter but reported a single daygrid.tsxholding.Grid/.Day/.WeekdayFound in a real browser, not by tests
jsdom has no layout:
scrollTopis always0andgetBoundingClientRectalways zeroes. Two bugs hid there.scrollTop: 0, 2026 group 540px down a 320px viewport. Every switch landed on 2016; clicking "Q3" committed Q3 2016scrollTop: 540,visible: true, commits2026-07-01, trigger readsQ3 2026That is the fourth bug this stack's browser probes have caught that the jsdom suite could not — three focus/dismissal, one layout. Worth extending the RFC's real-browser exit criterion beyond phase 2 to anything touching open state or scroll position.
Review notes
scalesdiscriminator (RFC Open Item 1).Picker's name (RFC Open Item 2).Body. Slot iscalendar-preview-bodyDD MMM YYYY— matches the frames and the shippedDatePicker. The RFC'sDD/MM/YYYYis superseded and needs updatingOn the day format —
formatDayLabelwas day-first so a rendered value could be typed straight back in, which only held becauselib/parse.tsaccepted exactly what it produced. Changing the format alone would have broken that, so the parser learned the rendered form:15 Aug 2026and15 August 2026now parse at day scale. Every previously accepted input still works.On the discriminator — TypeScript cannot test an array's contents, so the arms discriminate on the shape of
scales:scalesvalue'day'Date(orDateRangeValue) — unchangedScaleValueThe wart:
scales={['day']}takes the scale-aware arm wherescales='day'does not. An explicit second prop would avoid it, at the cost of a redundant API.Test Plan
scale-selection.test.tsxcalendar-preview/totalbiome check/tsc --noEmitbuild:apsara/ docs buildCovered — a scale switch fires no
onValueChangeuntil a pick;trailingValueflips the emitted date for all four period scales, month-end correct in a leap February (2028-02-29); the availability table asserted from both ends;.Scalesrenders nothing at one scale; each view renders alone with no day grid in the tree; the trigger annotation formats all five scales with no popover open; Escape drops the draft and restores the input;.Inputmoves the scale to match typed text and refuses a scale the root does not offer; the rendered day form round-trips back through the parser and31 Feb 2026is rejected;toDate()returns the produced period edge andperiodis right on both the clicked and typed paths; scale switching anchors on the view month, not today.Real browser, trusted input — 14 cases across the day picker, range and scale surfaces: focus-to-open with no flicker, Escape, outside press, Enter and outside-click commits, range completion auto-closing and staying closed, scale switch emitting nothing, period commit auto-closing and staying closed, and the period list opening on the active year.
SQL Safety (if your PR touches
*_repository.goorgoqu.*)Not applicable — TypeScript and CSS only. No Go files, no database access.