fix: make controlled month mode actually navigate#13
Open
dogmar wants to merge 1 commit into
Open
Conversation
In controlled mode (a `month` prop is set), the Prev/Next buttons did nothing and onMonthChange never fired: the old code relied on an effect watching `viewingYearMonth`, which derives from the `month` prop the parent owns, so navigation could never originate inside the component. With a stateful parent it was worse — `currentMonth` is rebuilt as a fresh object each render, so the `viewingYearMonth` memo produced a new reference every commit and the effect fired onMonthChange on every render, looping setMonth → re-render forever. Now: - Prev/Next buttons (goNext/goPrevMonth) notify the parent directly via onMonthChange with the adjacent month, then move focus. - A value-keyed skip ref stops the focus-sync effect from re-notifying for the button's own focus move (fires exactly once), without the strand-and-suppress hazard of a one-shot boolean flag. - Keyboard focus crossing into a non-visible month notifies the parent from the focus-sync effect (single source for that path). - The viewingYearMonth effect skips controlled mode entirely, which is what breaks the infinite loop. Adds 7 tests: single- and multi-month button nav, the stateful round-trip (would hang if the loop regressed), keyboard crossing in/out of the visible window, and the no-op-click-then-keyboard regression. Co-Authored-By: Claude Opus 4.8 <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
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
Audit fix B (2 of 5), follows #12. In controlled
monthmode the MonthView's Prev/Next buttons did nothing andonMonthChangenever fired — there was no way to drive a controlled month. With a stateful parent it was worse: an infinite render loop.Root cause
The old code only emitted
onMonthChangefrom an effect watchingviewingYearMonth, butviewingYearMonthderives from themonthprop — which only the parent can change — so navigation could never originate inside the component. And becausecurrentMonthis rebuilt as a fresh object every render, theviewingYearMonthuseMemoproduced a new reference each commit, firing the effect on every render →setMonth→ re-render → loop.Fix
goNextMonth/goPrevMonth(controlled) callonMonthChange(adjacentMonth)and then move focus.viewingYearMontheffect skips controlled mode, which is what actually breaks the loop.Tests (7 new, in a
controlled monthdescribe)Single-month next/prev (fire once, view stays until parent updates), multi-month button advance (notification must come from the button since the adjacent month is already visible), the stateful round-trip (would hang if the loop regressed), keyboard crossing in/out of the visible window, and the no-op-click-then-keyboard regression for the stranded-skip hazard.
Verification
vp run readyexits 0: lint+typecheck 0/0, package 672 tests, website 54.🤖 Generated with Claude Code