Skip to content

fix(react): WeeklyDayPicker shows correct week number in collapsed view - #36725

Open
Sanjay Santhanam (Sanjays2402) wants to merge 1 commit into
microsoft:masterfrom
Sanjays2402:fix/weekly-day-picker-week-number
Open

Sanjay Santhanam (Sanjays2402) wants to merge 1 commit into
microsoft:masterfrom
Sanjays2402:fix/weekly-day-picker-week-number

Conversation

@Sanjays2402

Copy link
Copy Markdown

Fixes #36721.

What was wrong

CalendarGridRow rendered the week number as weekNumbers[weekIndex], where weekNumbers comes from getWeekNumbersInMonth(...) anchored on the 1st of the navigated month. In collapsed single-week views (weeksToShow=1, e.g. WeeklyDayPicker) only one row is visible, so weekIndex is always 0 and the row always showed the first week-of-month number instead of the visible week's number.

How I reproduced it

Built a day grid for navigatedDate = Fri Sep 11 2026 with dateRangeType=Week, weeksToShow=1, firstDayOfWeek=Sunday, firstWeekOfYear=FirstFullWeek and ran the row's logic against it: the visible row (Sep 6 - Sep 12) rendered 35 while the actual week number is 36.

The fix

CalendarGridRow now derives the week number from the row's own days — getWeekNumber of the last day of the week — instead of indexing into the month-anchored array. This is a 3-line change in the component plus a regression test.

Tests

  • Verified the new values are identical to the old ones in full-month view: 561 grid rows checked across 12 months x 3 firstDayOfWeek values x 3 firstWeekOfYear values, zero differences.
  • Verified the collapsed view now shows the correct week for every day of 2026 (365/365).
  • Ran the existing dateMath week-number assertions (getWeekNumbersInMonth / getWeekNumber, all four option groups from dateMath.test.ts): 4/4 pass.
  • Added a regression test in CalendarDayGrid.test.tsx that renders a collapsed single-week grid with showWeekNumbers and asserts the visible row shows 36 for Sep 11 2026 (it shows 35 without the fix).
  • Added the required beachball change file.

Note: the repo's jest setup needs a full yarn install, which isn't available in this environment, so the component test above was validated through the same logic against the repo's real date-time sources rather than a jest run.

In collapsed single-week views (weeksToShow=1, e.g. WeeklyDayPicker),
CalendarGridRow rendered weekNumbers[weekIndex] from getWeekNumbersInMonth,
which is anchored on the 1st of the navigated month. With only one visible
row, weekIndex is always 0, so the row showed the first week-of-month number
instead of the visible week's number (e.g. navigated to Fri Sep 11 2026 it
showed 35 while the visible week is 36).

Derive the week number from the row's own days
(getWeekNumber of the week's last day) instead. This is identical to the
previous values in full-month view (verified: 561 rows across 12 months x 3
firstDayOfWeek x 3 firstWeekOfYear settings, zero differences) and correct in
collapsed views (verified for every day of 2026).

Copilot AI 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.

🟡 Changes recommended

The test omits a required prop, and transition week headers expose hidden content to assistive technology.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes v8 collapsed calendar views by calculating week numbers from each rendered row.

Changes:

  • Derives week numbers from row dates.
  • Adds a collapsed-view regression test.
  • Adds a patch change file.
  • Merge confidence: 55/100 due to unresolved build, test, and accessibility issues.
File summaries
File Description
CalendarGridRow.tsx Calculates each row’s week number directly.
CalendarDayGrid.test.tsx Adds regression coverage for issue #36721.
Change file Records the v8 patch fix.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

weeksToShow={1}
showWeekNumbers={true}
firstDayOfWeek={DayOfWeek.Sunday}
firstWeekOfYear={FirstWeekOfYear.FirstFullWeek}
Comment on lines +38 to +42
const displayedWeekNumbers = Array.from(container.querySelectorAll('th[scope="row"] span')).map(
cell => cell.textContent,
);
// The visible row must show 36 (its own week), not 35 (the first week-of-month number).
expect(displayedWeekNumbers).toContain('36');
Comment on lines +57 to 58
{showWeekNumbers && weekNumber !== null && (
<th
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: WeeklyDayPicker showing incorrect week number when navigating to different weeks

2 participants