Skip to content

Switch custom date range picker to two-click calendar selection#20

Merged
mariusvniekerk merged 9 commits into
mainfrom
t3code/calendar-range-selector
Jul 7, 2026
Merged

Switch custom date range picker to two-click calendar selection#20
mariusvniekerk merged 9 commits into
mainfrom
t3code/calendar-range-selector

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replaced the Custom tab’s native date inputs with an embedded calendar that selects a range in two clicks.
  • Added pending-range state so the UI shows which endpoint the next click will set, and only commits once the range is complete.
  • Kept committed custom ranges ordered even when the second click is earlier than the first.
  • Updated the component docs and demo copy to reflect the new interaction.
  • Added browser coverage for the calendar-only Custom tab, ordered range commits, and swapped endpoints.

Testing

  • Not run locally.
  • Browser test added: verifies the Custom tab renders a calendar and no native date inputs.
  • Browser test added: verifies two clicks commit an ordered custom range and the readout tracks the pending endpoint.
  • Browser test added: verifies an earlier second click swaps the range ends.

@mariusvniekerk
Switch custom date range picker to two-click calendar selection
51537a4 
- Replace custom tab date inputs with embedded calendar range picking
- Show pending from/to endpoint state and commit only complete ordered ranges
- Add browser coverage for calendar-only custom selection and swapped clicks
@roborev-ci

roborev-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (51537a4)

Summary verdict: One medium issue remains; no critical or high findings were reported.

Medium

  • src/lib/components/DateRangePicker.svelte:173: seed() always clears customPending, so a controlled incomplete custom range with from set and to empty opens with the From endpoint active. The next click overwrites the existing start instead of completing the range.
    • Fix: Preserve incomplete custom state when seeding, for example set pending when customFrom && !customTo and anchor customMonth to customTo || customFrom || todayStr().

Reviewers: 2 done | Synthesis: codex, 7s | Total: 4m40s

@mariusvniekerk @claude
Preserve mid-pick custom state when the panel reopens
c02a37d 
Review finding on 51537a4: seed() unconditionally cleared customPending,
so a controlled incomplete custom selection (from set, to empty — a
state the trigger label already supports via customRangeLabel) reopened
with the From endpoint armed. The next calendar click then overwrote
the existing start instead of completing the range the consumer handed
in, silently discarding their chosen start date.

Seeding now arms the To endpoint whenever from is set without to, and
the custom tab's month anchors to the start date in that state (falling
back to today only when both ends are empty), so reopening lands on the
month the pick began in.

Validation: svelte-check 0/0, bun test checks/ (143), fmt:check, the 3
date-range-picker browser specs.

Generated with Claude Code (claude-fable-5)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (c02a37d)

Verdict: One medium issue remains in the custom date-range draft flow.

Medium

  • src/lib/components/DateRangePicker.svelte:245
    The first custom-calendar click only updates local state and returns before onSelect, so the controlled selection never records the in-progress range. If the popover is dismissed after that first click, reopening calls seed() from the old selection and clears the pending start date, forcing the user to restart the range.
    Fix: Persist the pending draft across close/reopen, or emit an explicit incomplete custom selection on the first click and cover that state in docs/tests.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 4m59s

@mariusvniekerk @claude
Keep the custom-range draft when the popover is dismissed mid-pick
118b0ef 
Review finding on c02a37d: the first custom-tab click is deliberately
local — emitting {from, to: ""} through onSelect would push an
unresolvable range into consumers that reload data on every commit —
but seed() reseeded from the committed selection on every open, so an
Escape or outside click between the two picks silently threw away the
chosen start date.

seed() now returns early onto the Custom tab while a draft is pending,
so the start survives dismiss/reopen. The draft is keyed to the
selection object it was started against and is dropped when the
controlled selection changes while closed, so an externally swapped-in
range can never resurface a stale start date.

Validation: svelte-check 0/0, bun test checks/ (143), fmt, the 4
date-range-picker browser specs including a new dismiss/reopen spec.

Generated with Claude Code (claude-fable-5)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (118b0ef)

Medium issue found; no Critical or High findings.

Medium

  • src/lib/components/DateRangePicker.svelte:169
    The pending custom draft is tied to the controlled selection by object identity instead of by value. If a parent mutates the same selection object while the popover is closed, the stale draft can be incorrectly preserved; if it recreates an equivalent selection object, the draft can be incorrectly dropped.
    Fix: Store and compare a value snapshot/key for selection when the draft starts, or clear pending state reactively when the selection values change.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 4m4s

