Skip to content

fix(DSTSUP-276): anchor the Calendar year list at year 1 - #5766

Open
aromko wants to merge 4 commits into
mainfrom
dstsup-276_year-list-era-boundary
Open

fix(DSTSUP-276): anchor the Calendar year list at year 1#5766
aromko wants to merge 4 commits into
mainfrom
dstsup-276_year-list-era-boundary

Conversation

@aromko

@aromko aromko commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

Opening the year dropdown on a year below 21 produced a list that counted down and then back up — focused on AD 5 it read 16 15 14 … 2 1 1 2 3 … 25, with two options sharing the accessible name 1.

CalendarYearPicker centres a fixed visibleYears window on the focused year, so a 41-year window reaches 20 years back. There is no year 0 in the Gregorian calendar, so from AD 5 that lands in 16 BC, and react-aria renders an era marker only when the focused date is BC, never the individual entries. The first 16 options were BC years wearing bare AD numbers.

The window is now anchored at year 1 rather than centred whenever the focused year sits too close to the boundary to centre it. The list stays 41 entries long: AD 5 offers 1 … 41, as does AD 1. Since react-aria only shifts its window for minValue, which this component does not own, the window is instead sized to guarantee 41 in-era years and the entries that rolled into the previous era are dropped.

minValue calendars are untouched: react-aria re-anchors its own window there, and a BC minValue is an explicit opt-in, so neither the anchor nor the era filter applies. A maxValue-only calendar focused below AD 21 does change — the clamp drags the window back across the era boundary and those entries are dropped the same way, so maxValue AD 10 focused on AD 5 lists 1 … 10 instead of 31 BC … AD 10. Every other bounded calendar renders exactly as before: the window still reaches whichever bound is farther and renders every in-range year. Verified against every bound combination (min only, max only, both, wide range, single-year range, BC bounds) before and after the change.

Closes DSTSUP-276

Screenshots / Preview

Before After

Test Instructions

  1. Open Storybook → Components/Calendar/Basic and set defaultValue to a year below 21, e.g. new CalendarDate(5, 6, 15).
  2. Open the year dropdown. It should list 1 through 41 ascending — no duplicate 1, no descending run at the top.
  3. Repeat with minValue and/or maxValue set and confirm those lists are unchanged.
  4. pnpm test:sb — the new Anchors the year list at year 1 instead of rolling into the previous era story test covers step 2.

Breaking Changes

No

Checklist

  • Storybook preview and Marigold docs preview are available
  • Stories added/updated (with component-test tag where applicable)
  • Unit tests added/updated
  • Component documentation added/updated (if it exists)
  • Accessibility reviewed against ARIA APG (for new/changed interactive components)
  • Visual regression tests updated (for UI changes)
  • Changeset added (pnpm changeset)

The year dropdown centres a 41-year window on the focused year, so below
year 21 it reached past the Gregorian era boundary. react-aria labels
entries with an era only when the focused date is BC, so focused on AD 5
the list read "16 15 … 2 1 1 2 … 25" with two options named "1".

Anchor the window at year 1 when the focused year is too low to centre
it, keeping 41 entries. Bounded calendars are unchanged.
@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marigold-docs Ready Ready Preview Aug 31, 2026 7:26am
marigold-storybook Ready Ready Preview Aug 31, 2026 7:26am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
marigold-production Ignored Ignored Aug 31, 2026 7:26am

Request Review

@changeset-bot

changeset-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7432c0e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@marigold/components Patch
@marigold/docs Patch
@marigold/theme-rui Patch
@marigold/system Patch
@marigold/icons Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the type:feature New feature or component label Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Accessibility tests executed. Download the report here.

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Marigold Code Coverage

Status Category Percentage Covered / Total
🔵 Lines 98.81% 2993 / 3029
🔵 Statements 97.81% 3138 / 3208
🔵 Functions 97.97% 823 / 840
🔵 Branches 91.26% 1943 / 2129
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/components/src/Calendar/YearListBox.tsx 100% 100% 100% 100%
Generated in workflow #23754 for commit 7432c0e by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Accessibility tests executed. Download the report here.

@jim761 jim761 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Four notes on the year-window change. The core logic checks out — I traced useCalendarYearPicker in react-aria@3.51.0 and confirmed the window math (focused AD 5 → visibleYears: 731 … 41 after the filter), and that filtering items is safe for selection because RAC's id is an index into the unfiltered array that onChange resolves by lookup. One warning and three nits below.


Generated with Claude Code review-pr skill

Comment thread packages/components/src/Calendar/YearListBox.tsx Outdated
Comment thread packages/components/src/Calendar/YearListBox.tsx Outdated
Comment thread .changeset/dstsup-276_year-list-era-boundary.md Outdated
Comment thread .changeset/dstsup-276_year-list-era-boundary.md Outdated
…lendars

The year-1 anchor and the era filter both assumed a Gregorian era floor.
`focusedDate.year` is era-relative in other calendar systems, so under the
Japanese calendar Reiwa 8 clamped to Reiwa 1, and the filter confined the
dropdown to the focused era — making pre-2019 unreachable. Both now gate on
`calendar.identifier === 'gregory'`, restoring the previous centred, era-
crossing window everywhere else.

The anchor is also skipped when `minValue` is set, since react-aria already
re-anchors the window there. `minValue: AD 1` with a focused AD 5 listed
`1 … 73` instead of `1 … 41`.

Extracts `YEARS_AROUND_FOCUS` so the 20/40 coupling that keeps the list 41
entries long is explicit, and mirrors the anchor story test in
`RangeCalendar`, which already mirrors the min/max clamp test.
@github-actions

Copy link
Copy Markdown
Contributor

Accessibility tests executed. Download the report here.

Comment thread packages/components/src/Calendar/YearListBox.tsx Outdated
Comment thread .changeset/dstsup-276_year-list-era-boundary.md Outdated
Comment thread packages/components/src/Calendar/Calendar.stories.tsx
The anchor was gated on `gregory && !minValue` while the era filter was
gated on `gregory` alone, so a BC `minValue` lost the 16 in-range BC years
the consumer had explicitly opted into. Both now share one flag.

Adds a selection round-trip test pinning that `item.id` indexes react-aria's
unfiltered year array, and a Japanese-calendar test pinning that non-Gregorian
windows stay centred.
@github-actions

Copy link
Copy Markdown
Contributor

Accessibility tests executed. Download the report here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature or component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants