fix(SideNav): persist collapse state, not a zero width - #4824
Open
AKnassa wants to merge 3 commits into
Open
Conversation
Collapsing a resizable SideNav with resizable.autoSaveId wrote a plain 0 to localStorage. On reload the resize hook restored collapsed while SideNav's own collapse state stayed expanded, rendering an invisible zero-width nav with no way to recover (facebook#4790). useResizable now persists {size, isCollapsed} so the pre-collapse width survives a collapsed session, and SideNav seeds both its own collapse state and the hook (via the new initialIsCollapsed config) from the same entry, so the two can never disagree at mount. Legacy entries still load: a plain 0 restores as collapsed, so already-affected users recover without clearing storage; plain width entries keep the width but no longer override defaultIsCollapsed.
Edge-case hardening round for facebook#4790, test-first (34 new tests): - SideNav's toggle notified onCollapsedChange twice per click — once directly and once via the resize hook's onCollapseChange. The hook is now the single notifier when resizable; the direct call remains for non-resizable navs and as a fallback when the hook already matches. - useResizable's resize() out of the collapsed state now fires onCollapseChange(false), matching the drag path and the callback's documented contract. - loadPersistedState salvages an explicit isCollapsed: true from object entries whose size is unusable (0, negative, missing), mirroring the legacy plain-0 mapping — the region restores as a recoverable collapsed rail instead of expanded. - UseResizableSingleConfig gains regions?: never so multi-region configs held in variables resolve to the multi-region overload instead of silently matching the single-region one. New pins: multi-region persistence (per-region entries, migration, orphaned pre-multi entries), hostile storage (quota/security throws, SSR guard, wrong-typed payloads), snaps and percentage defaults interacting with persistence, StrictMode mount restore, and no-autoSaveId never touching storage.
|
@AKnassa is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
community
meta-cla
Bot
added
the
CLA Signed
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsResizable (@astryxdesign/core) · View in Storybook
SideNav (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
Self-review round on facebook#4790 (7 findings, all test-first): - useResizable read isCollapsed from the closure captured at the last render, so two imperative calls in one tick misreported: expand() then resize() fired onCollapseChange(false) twice, and collapse() then resize() reported only the collapse while ending expanded. A ref now mirrors the state, and expand/resize/onResizeMove read it. - Same staleness governed real drags: ResizeHandle registers its pointermove listener once at pointer down, so the whole gesture ran against one snapshot — every below-threshold move re-fired onCollapseChange(true), and dragging back above the threshold could not re-expand. Both now behave once and correctly, and the drag test replays a gesture through the pointer-down props like production does. - The "regions?: never" discriminant let an explicit "regions: undefined" type-check as a single-region config, which then reached Object.entries(undefined) — a compile-clean crash that was a compile error before the discriminant. The runtime branch now null-checks. - Toggling a resizable SideNav with collapse disabled left the imperative handle reporting a collapse that never happened, with no re-render to correct it; it is now a no-op. New pins: the hook-already-collapsed toggle branch (drag-collapse refused by a controlled parent, then click), the isCollapsible gate on the persisted seed, resize() staying silent while already expanded, and collapse() not re-notifying when already collapsed.
AKnassa
marked this pull request as ready for review
August 8, 2026 16:30
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
1 participant
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.
Refs #4790
What this does
Fixes the bug where a sidebar that can both resize and collapse would disappear after a page reload. Once collapsed and reloaded, the nav came back 0 pixels wide — invisible, with no way to bring it back.
Why
Collapsing saved "width: 0" to the browser's storage. On the next visit the sidebar treated that 0 as a real width, so it rendered invisible and the expand button vanished with it. People had to clear browser storage by hand to recover (full report in #4790).
What changed
useResizablegains aninitialIsCollapsedoption so a component that owns collapse state (like SideNav) can seed the hook and the two can never disagree at startup — the disagreement is what made the nav invisible.How to see it
SideNavwith bothcollapsibleandresizable={{ autoSaveId: '...' }}.Note for server-rendered apps: restoring a collapsed session can log a recoverable hydration notice on first paint (same family as the existing restored-width notice); details in the changeset.