Repository navigation
feat(Resizable): a resizable LayoutPanel slides closed and open, and focus leaves a collapsing panel - #9300
feat(Resizable): a resizable LayoutPanel slides closed and open, and focus leaves a collapsing panel#9300vjeux wants to merge 57 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:spec-owner-review
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
The slide and the focus move match the current component:Resizable FR8 and component:LayoutPanel FR7 and AR2 records, but approval waits on one spec decision and three fixes. The head rewords component:Resizable FR3 so that createResizable is the owner ResizeHandle goes through, which no current record states. An engineering owner needs to approve or reject that wording by commenting /approve-spec with the full head SHA; the author may do this if they are an engineering owner. Independently of that decision, please address:
- An end
LayoutPanelreversed by<ResizeHandle isReversed>whose region starts collapsed is neitherinertnoraria-hiddenuntil the factory's next update, because the factory looks for the pane before the handle reports its reversal (findPaneandconfigureincreateResizable.ts). - Move focus with the shared
focusForLastInput(utils/focusReturn.ts) instead of a raw.focus()inLayoutPanel.tsx. - The diff carries about 8,000 authored lines from three stacked changes; land the lower pull requests first so this one carries only its own commits.
[Automated review]
| const binding: ResizeHandleBinding = { | ||
| gesture, | ||
| read: () => view as ResizeHandleView, | ||
| configure(config) { |
There was a problem hiding this comment.
configure only refreshes the view. The factory's first commit() already ran findPane() with isReversed() false, so an end panel reversed by <ResizeHandle isReversed> is not found, and is not held collapsed or made inert, until the next commit. A region that starts collapsed leaves that panel reachable by Tab (component:LayoutPanel AR2). Re-finding the pane here when reversal changes (for example by calling commit()) fixes it.
There was a problem hiding this comment.
Fixed in ff985c2: a ResizeHandle's own isReversed makes the factory look for its pane on the other side, so an end panel whose region starts collapsed is inert from its first commit. A test covers it.
| const element = elementRef.current; | ||
| const active = element?.ownerDocument.activeElement; | ||
| if (element != null && active != null && element.contains(active)) { | ||
| (paneLink.focusTarget() ?? resizeHandleBeside(element, area))?.focus(); |
There was a problem hiding this comment.
This programmatic move bypasses focusForLastInput (utils/focusReturn.ts), which owns focus moves under architecture:interaction-modality INV1 and architecture:react-component-runtime INV16. Calling it here keeps this path on the shared pointer handling.
There was a problem hiding this comment.
Fixed in ff985c2: the move goes through moveFocusLeftBehind, which moves focus with focusForLastInput.
8c85319 to
93f3dfa
Compare
There was a problem hiding this comment.
This pull request isn't ready to approve: it waits on one spec decision, and three fixes are needed regardless. The head rewords component:Resizable FR3 so that createResizable is the owner ResizeHandle and useResizable go through, which no current record states; an engineering owner needs to approve or reject that wording by commenting /approve-spec with the full head SHA, and the author may do so if they are an engineering owner. Independently of that decision, please address:
- Keep a region's size and collapse state across an
<Activity mode="hidden">hide and show: bothuseResizableforms now come back at their defaults because the layout-effect cleanup destroys the factory and the next one starts from the options alone (useResizable.ts), where the base kept both. - Replace ResizeHandle's two ref-guarded warning effects with the shared
useDevWarninghook. - Land #9297 first and retarget so this pull request carries only its own slide and focus commits; the diff is about 7,400 authored lines today.
[Automated review]
| }); | ||
| useIsomorphicLayoutEffect( | ||
| () => () => { | ||
| slot.get()?.resizable.destroy(); |
There was a problem hiding this comment.
This cleanup destroys the factory that holds the region's size and collapse state. When <Activity mode="hidden"> hides the tree and shows it again, the create effect above builds a new factory from the options alone, so the region comes back at defaultSize and defaultIsCollapsed; the base's configuration-object form kept both in React state. Carrying readResizableSelection(...) across this cleanup, as the element-swap path does, would keep them.
There was a problem hiding this comment.
Fixed in #9297: the factory keeps the region while an <Activity> hides its component, so the region comes back at its size and collapse state.
| if (isDisabled || !resizable || !resizable._collapsible) { | ||
| return; | ||
| }, [direction, regionDirection]); | ||
| useEffect(() => { |
There was a problem hiding this comment.
This ref-guarded useEffect + devWarn (and the axis warning above it) is what useDevWarning(component, message, condition) in hooks/useDevWarning.ts already does once per mount. Calling the shared hook keeps one path for dev warnings.
93f3dfa to
3522fbf
Compare
There was a problem hiding this comment.
This pull request isn't ready to approve: it waits on one spec decision, and several fixes are needed regardless. The head rewords component:Resizable FR3 so that createResizable is the owner ResizeHandle and useResizable go through, which no current record states; an engineering owner needs to approve or reject that wording by commenting /approve-spec with the full head SHA, and the author may do so as an engineering owner. Independently of that decision, please address:
- Route LayoutPanel's focus move through
useFocusLeavingChangeandmoveFocusLeftBehind, whichmainnow uses for SideNav and Collapsible, instead of a second layout effect. - Move focus out of, and make inert, a
LayoutPanelsized bywidth={region.size}from the configuration-object form, which still keeps focus when it collapses (component:LayoutPanelAR2). - Replace ResizeHandle's remaining ref-guarded axis warning with
useDevWarning. - Land the stacked #9297, #9370 and #9372 first and retarget, so this pull request carries only its own commits; the diff is about 7,800 authored lines today.
[Automated review]
| if (!isRegionCollapsed && isSettledCollapsed) { | ||
| setIsSettledCollapsed(false); | ||
| } | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
This effect re-implements the focus-leaving sequence that main now owns in useFocusLeavingChange (utils/useFocusLeavingChange.ts) with noteFocusWithin and moveFocusLeftBehind (utils/focusReturn.ts), which SideNav and Collapsible call from their LayoutChangeSnapshot. architecture:react-component-runtime INV16 asks for one call to the shared owner and no component-level effect for the concern. After rebasing, note focus before the collapse with useFocusLeavingChange, move it with moveFocusLeftBehind(note, paneLink.focusTarget() ?? resizeHandleBeside(panel, area), panel), and keep the settle state only for inert.
There was a problem hiding this comment.
Fixed in ff985c2: LayoutPanel notes focus with useFocusLeavingChange, keyed on the region's collapse state, and moves it with moveFocusLeftBehind. The settle state is only for inert.
| paneLink.isCollapsed, | ||
| returnFalse, | ||
| ); | ||
| const isFoundBeside = resizable == null; |
There was a problem hiding this comment.
A panel given width={region.size} from the configuration-object form, beside <ResizeHandle resizable={region.props}> (the shape of the HorizontalSplit story), lends itself here, but no controller ever finds it: the configuration-object factory has no handle, and this panel has no resizable prop. Collapsing it from a button inside leaves focus there, and the panel is neither inert nor aria-hidden, which component:LayoutPanel AR2 asks for under any resize controller. The ResizeHandle given resizable props could tell the panel beside it the region's collapse state for the focus move and inert only, with no slide.
There was a problem hiding this comment.
Fixed in ff985c2: a ResizeHandle given resizable props holds the LayoutPanel beside it collapsed, so a panel at width={region.size} moves focus to the handle and goes inert. A test fails before the fix.
| hookDirection !== direction && | ||
| !didWarnAxisRef.current | ||
| ) { | ||
| if (didWarnAxisRef.current) { |
There was a problem hiding this comment.
This ref-guarded effect is what useDevWarning(component, message, condition) already does, and the call just below uses it. useDevWarning('ResizeHandle', <the same message>, regionDirection != null && regionDirection !== direction) replaces didWarnAxisRef and the effect.
3522fbf to
c5b5bca
Compare
There was a problem hiding this comment.
This pull request isn't ready to approve: it waits on one spec decision, and several fixes are needed regardless. The head rewords component:Resizable FR3 so that createResizable is the owner ResizeHandle and useResizable go through; an engineering owner needs to approve or reject that wording by commenting /approve-spec <full head SHA> (the author may do so as an engineering owner), or FR3 keeps its current wording here. Independently of that decision:
- Move LayoutPanel's focus through
main'suseFocusLeavingChangeandmoveFocusLeftBehindinstead of its own layout effect (LayoutPanel.tsx:302). - A
LayoutPanelgivenwidth={region.size}from the configuration-object form, beside<ResizeHandle resizable={region.props}>, still keeps focus and stays reachable when it collapses, whichcomponent:LayoutPanelAR2 covers; fix it or narrow the[fix]changeset. - At 7,963 authored lines the diff carries all of #9297, #9370 and #9372; land those first so this pull request carries only its own commits.
- Replace ResizeHandle's ref-guarded axis-warning effect with
useDevWarning(ResizeHandle.tsx:557).
[Automated review]
| if (!isRegionCollapsed && isSettledCollapsed) { | ||
| setIsSettledCollapsed(false); | ||
| } | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
This effect re-implements the focus-leaving sequence main owns in useFocusLeavingChange (utils/useFocusLeavingChange.ts) with noteFocusWithin and moveFocusLeftBehind (utils/focusReturn.ts), which SideNav and Collapsible call from their LayoutChangeSnapshot's onBeforeChange (architecture:react-component-runtime INV16). After rebasing, note focus with useFocusLeavingChange keyed on the region's collapse state and move it with moveFocusLeftBehind(note, paneLink.focusTarget() ?? resizeHandleBeside(panel, area), panel); keep the settle state only for inert.
There was a problem hiding this comment.
Fixed in ff985c2: LayoutPanel notes focus with useFocusLeavingChange, keyed on the region's collapse state, and moves it with moveFocusLeftBehind. The settle state is only for inert.
| paneLink.isCollapsed, | ||
| returnFalse, | ||
| ); | ||
| const isFoundBeside = resizable == null; |
There was a problem hiding this comment.
A panel given width={region.size} from the configuration-object form, beside <ResizeHandle resizable={region.props}> (the Collapsible story's shape), lends itself here, but the configuration-object factory has no handle and never looks for it (createResizable.ts:832). It keeps focus and stays reachable when it collapses, which component:LayoutPanel AR2 covers ("the configuration-object form (FR7) included").
There was a problem hiding this comment.
Fixed in ff985c2: a ResizeHandle given resizable props holds the LayoutPanel beside it collapsed, so a panel at width={region.size} moves focus to the handle and goes inert. A test fails before the fix.
| // means there is no region axis to compare, not that it is horizontal. | ||
| const didWarnAxisRef = useRef(false); | ||
| const regionDirection = resizable?._direction ?? view?.direction; | ||
| useEffect(() => { |
There was a problem hiding this comment.
This ref-guarded effect is what useDevWarning(component, message, condition) already does, and the call just below uses it: useDevWarning('ResizeHandle', <the same message>, regionDirection != null && regionDirection !== direction), deleting didWarnAxisRef and this effect.
c5b5bca to
ff985c2
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
This pull request isn't ready to approve: it waits on two spec decisions, and three fixes are needed regardless.
Spec decisions. An engineering owner records each by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner.
- The head rewords
component:ResizableFR3 so thatcreateResizableowns the drag, keyboard, collapse, snapping, persistence and size behavior that ResizeHandle anduseResizablego through; no current record says ResizeHandle is built on the factory, so either that wording is approved or FR3 keeps its current text. - The Grab zone now stays centred on the divider whatever
pillPlacementsays, shrinks to half of a pane shorter than--spacing-4, and lets the painted pill take the pointer, none of which current FR1 states.
Required changes:
ResizeHandle.tsx:617–644copiescreateResizable's find-and-hold-the-pane sequence (createResizable.ts:860–881and:1200–1207), so move it into one helper both call and subscribe to pane changes only while the handle has a region.- The Grab zone clamp contradicts FR1's
--spacing-4strip on short panes, so split the Grab zone change into its own pull request with an FR1 amendment, or drop it here. - At 9,917 authored lines (7,519 from the stack below and 2,435 of this pull request's own), the diff is far over the 2,000-line scope limit, so land the stack first and retarget this pull request to its own commits.
The slide, the focus move and the new factory otherwise match component:Resizable FR8, component:LayoutPanel FR7 and AR2, and the public API record. Non-blocking: two changesets still call the factory internal, and ResizablePaneLink.render is never called.
[Automated review]
| const heldPaneRef = useRef<ResizablePane | null>(null); | ||
| const isRegionCollapsed = resizable?._isCollapsed ?? false; | ||
| useEffect(() => watchResizablePanes(rerender), []); | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
This repeats createResizable's findPane/setPane (createResizable.ts:860–881) and its pane watcher (:1200–1207): find the lent panel beside the handle, release the old one, hold the new one with the region's collapse state. Two copies already read reversal differently. Could both call one helper in resizablePane.ts (or could a handle given the hook's own props hand its element to the region, so the factory's findPane serves it)? The watcher on :625 also runs for every ResizeHandle, region or not.
There was a problem hiding this comment.
Done in e210af4: ResizeHandle and createResizable find the pane with resizablePaneFor and move their hold with handOverPane. A ResizeHandle watches for lent panes only while it has a region.
| // An inline Handle's zone is centred on its line, so half of it reaches into | ||
| // the pane. On a pane shorter than the zone, that is at most half the pane. | ||
| hitAreaLengthX: (pane: number) => ({ | ||
| width: `min(${spacingVars['--spacing-4']}, ${pane}px)`, |
There was a problem hiding this comment.
component:Resizable FR1 states the Grab zone is "a --spacing-4 strip on the resize axis"; this caps it at the pane's length, so a 12px item gets a 12px zone. The centring, this clamp and the pill as a pointer target need an FR1 amendment an engineering owner approves; they are separable from the slide, so they could move to their own pull request.
There was a problem hiding this comment.
FR1 states it now: the owner-approved amendment in #9529, which this carries.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] The configuration-object `useResizable` builds on one internal resize region, through the same size model as before, so the region's state has one implementation. |
There was a problem hiding this comment.
Non-blocking: this and create-resizable-plain-handle.md call the factory internal, while create-resizable.md announces it as public in the same release. Folding them into the public entry keeps the release notes consistent.
ff985c2 to
44ba216
Compare
…and useResizable go through component:Resizable FR3 owned the drag, keys, double-click, snapping, persistence, reversal and size in ResizeHandle and useResizable. Both forms of the hook and both paths of the handle go through createResizable, now public, so FR3 names it as the owner they go through (architecture:public-component-api INV22).
…abel The doc page lists options.collapsedLabel, the text a collapsed handle announces in place of the catalog's "Collapsed", and its guidance says the collapsed text outside a provider is the shipped English one unless collapsedLabel is passed. The changeset says label names the handle and collapsedLabel replaces its collapsed text.
…, with no record citations Each comment sentence the public export adds has at most 20 words, the label option's JSDoc cites no record, and the browser proof's header fits in about 12 lines. No code change.
…one is centred on the Handle The ownership line called the Grab zone "aligned", which reads as aligned with the pill. FR1 now centres the zone on the Handle for every pillPlacement, so the line says that.
… internal ones Two changesets called the factory internal while this one announces it as public in the same release. They fold into it: the configuration-object useResizable and ResizeHandle build on the factory, with no behavior change.
…are listed separately
… region at the end The isReversed option's description said the handle sits after the region it resizes when the region is at the end. It sits before it, as the option's JSDoc says.
…asured before the DOM changes, and focus leaves a collapsing pane (component:Resizable FR8; component:LayoutPanel FR7, AR2) Red: nothing plays and no focus moves. A LayoutPanel at a ref-form region's size, or given the configuration-object hook's own props, is measured before the DOM changes and played from the edge facing its handle once it renders its new size, for every collapse and open (collapse(), expand(), a resize out of collapse, Enter, a double-click, the grow arrow, a controlled change, a drag past the threshold); a controlled change is measured in the commit that renders it, which the hook reports in the commit of the parent's state. A drag that reopens, a plain resize and the first render play nothing, an unmount lands a running motion, and a LayoutPanel that replaces another reaches its region through its callback ref. A hook-only pane and a hand-built ResizableProps object show no reveal. Focus inside a collapsing pane moves to its ResizeHandle and the collapsed pane is inert and aria-hidden, in every form; a collapse from the handle or from outside the pane keeps focus.
…d before the DOM changes, and focus leaves a collapsing pane (component:Resizable FR8; component:LayoutPanel FR7, AR2) A LayoutPanel sized by a resize controller lends it its element in its callback ref, through an internal channel (resizablePane.ts), with no new prop: a ref-form factory finds the lent panel beside its handle, and the configuration object's region gets the pane its LayoutPanel hands over, through the hook's own props (a hand-built ResizableProps object has no region and never slides). The factory decides each collapse and open as it makes it (a drag that reopens plays nothing; a key step plays), and LayoutPanel tells it of the change in the commit that renders it, before React changes the DOM (LayoutChangeSnapshot), where the pane is measured; it plays spec:AST-105 FR9's edge reveal from the edge facing the handle once the pane renders its new size, on the factory's own animated-layout handle over the pane's row. A controlled isCollapsed, which both forms report in the commit of the parent's state, is measured the same way before the factory hears of it. A resize that neither collapses nor opens, a pointer reopen, the pane leaving and destroy land a running motion. The animated-layout boundary check lists the controller's edge reveal among the module's users. Focus inside a collapsing panel moves to its ResizeHandle, then a commit later the panel goes inert and aria-hidden until it opens, in every form; LayoutPanel owns both, as family:layout-regions says.
…docs say a pane slides and focus leaves it; changesets for both LayoutPanel's width and resizable entries, useResizable's handle ref and createResizable's practices say that a LayoutPanel beside its ResizeHandle at the region's size slides closed and open, that focus inside it moves to the handle as it collapses, and that it is inert while collapsed; the deprecated props give no slide. A [feat] changeset covers the slide and a [fix] changeset the focus move, in both forms.
…ject does not LayoutPanel's resizable entry and the [feat] changeset say the configuration-object hook's own props slide the panel like the ref form and a hand-built ResizableProps object does not; the [fix] changeset's focus move and inert state hold for both.
…le on every frame, in Chromium and WebKit (component:Resizable FR8; component:LayoutPanel FR7, AR2) Two stories render a LayoutPanel at a useResizable(handleRef) region's size beside its ResizeHandle, at the start and at the end of a Layout. The browser proof checks the pane, handle and content are present and painted, then collapses the pane from a button inside it: the handle travels over several frames, the pane's shown edge stays at its resting offset from the handle on every frame, the pane ends at its collapsed layout, focus is on the handle, and the pane is inert and aria-hidden. Enter on the handle opens it the same way, with no focus move. Under Reduce Motion nothing travels and focus still leaves the pane. Without the controller's reveal the slide cases fail. Each element is read on its own animation's clock: WebKit reports a compositor-run translate about a millisecond ahead of the frame time (up to about 1.6 px at the fastest part of a collapse) while the pane's clip runs on the main thread at the frame time, and both share one start time and sample grid; in Chromium this equals the rect. The shipped-axis guard pairs a handle with its region by the ref it holds, as well as by the resizable prop, so the ref-form stories pair.
…e browser; a hand-built object collapses without a slide (component:Resizable FR8; component:LayoutPanel FR7, AR2) Two browser fixtures, kept out of the docs and the visual matrix, render a start LayoutPanel with the configuration-object hook's own props and with a hand-built copy of them. The first passes the same frame proof as the ref form; the second collapses and opens with no travelling frame, and focus still moves to the handle and the panel goes inert. The proof now places the handle at its layout place, where the pane's margin box ends in the row, plus its glide's keyframes at the glide's own time, instead of reading the handle's rect: WebKit reports a compositor-run translate ahead of the frame time in the rect and the computed style alike, by up to about 1.8 px. The pane moves by no transform, and its clip runs on the main thread at the frame time.
…paused motion, so a starved renderer cannot fail it Under load the proof failed: it sampled the frames the renderer drew, and with 16 WebKit workers on 3 CPUs (and once with 4 Chromium workers on 2 cores) it drew 1 to 5 frames over a whole collapse, so its "a slide, not a jump" check failed although the motion itself runs on its own clock. The proof now pauses every animation the collapse or open starts the moment it is created, and holds it there (the module starts each one on the timeline's current time by setting its start time, which would set it running), so none advances while the test waits for a starved page. Then, in one task, it sets each to the same one of 24 evenly spaced moments before the end and reads the pane and its handle there, finishes them, and checks what they leave. A slide is the pane's clip animation and the handle's glide, with the handle on its way at a third of the moments or more and the pane's shown edge on the handle at every moment; a hand-built object and Reduce Motion start no glide. The frame reader is installed once in the page and read the same way at rest and at every moment.
…led isCollapsed, a double-click, the grow arrow and a drag past the threshold (component:Resizable FR8; component:LayoutPanel FR7) The sliding stories gain a toggle outside the panel, and a story whose parent owns isCollapsed through onCollapseChange with its own toggle. Each path proves the pane clips and the handle glides with the pane's shown edge on the handle at every captured moment: the toggle (which keeps its focus while the panel goes inert), a controlled change both ways, a double-click on the handle, the grow arrow from collapsed, and a drag past the collapse threshold. A drag that reopens the panel follows the pointer with no clip and no glide. Without the panel telling its controller of the change before the DOM changes, every slide case fails and the hand-built and Reduce Motion cases still pass.
…om its first commit, and focus moves to the handle with the last input's ring (component:LayoutPanel AR2; architecture:interaction-modality INV1) Red: an end LayoutPanel whose region starts collapsed, beside a ResizeHandle that reverses it through its own isReversed prop, is neither inert nor aria-hidden after the first commit, because the factory looked for its pane before the handle reported its reversal. And focus moved out of a collapsing panel by a raw focus(), which shows the ring after pointer input too.
…once, and focus moves to the handle with the last input's ring (component:LayoutPanel AR2; architecture:interaction-modality INV1) When a ResizeHandle reports its own isReversed after the factory's first commit, the factory looks for its pane again on the other side of the handle and tells it whether the region is collapsed, so an end panel whose region starts collapsed is inert and aria-hidden from its first commit. LayoutPanel moves focus out of a collapsing panel with the shared focusForLastInput, and tracks the interaction modality of its document, so the handle shows its ring after keyboard input and not after pointer input.
…le given the configuration-object props, gives up focus and goes inert as the region collapses
A panel given width={region.size} from the configuration-object form,
beside <ResizeHandle resizable={region.props}>, moves focus inside it to
the handle as the region collapses, and is inert and hidden from
assistive technology until it opens, from its first commit when the
region starts collapsed. It does not slide.
Both fail here: no controller finds that panel, so it keeps focus and
stays reachable.
…el beside it collapsed with the region
A LayoutPanel at width={region.size}, with no resizable prop of its own,
is lent beside its handle, but the configuration-object form's factory
has no handle and never looks for it. The ResizeHandle that takes the
region's props now finds that panel beside it, tells it whether the
region is collapsed, and names itself as the panel's focus target. Focus
inside the panel then moves to the handle as it collapses, and the panel
is inert and hidden from assistive technology while collapsed, as under
the other controllers. A panel lent after the handle mounts is found at
once.
…ared focus-leaving sequence LayoutPanel noted and moved focus in its own layout effect. It now goes through the one sequence that SideNav and Collapsible use: useFocusLeavingChange, keyed on the region's collapse state, notes focus with noteFocusWithin in the commit before the DOM changes, and moveFocusLeftBehind moves it to the panel's ResizeHandle once the collapse commits, with the ring that the last input calls for. The settle state now only delays inert to the next commit. The behavior and its tests are unchanged.
… list the slide and the focus move as missing The pane slides and focus leaves a collapsing panel, so the records drop what said otherwise: Resizable FR8's implementation-gap sentence and its Current-state line, and LayoutPanel FR7's and AR2's implementation-gap sentences and their Current-state line. FR8's and FR7/AR2's verification rows name the browser spec in place of "pending", and FR8's row names the unit test for a hook-only pane.
…d createResizable, and a handle watches for panes only with a region ResizeHandle repeated createResizable's find-and-hold sequence for the LayoutPanel beside it. Both now find the pane with resizablePaneFor, by reversal, and move their hold with handOverPane: a pane the region no longer sizes stops listening and opens, and the held one takes the region's collapse state. A ResizeHandle subscribes to lent panes only while it has a resizable region. ResizablePaneLink.render, which nothing called, goes.
…ook's first layout effect still slides its pane A collapse() from a child's layout effect runs before the hook's own. The factory it started never followed the region a LayoutPanel lends, so a later expand() played no slide.
…a LayoutPanel lends An early collapse(), expand() or resize() started the factory without following the region, so its pane never slid. The commit's start and an early call's start now go through one helper. It names the factory's messages, selects a carried size, and follows the region.
…ndle, and finds it when it resumes useResizable suspends its factory on unmount, and suspend kept the pane watcher that only destroy removes. So an unmounted region stayed in the module's watcher set and took a panel lent beside its removed handle.
…nd resume watches again suspend lets go of the page, the pane watcher included. A suspended region hears no pane lend, so a running reveal lands as it suspends. resume starts watching again before it commits, which looks for the pane. An unmounted useResizable(handleRef) region no longer answers later panel lends.
cc63a8a to
1e97286
Compare
There was a problem hiding this comment.
This pull request isn't ready to approve: it waits on one spec decision, and one change is needed regardless.
Spec decision. The head rewords component:Resizable FR3 so that createResizable owns the pointer, keyboard, collapse, snapping, persistence, reversal and size behavior that ResizeHandle and useResizable go through, where the current record says ResizeHandle and useResizable own it. An engineering owner needs to approve or reject that wording by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; without approval, FR3 should keep its current wording here.
Required change:
- The branch carries 8,603 authored lines, 6,103 of them from the open #9372 stack, so land that stack and retarget this pull request onto it to leave only the slide and focus work.
The slide and the focus move otherwise match the current component:Resizable FR8 and component:LayoutPanel FR7 and AR2, and the earlier pane-watcher leak is fixed. Two smaller points: the configuration-object useResizable is not yet deprecated beside its new ref-taking form, as architecture:public-component-api INV22 describes, and the FR8 verification row says the browser spec covers paths it does not drive.
[Automated review]
| | --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------- | | ||
| | FR1 | ResizeHandle renders one focusable Handle and one enlarged Grab zone (a `--spacing-4` strip on the resize axis); the default render also contains a Grip pill, and caller `children` replace the pill. The grab zone is centred on the Handle for every `pillPlacement`, and the painted pill is its own grab target. On a pane too short for the full strip, the zone shrinks so the pane keeps at least half of itself. A pill shown only on hover also takes the pointer across the gap between it and the Grab zone, never past the pill's far side. | Current source, docs, tests, and an owner ruling | | ||
| | FR2 | The Handle carries `resize-handle`, the Grip pill carries `resize-handle-pill`, and the Grab zone carries no public target. | Current source and docs | | ||
| | FR3 | Pointer drag, keyboard, double-click collapse, snapping, persistence, reversal, and size behavior are owned by `createResizable`, which ResizeHandle and `useResizable` go through; dragging under right-to-left inverts the pointer delta so the divider follows the pointer. A disabled Handle ignores pointer and keyboard input and exposes `aria-disabled`, and keeps its tab stop (`spec:AST-085` FR1). With `autoSaveId`, storage holds the last settled size and receives the final size once when a drag ends, cancels, or unmounts. | Current source, docs, and tests | |
There was a problem hiding this comment.
This rewording is a normative change to FR3 and waits on an engineering owner's /approve-spec <full head SHA>. Note that a ResizeHandle given a hand-built ResizableProps still runs its own gesture against that object's callbacks, not createResizable.
| | FR6 | `useResizable.test.ts`; typecheck | Numeric bounds; conflicting bounds | A removed alias that type-checks, a numeric bound that is not pixels, or a minimum that wins a conflict fails. | `audit:Resizable/behavior` | | ||
| | FR7 | `ResizeHandle.test.tsx` consumer `onKeyDown` test; source review | Caller props and `onKeyDown` | A consumer `onKeyDown` that stops keyboard resizing, or keyboard resizing that skips the consumer handler, fails. Gap: no test covers other caller props, the styling merge, caller `onFocus`, `onBlur`, or `onDoubleClick`, or a caller `role` or ARIA attribute. | `audit:Resizable/api` | | ||
| | FR8 | Real-browser frame capture of a start and an end LayoutPanel collapsing and opening by each listed path, a drag reopening a pane, a hook-only pane, and a hand-built `ResizableProps` pane (pending) | Start and end panes; `collapse()`, `expand()`, Enter, double-click, the grow arrow, a controlled `isCollapsed` change, a drag across `collapsedSize`; a reopening drag; the hook's `props` and a hand-built object through `resizable`; Reduce Motion | A jump instead of a slide, a slide from the wrong edge, a slide during a reopening drag or on first render, a slide for a hook-only pane or a hand-built object, no slide for the hook's `props`, or a new prop, option or return member fails. | `audit:Resizable/behavior` | | ||
| | FR8 | `paneSlide.interaction.browser.spec.ts`: a start and an end LayoutPanel collapsing and opening by each listed path, a drag reopening a pane, and a hand-built `ResizableProps` pane; `paneReveal.test.tsx` for a hook-only pane | Start and end panes; `collapse()`, `expand()`, Enter, double-click, the grow arrow, a controlled `isCollapsed` change, a drag across `collapsedSize`; a reopening drag; the hook's `props` and a hand-built object through `resizable`; Reduce Motion | A jump instead of a slide, a slide from the wrong edge, a slide during a reopening drag or on first render, a slide for a hook-only pane or a hand-built object, no slide for the hook's `props`, or a new prop, option or return member fails. | `audit:Resizable/behavior` | |
There was a problem hiding this comment.
Non-blocking: paneSlide.interaction.browser.spec.ts drives neither expand() nor a resize out of collapse, and the end panel only through Enter, so "each listed path" overstates it. Name paneReveal.test.tsx for those paths, or add them to the browser spec.
| * <ResizeHandle ref={handleRef} /> | ||
| * ``` | ||
| */ | ||
| export function useResizable( |
There was a problem hiding this comment.
Non-blocking: architecture:public-component-api INV22 pairs this ref-taking signature with deprecating the configuration-object one under INV9; the two overloads below stay current, so the INV22 gap for useResizable stays open.
Batch: Resizable · the LayoutPanel slide
Size: 2,372 added lines, 1,431 of them tests, the evidence for one collapse path.
Stacked on #9372 and the PRs below it; until they merge, this branch shows their commits. Its own commits: vjeux/astryx@f2b0a93...1e97286.
User need
A collapsing LayoutPanel jumped shut, and focus stayed inside it at zero width. It now uses
spec:AST-105FR9's edge reveal.Problem and solution fit
useResizable(handleRef)region, or given the hook's props, slides closed and open from the edge facing its handle. A reopening drag follows the pointer; a hand-builtResizablePropsobject does not slide.aria-hiddenuntil it opens, under any resize controller.Current specification
spec:AST-105FR2 and FR9;architecture:react-component-runtimeINV12;family:layout-regionscomponent:ResizableFR8;component:LayoutPanelFR7 and AR2 (edited)Public delta and caller burden
No new prop, option or member. Changesets:
[feat]slide,[fix]focus move.Design and alternatives
useFocusLeavingChangeandmoveFocusLeftBehind, so the ring follows the last input (architecture:interaction-modalityINV1).resizablePane.ts.Evidence
Writing rubric results
Over
component:Resizableandcomponent:LayoutPanel: FR8's, FR7's and AR2's implementation-gap sentences and their Current-state lines go, and their verification rows name the browser spec. The records are owner-gated, so they wait for a spec owner's/approve-spec.Scope
Testing
Local checks at 1e97286, on main b377b21: core typecheck, the Resizable, Layout, SideNav and AppShell units (868 passed), strict eslint and prettier on the changed files, and the comment-length check. The test commits fail before their changes, including a region collapsed before the hook's first layout effect, whose pane played no slide, and a suspended region that took a panel lent beside its handle. The full gates run at this head once CodeBunny approves.