Repository navigation
fix(bulma-ui): stop the date pickers at year 1 - #919
Conversation
HTML's date, month and datetime-local inputs hold no year below 1, but the pickers let a date in year 0 or earlier through. With no min, the calendar's year list reached negative years, and typing, the default parse and the time wheels committed such dates. Every picker path now treats a date before year 1 the way it already treats one before min. The calendar disables those days and starts its year list at year 1, typing and parsing reject them, and a min before year 1 counts as 1 January of year 1, on the native inputs too. TimeInput applies the same floor to the date its value carries. Years 1 to 99 stay in range as written, and the YYYY token still pads them to four digits.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (18)
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 |
Preview DeploymentPreview URL: https://31296273.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 2 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Correctness | A max before year 1 leaves focusedDate in year 0, so the day header walks into negative years and the year dropdown renders empty |
bulma-ui/src/form/_pickerInternals/Calendar.tsx:467 |
| 2 | 🔵 Advisory | Robustness | The floor raises min but not toIsoValue, so the hidden form input of an inline picker still submits 0000-… / 00-1-… for a caller-supplied value before year 1 |
bulma-ui/src/form/DateInputBase.tsx:535 |
Overall: The change is sound and unusually well bounded. One helper (floorMin) carries the whole decision, each path applies it exactly where it already applied min, and for years ≥ 1 every edit is a provable no-op — the day-granularity prevDisabled simplification is equivalent once min is always defined, periodMin / lowerBound only add a lower comparison that normal dates pass, and the yearList default gains a Math.max(FIRST_YEAR, …) that is inert above year 101. The riskiest spot is that the floor is one-sided: min is raised, max is not, and clampDate prefers max when the two cross (finding 1). A human should look first at whether a max below the floor is worth defending at all, and second at whether a caller-supplied value before year 1 should still serialize as 00-1-06-15 into a submitted form field (finding 2) — both are product calls rather than defects, and a reasoned "caller error" settles either.
Residual risk:
- Calendar header below the floor — open, posted as finding 1. Reachable only with
maxbefore 1 January of year 1, where nothing is selectable either way; pre-PR the same config also focused year 0, so it is not a regression. - Hidden and native
valueserialization — open, posted as finding 2. Entered only through a caller-suppliedvalueordefaultValue, which the PR body scopes out. - Segmented typing, every granularity — refuted.
useSegmentedEntry.isAllowedgates every manual commit onisWithin(d, min, max)withmin: periodMin(useSegmentedEntry.ts:210), andperiodMinis now unconditionally the floor, so arrow stepping, digit entry, Enter and the blur re-parse all reject year ≤ 0 — including a date from a customparse. Covered by the new tests on DateInput, DateTimeInput and TimeInput; 615 tests across the six touched suites pass. isBlockedstill built from the rawmin(DateInputBase.tsx:432) — refuted as a gap. It is only ever consumed byisAllowed, which ANDs it with the flooredisWithin, so the weaker predicate changes no outcome: a month or year in year 0 is rejected by the bounds check beforeisBlockedcould matter.focusedDatedrifting below year 1 by some other route — refuted. It is clamped at mount and re-clamped whenlowerBoundormaxchange;moveFocusandmoveMonthFocusreturn at the floor; the year list starts at 1; a day cell takesonFocusonly for in-current-month cells; andcommitValuesets it from an already-gated value. The one exception is themaxbelowmincrossing in finding 1.- The TimeInput native
minkeeping the given time rather than the floor — refuted as a defect. The native time input carries no date, the divergence predates this change (amindated in year 0 never constrained a today-dated value either), and the TSDoc scopes its claim to "the wheels and typing", so it is not false about the native path. - Year-list cost at
yeargranularity — refuted.isPeriodUnselectableshort-circuits on the first in-range day, so the flooredmindoes not turn the 201-year list into a 73k-day walk. - Generated artefacts drifting — refuted by inspection: the three
bestax-mcp/data/componentsrows and the three API-pageminrows are the only copies of those descriptions in the tree, every table row intimeinput.mdis a uniform 456 characters so Prettier is satisfied, and nothing inskills/states a picker year range. - Coverage — refuted. The changed files sit at 100% lines and 99.3% branches, with only pre-existing partials (
DateInputBase.tsx:395,632,Calendar.tsx:794) uncovered; every new branch — bothfloorMinarms, bothminYeararms, bothprevDisabledarms, bothperiodMinarms — is exercised.
🏄 Clean little set wave, this one — one tiny helper paddles out and every picker drops in right behind it, no wipeouts for anybody living after year 1. The only thing still bobbing past the break is a
maxset before the dawn of time, and nobody is surfing that. Good to go.
A max before year 1 crosses the floor, and clampDate returned max when the bounds crossed, so clamping twice swapped between them. Under StrictMode the focus re-clamp runs twice on mount, so the calendar opened on December of year 0 and its header stepped back from there. When min is after max, clampDate now lands on min, so the focus stays at year 1. The day grid's header also turns off once the month on show holds min or max or lies past it, rather than only when it holds it, which picking a year from the list could get around. A value before year 1 has no shape in HTML's date strings, so the inline pickers' hidden form input now submits it empty, as a native date input would, rather than a malformed string such as 00-1-06-15. The value still shows as given and is never committed.
|
deep-review: verify Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on 4f6eb18. |
Preview DeploymentPreview URL: https://c89cd4a7.bestax.pages.dev |
There was a problem hiding this comment.
Deep review (verify) — 2 resolved · 0 open
| N | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Correctness | A max before year 1 still puts the calendar below the floor — verified fixed |
bulma-ui/src/form/_pickerInternals/Calendar.tsx:470 |
| 2 | 🔵 Advisory | Robustness | The floor reaches min but not the value the hidden form input submits — verified fixed |
bulma-ui/src/form/DateInputBase.tsx:538 |
Overall: This pass settled the two threads the earlier review left open and reviewed no commits. Both were answered by the PR author (admin) with a fix in 4f6eb18b plus a reasoned decision on the part left alone, and both check out against the current code: clampDate now caps against max then floors against min, so min wins on crossed bounds and clamping is idempotent; the day-grid header compares startOfMonth(focusedDate) to the bound instead of requiring isSameMonth, closing the year-list route past a bound as well as the crossed-bounds case; and toIsoValue/toIsoDateTime return an empty string below FIRST_YEAR, so an inline picker hidden input submits nothing rather than 00-1-06-15. The two declined halves — an empty year list when no year is in range, and not clamping a caller-supplied value on submit — are the deliberate calls the original advisories offered as settlements. Nothing is left for a human here.
Residual risk:
- Header reachable below the floor: refuted —
prevDisabled/nextDisabledare now purestartOfMonthcomparisons against the bound (Calendar.tsx:467-473), and the keyboard path already returned early onmonth < startOfMonth(min)(Calendar.tsx:403-405). The new Calendar cases are load-bearing: the Mar-2024-focus / Jun-2024-mincase wasfalseunder the oldisSameMonthguard. - Clamp swapping between crossed bounds: refuted —
clampDateis idempotent by construction anddateUtils.test.tspinsminwinning; the DateInput and DateTimeInput cases render underReact.StrictMode, which is what exposed the double re-clamp. - Malformed year serialized anywhere else: refuted for the paths in the diff —
toIsoValueandtoIsoDateTimeare the only producers of theYYYY-…shape, andlowerBoundis always floored, so the nativeminattribute still emits0001-01-01rather than an empty string. - All 577 tests across
Calendar,DateInput,DateTimeInput,TimeInputanddateUtilspass on4f6eb18b, and each of the six new assertions was confirmed to run by name rather than silently skip.
🏄 Author caught both advisories on the inside and rode them clean — the clamp no longer flip-flops between crossed bounds, and nothing malformed makes it onto the wire. All threads closed out, zero open, this one is good to go.
allxsmith
added
the
needs-human-review
📸 Story screenshots at handoff —
|
allxsmith
removed
the
needs-human-review
github-actions
Bot
added
the
review-converged
allxsmith
added
the
needs-human-review
github-actions
Bot
added
review-converged
|
🎉 This PR is included in version 5.26.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.25.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.2.13 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.14.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
HTML's date, month and datetime-local inputs hold no year below 1, but the date pickers accepted dates in year 0 and earlier. With no
min, the calendar's year list reached far enough below the focused year that a picker focused on a year below 100 listed negative years. Segmented typing, the defaultparseand the time wheels committed such dates, which the native inputs then couldn't show and theYYYYtoken rendered with a minus sign inside its padding.This bounds the pickers at year 1, as decided on the issue. A shared helper raises
minto midnight on 1 January of year 1 when it's earlier or absent, and each path applies it the way it already appliesmin. The calendar disables the days before year 1, keeps keyboard focus and its previous-month and previous-year buttons from going past it, and starts its default year list there. DateInput, DateTimeInput and TimeInput reject a typed, stepped or parsed date before year 1, including one from a customparse, and the time wheels leave a value dated before year 1 unchanged. An explicitminbefore year 1 counts as 1 January of year 1, including in the native date, month and datetime-local inputs'minattribute. TheminTSDoc on all three pickers says so, and the DateInput and DateTimeInput docs pages mention it under Min and Max.This is a narrow change to what the pickers accept: only year 0 and earlier move out of range. Years 1 to 99 stay in range as #890 made them, and its tests pass unchanged. The
YYYYtoken keeps padding years 1 to 999 to four digits, now covered by a test.Some things are left as they are. The native inputs' read-back needed no change: HTML's value format has no year 0 or negative year, so the browser empties such a value before the picker reads it, and a test covers that alongside year 1 reading back. A controlled
valuedated before year 1 still displays as given, the same way avaluebeforemindoes; the pickers just won't commit one. TimeInput's native time input takes only a time of day, so itsminattribute keeps the givenmin's time rather than the floor's.pnpm allpasses locally.Fixes #902