Resizable: pin #5934's legacy maxSizePx shrink-after-drag clamp - #6041
Conversation
The table-inbox reading pane (#5934) computes maxSizePx itself from a plain ResizeObserver measurement (Math.max(paneFloor, surfaceWidth - listFloor)) rather than a containerRef percentage bound. A same-head review on #5934 found that after widening the pane past a later surface-derived maximum, the pane stayed oversized and overlapped the list instead of clamping down to the new ceiling. Investigation shows this exact literal-maxSizePx-shrink path is already covered by useResizable's general FR4 bounds-reclamp (landed in #5783, commit 311deef): re-resolving resolvedMax from a changed caller-supplied maxSizePx every render already re-clamps the committed selection, independent of whether the bound is a percentage or a plain number. Both a hook-level and a page.tsx-composition-level reproduction of the #5934 review's exact sequence (drag to 1066px, then narrow the derived max to 834px) already pass on current main. Add the regression directly to useResizable's FR4/FR6 suite so this caller-recomputed-pixel path can't silently regress again: one test pins the clamp itself and the ARIA-visible _maxSizePx/_size, the other pins that an in-range user choice is left untouched (AST-010 FR4: bounds clamp, they never scale). No runtime change: the fix is already on main.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
cixzhang
left a comment
cixzhang
left a comment
There was a problem hiding this comment.
Semantic verdict: request changes
The regression test still lets the old pane width revive after the surface grows again, so the reading pane can jump without another drag. I limited committed-bound updates to percentage bounds and all 112 tests still passed. Please grow maxSizePx back to 1500 after the 834 clamp and assert the size stays 834; that pins the committed selection this PR says it protects.
[Reviewed by Robohands]
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsResizable (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
1 participant
What
Test-only closure for a same-head review finding on #5934 (
table-inbox).The reading pane in the table-inbox template derives
maxSizePxitself —Math.max(paneFloor, surfaceWidth - listFloor)from a plain ResizeObserver measurement, not acontainerRefpercentage bound. A dismissed same-head review on #5934 found that after widening the pane past a later surface-derived maximum, the pane stayed oversized and overlapped the list instead of clamping to the new ceiling.Investigation
Reproducing the review's exact sequence — drag the pane to 1066px, then narrow the surface so the derived max drops to 834px — at both the
useResizablehook level and a page.tsx-composition-level replica shows it already clamps correctly on currentmain.useResizable's general FR4 bounds-reclamp (landed in #5783, commit311deefc) re-resolvesresolvedMaxfrom a changed caller-suppliedmaxSizePxon every render and re-clamps the committed selection — independent of whether the bound is a percentage or a plain number. No template or hook behavior change is needed.The change
Adds the reproduction directly to
useResizable's FR4/FR6 suite (AST-010) so this caller-recomputed-pixel path — distinct from the existing containerRef/percentage-basis tests already there — can't silently regress:_maxSizePx/_sizewhen a legacy numericmaxSizePxshrinks below the user's chosen size;Risk
None — test-only, no runtime change.
Testing
vitest run packages/core/src/Resizable/— 150/150 passing (112 inuseResizable.test.ts, incl. the 2 new tests)tsc --noEmit,eslinton the changed file — cleanpnpm check:repo— cleanRefs #5934, #5783. AST-010 FR4/FR6.