@mariusvniekerk @claude
Key the mid-pick draft by selection value; document the commit-only c…
52add7c 
…ontract

Roborev findings on 51537a4 and 118b0ef.

The stale-draft guard compared the controlled selection by object
identity, silently assuming replace-only consumers: a parent mutating
a $state selection in place while the popover was closed would keep a
stale draft alive, and one recreating an equivalent object would drop
a draft the user still wanted. The guard now snapshots a canonical
value key of the selection when the draft starts and compares by value
on reopen, so neither consumer style misbehaves.

The docs still promised onSelect 'on every edit', which the two-click
custom flow deliberately broke — the first click is internal so
consumers that reload data per commit never see an unresolvable
half-range. The controlled contract now reads commit-only, selection
is documented replace-only, and the mid-pick draft lifecycle (survives
tab switches and dismiss/reopen, cleared by any commit, dropped on
external value change) is spelled out instead of implied.

Skipped: a browser spec for the external-selection-change-while-closed
path — the demo only changes selection through the picker itself, so
that path isn't drivable without demo-only test hooks.

Validation: svelte-check 0/0, bun test checks/ (143), fmt, the 5
date-range-picker browser specs including a new tab-switch-while-
pending spec.

Generated with Claude Code (claude-fable-5)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (52add7c)

Summary verdict: One medium issue needs attention; no critical or high findings were reported.

Medium

  • src/lib/components/DateRangePicker.svelte:194
    seed() can create a pending custom draft from a controlled incomplete custom selection, but leaves pendingFor keyed to an older draft. A later reopen can match that stale key and preserve the wrong customFrom/customTo, allowing a stale range to be completed against a different committed selection.
    Fix: After a full reseed, reset the draft key, e.g. pendingFor = customPending ? selectionKey(selection) : null, and add test coverage for external selection changes.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 5m19s

@mariusvniekerk @claude
Re-key the mid-pick draft on every reseed, not just on completion
4f94a8e 
Review finding on 52add7c: seed()'s full-reseed path could arm a pending
custom draft (from a controlled incomplete custom selection) while leaving
pendingFor pointing at an OLDER draft's key. The sequence that breaks:
draft started under selection A keys pendingFor=key(A); the app swaps in
an incomplete custom selection B while closed, so reopen reseeds and
re-arms customPending from B but the key still reads key(A); the app then
swaps back to A, and the next reopen sees key(A)===pendingFor and adopts
B's stale customFrom/customTo as A's draft.

seed() now sets pendingFor = customPending ? selectionKey(selection) : null
on the reseed path too, so the key always tracks the selection the armed
draft actually belongs to.

Adds demo hooks (External buttons that replace the committed selection
while closed) and three browser specs the external-change paths were
previously undrivable without: draft dropped on a differing external
selection, the stale-key regression above, and a controlled incomplete
custom selection reopening armed to complete.

Validation: svelte-check 0/0, bun test checks/ (143), fmt:check, lint,
check:usage 0 findings, the 8 date-range-picker browser specs.

Generated with Claude Code (claude-fable-5)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (4f94a8e)

Summary verdict: one medium issue remains; no critical or high findings were reported.

Medium

  • src/lib/components/DateRangePicker.svelte:268
    pickCustomDay completes a pending draft without re-checking that the current controlled selection still matches pendingFor. If the parent changes selection while the popover remains open, the next custom click can commit a stale draft over the newer external selection.
    Fix: At the start of pickCustomDay, when customPending is true, compare selectionKey(selection) with pendingFor; if it differs, drop or reseed the draft and treat the click as a fresh first pick.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 3m52s

@mariusvniekerk @codex
Drop stale custom drafts before completing a pick
e68ec41 
Review found that the reopen guard did not cover controlled selection changes while the DateRangePicker popover stayed open. In that state, the next custom calendar click could complete a draft that belonged to the old selection and overwrite the newer parent-controlled range.

The pending draft is now rechecked against the current selection key at completion time, so a mismatch restarts the custom draft instead of committing stale bounds.

Validation: date-range-picker Playwright spec with a temporary isolated config on port 4298; bun run check; bun test checks/.

Generated with Codex (GPT-5)
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (e68ec41)

Medium issue found; no high or critical findings.

Medium

  • src/lib/components/DateRangePicker.svelte:177 - A stale custom draft can be resurrected if the parent changes selection away from the draft's key while the popover is closed, then changes it back before reopening. The current seed() check only compares the final current key to pendingFor, so it misses the intervening external change and can complete an old start date against a newer controlled selection.

    Suggested fix: Invalidate pending drafts reactively when selectionKey(selection) changes to a value different from pendingFor, rather than waiting until seed()/pickCustomDay; add a regression test for closed-popover selection A -> B -> A before reopening.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 3m18s

@mariusvniekerk @codex
Invalidate stale custom drafts on selection drift
9cc6daa 
Review found that checking a pending draft only on reopen or completion misses a closed-popover A-to-B-to-A sequence: once the parent briefly controls a different selection, the old draft no longer belongs to the restored key.

The picker now clears pending draft state as soon as the controlled selection diverges from pendingFor, so a later return to the same key cannot resurrect an old start date.

Validation: date-range-picker Playwright spec with a temporary isolated config on port 4298; bun run check; bun test checks/.

Generated with Codex (GPT-5)
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (9cc6daa)

Summary verdict: one medium issue remains in the date range picker UI state handling.

Medium

  • Location: src/lib/components/DateRangePicker.svelte:290
  • Problem: When selection changes while a custom draft is pending and the panel is open, the effect clears customPending/pendingFor but leaves customFrom, customTo, and customMonth pointing at the stale draft. The UI can still display and highlight the dropped start date until the next click.
  • Fix: When invalidating a stale draft, also reseed or clear the custom readout/calendar state from the current selection while preserving the intended tab behavior.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 6m32s

@mariusvniekerk @codex
Reseed custom UI when pending drafts go stale
f79bca0 
Review found that invalidating a stale pending draft only cleared the internal pending flags. When the panel was still open, the Custom tab could continue showing the dropped start date and one-day highlight even though the controlled selection had already changed.

Invalidation now reseeds the custom readout and calendar state from the current controlled selection while leaving the active tab alone, so the open panel reflects the external range immediately and still treats the next click as a fresh pick.

Validation: date-range-picker Playwright spec with a temporary isolated config on port 4298; bun run check; bun test checks/.

Generated with Codex (GPT-5)
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (f79bca0)

Medium issue remains: controlled calendar state can drift after an external selection change.

Medium

  • src/lib/components/DateRangePicker.svelte:301
    When an external selection change invalidates a pending custom draft while the popover is open, only the custom fields are reseeded. If the new controlled selection is calendar, calUnit, calAnchor, and calMonth remain from the previous selection, so switching to the Calendar tab can show and commit the wrong period.
    Fix: On drift, perform a full state reseed from selection, or update the calendar state alongside the custom fields while intentionally preserving the current tab if desired.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m48s

@mariusvniekerk @claude
Reseed calendar state alongside stale-draft custom fields
e4bb046 
Review found that draft invalidation reseeded only the Custom tab's fields. With the panel open, calUnit/calAnchor/calMonth kept the pre-drift selection's values, so switching to the Calendar tab after an external selection change showed the old period — and a unit-pill click would commit that stale anchor over the newer parent-controlled range.

Invalidation now runs the same full reseed the popover performs on open (extracted as reseedFromSelection), minus the tab switch: the active tab is deliberately preserved so an open panel doesn't jump tabs under the user.

Validation: full Playwright suite (47 passed) including a new drift-to-calendar regression spec driven by a new demo hook; bun run check; bun test; fmt/lint clean.

Generated with Claude Code (claude-fable-5)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@mariusvniekerk

mariusvniekerk commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in e4bb046: draft invalidation now runs the same full reseed as popover open (extracted as reseedFromSelection()), so calUnit/calAnchor/calMonth track the new controlled selection; the active tab is intentionally preserved. Added a drift-to-calendar regression spec (tests/browser/date-range-picker.spec.ts) driven by a new demo hook.

generated by a clanker

@roborev-ci

roborev-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (e4bb046)

Summary verdict: one Medium test flake should be fixed before relying on this coverage.

Medium

  • tests/browser/date-range-picker.spec.ts:160
    The test expects the “last 7 days” range to render 7 selected calendar cells, but near the start of a month some selected days can fall outside the visible month grid, making the test date-dependent and flaky.
    Fix: Use a fixed visible range/demo hook, compute expected visible selected cells from the rendered grid, or assert the draft reset without relying on a hard-coded selected-cell count.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m46s

@mariusvniekerk mariusvniekerk merged commit ec894f6 into main Jul 7, 2026
6 checks passed
@mariusvniekerk mariusvniekerk deleted the t3code/calendar-range-selector branch July 7, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mariusvniekerk