Skip to content

refactor(Resizable): one internal createResizable factory, which useResizable's configuration-object form and ResizeHandle's resizable path go through - #9297

Merged
cixzhang merged 7 commits into
meta:mainfrom
vjeux:create-resizable-factory
Nov 3, 2026

Conversation

@vjeux

@vjeux vjeux commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

One internal resize region, with the configuration-object useResizable moved onto it. Next come the factory's element path (#9718), useResizable(handleRef, options) (#9370), the public export (#9372), the LayoutPanel slide (#9300) and the deprecation (#9202).

Maintainer impact

architecture:public-component-api INV22 has every public interaction hook delegate to a create…(element, options) factory (spec:AST-107 FR6). The region's state lived in useResizable. A factory beside it would leave two copies of the size model, so each sizing fix would land twice. This pull request adds the region and moves the configuration-object form onto it at once.

Intended invariant

No public API or record changes. The configuration-object form keeps its signature, props, warnings, one-render-pass mounts and same-commit reports (component:Resizable FR8).

Two behaviors change. Each is a [fix] that restores what component:Resizable FR3 promises for persistence and size (spec:AST-017 FR5):

  • A server-rendered region with an autoSaveId hydrates with the server's markup, then takes its saved size.
  • A server-rendered region with a percentage defaultSize and no containerRef hydrates with the server's markup, then resolves against the window.

Change

  • The region. createResizable(options) keeps a resizable region with no React and no element. It resolves, clamps, snaps, collapses and saves the size through resizableModel.ts, follows a percentage's basis, and reports the size. ResizeHandle's resizable path drives it through the gesture model. feat(Resizable): createResizable drives a plain handle element, with one separator derivation #9718 adds the element path.
  • One state. The configuration-object form is the region in React, with one factory. While its component is unmounted or an <Activity> hides it, the factory keeps the region, so collapse(), expand() and resize() always apply.
  • Hydration. The hook's server snapshot is the state the server renders, so a hydrating render matches the server's markup.
  • Housekeeping. Comments cite no records, and the provenance baseline loses the useResizable.ts sites. One [chore] and two [fix] changesets.

Evidence

  • Failure before the change: the region's tests fail (no module), the configuration-object form's stand-in tests fail, and both hydration cases fail. On main, the saved size logs a hydration mismatch, and the percentage stays at 360, not 240.
  • Success after the change: those tests pass.
  • Product/runtime behavior verified unchanged: guards that pass before and after cover onCollapseChange's commit, the size across an <Activity>, early and hidden calls, the warning words and a swapped container. The ResizeHandle, Layout, SideNav and AppShell suites pass unchanged.

Scope

  • No intended public API, visual direction, or policy change. The behavior changes are the two [fix] entries above.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Testing

Local checks at dd345f4, on main b377b21: core typecheck, the Resizable, Layout, SideNav and AppShell units (775 passed), strict eslint and prettier on the changed files, and the comment-length check. The test commits fail before their changes: the region's module is missing, the stand-in tests fail, both hydration cases fail, and a WIDE, NARROW, WIDE switch keeps NARROW's bounds. The full gates run at this head once CodeBunny approves.

@vercel

vercel Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 3, 2026 4:36pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 1, 2026
@astracat-bot

astracat-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 3, 2026, 4:51 PM UTC

@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · dd345f4

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
SideNav 68.3 kB (+1.4 kB) · 11.2 kB CSS (0) — — · — · —
ResizeHandle 19.5 kB (−1 B) · 4.3 kB CSS (0) — ✓ · ✓ · ✓

Package size: @astryxdesign/core 6.22 MB packed (+3.5 kB); no new dependencies.

API changes: none in this pull request

Committed API report behind this build: core 2 rows; run pnpm api:report.

Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot 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.

This needs a spec decision before it can be approved. The rewritten component:Resizable FR3 owner sentence is a new normative statement, so an engineering owner (the author qualifies) needs to approve it by commenting /approve-spec <full head SHA>, or say what to change. Separately, these need changes:

  • Split the 5,756 changed lines: land #9197 first, then the factory, the useResizable(handleRef, options) overload, and the move of the configuration-object form and ResizeHandle onto the factory.
  • A ResizeHandle or plain element that a child mounts from its own state is never bound to the region (no value, and keys and drags do nothing), because useResizable re-reads the ref only when its own component commits.
  • A configuration object kept stable across renders no longer follows a container replaced behind its containerRef, as it does on the base, so the changeset's "behavior unchanged" does not hold yet.

[Automated review]

// attributes, and a factory with no handle keeps the region answering
// collapse, expand and resize; a new element gets a new factory. Each new
// factory starts where the old one stood.
useIsomorphicLayoutEffect(() => {

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.

This effect re-reads ref.current only when this hook's component commits. A ResizeHandle or plain element that a child mounts from its own state is therefore never bound: the separator has no aria-valuenow, and keys and drags do nothing. An element a child removes keeps its factory until the owner re-renders. ResizeHandle already receives this ref object as ref, so registering the slot against the ref object and letting ResizeHandle's claimRef ask it to re-check on attach and detach would cover the component case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9370: ResizeHandle's callback ref tells the hook watching its ref as the element attaches and lets go, so a handle a child mounts, or hides and shows in its own <Activity>, is bound at once (tests there).

},
[slot],
);
useIsomorphicLayoutEffect(() => {

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.

update runs only when options changes identity, and the factory re-reads containerRef.current only inside commit(). With a config kept stable (a module constant or useMemo) and its container replaced behind the same ref, the factory keeps observing the old element: maxSize: '50%' stays at 199px, where the base moves to 399px. Re-syncing the container on every commit would keep the released behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here: the factory re-reads the container at each commit of the hook's component, so a configuration object kept across renders follows a swapped container (test added).

name: 'options.isDisabled',
type: 'boolean',
description:
'Ignores the drag and the keys and sets `aria-disabled`. The handle keeps its tab stop, so it stays discoverable. On a ResizeHandle, its own `isDisabled` prop does the same: either one disables the handle.',

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.

On a ResizeHandle this option gives tabindex="-1" (ResizeHandle.tsx:599, the FR3 gap), so "keeps its tab stop" holds only for a plain element. Likewise, a ResizeHandle ignores collapsedLabel.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc page, now in #9372, says a plain element keeps its tab stop and a ResizeHandle takes its own label; collapsedLabel is gone.

@vjeux
vjeux force-pushed the create-resizable-factory branch from eedebe1 to 73ca4d9 Compare November 1, 2026 11:12
@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 1, 2026
@cixzhang
cixzhang marked this pull request as ready for review November 1, 2026 11:37

@astracat-bot astracat-bot Bot 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.

This needs a spec decision before it can be approved. The rewritten component:Resizable FR3 owner sentence and the factory's new handle options (isReversed, isDisabled, label, collapsedLabel) have no current spec authority, so an engineering owner (the author qualifies) needs to approve them by commenting /approve-spec <full head SHA>, or say what to change. Separately, these need changes:

  • A configuration-object region returns to its default size when an <Activity> hides and shows it, because the hook's unmount cleanup discards the factory's state.
  • A ref-form ResizeHandle that a child mounts, or hides and shows inside its own <Activity>, is never bound, so it has no value and ignores keys and drags.
  • A configuration object kept stable across renders no longer follows a container swapped behind containerRef, so the changeset's "behavior unchanged" does not hold yet.
  • The new warn-once Effect in ResizeHandle should use useDevWarning, and the 5,045 changed lines should be split into the factory, the ref form, and the migration onto it.

[Automated review]

Comment on lines +569 to +575
useIsomorphicLayoutEffect(
() => () => {
slot.get()?.resizable.destroy();
slot.set(null);
},
[slot],
);

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.

This cleanup also runs when an <Activity> hides the subtree, and the reveal then creates a fresh factory from optionsRef.current, so the user's size is lost: a sidebar dragged to 300px comes back at its 250px default, where the base keeps 300px. Keeping readResizableSelection(...) of the destroyed factory in the slot and seeding the next factory from it, as the swap path above does, would keep it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here: the hook keeps where the region stood while an <Activity> hides it and seeds the next factory from it (test added).

Comment on lines +468 to +476
export function releaseResizeHandle(
element: Element,
claim: ResizeHandleClaim,
): void {
if (claims.get(element) === claim) {
claims.delete(element);
claim.set(null);
}
}

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.

Releasing clears the claim, but the factory binds only once, at creation (createResizable.ts:981), and the hook's effect returns early for the same element. After a child's <Activity> hides and shows the handle it stays unbound, with no aria-valuenow and no key or drag input, even after the owner re-renders. Keeping the binding per element and restoring it in claimResizeHandle would cover this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9370: a factory stays bound to its element while the factory lives, so a handle that lets go and claims the element again renders it again.

Comment on lines +532 to +541
useEffect(() => {
if (resizable != null && binding != null && !didWarnBothRef.current) {
didWarnBothRef.current = true;
devWarn(
'ResizeHandle',
'has a `resizable` prop and a useResizable(handleRef) region at ' +
'once. The `resizable` prop wins; pass one of them.',
);
}
toggleCollapse(resizable);
}, [isDisabled, resizable]);
}, [resizable, binding]);

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.

useDevWarning('ResizeHandle', message, resizable != null && binding != null) (hooks/useDevWarning.ts) already owns warn-once-per-mount; this Effect and didWarnBothRef copy it, and the axis warning above can move to it too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9370: the warn-once uses useDevWarning.

Comment thread .changeset/create-resizable.md Outdated
'@astryxdesign/core': patch
---

[feat] `createResizable(handle, options)` makes a plain element a window splitter that resizes a region, without React: a pointer drag that holds its pointer across embedded frames, the arrow keys, Home, End, Enter and a double-click, with bounds, snap points, collapse and saved sizes. It reports the size through `subscribe` and `onSizeChange`, and you size the panel; `destroy` removes every listener and attribute it added. `useResizable(handleRef, options)` is its React form, beside the configuration-object form: give the same ref to `ResizeHandle` (or to a plain element) and render the panel at `size`; a plain element is named and announced through the message catalog. `ResizeHandle` and the configuration-object form go through the same implementation, with their behavior unchanged.

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.

"with their behavior unchanged" does not hold yet for the configuration-object form: its size is lost across an <Activity> hide and reveal, and a memoized config no longer follows a container swapped behind containerRef.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request's [chore] changeset says no behavior changes, and both cases named here are fixed and tested.

@vjeux vjeux changed the title feat(Resizable): createResizable, with useResizable(handleRef, options) as its React wrapper feat(Resizable): createResizable makes a plain element a window splitter that resizes a region, without React Nov 1, 2026
@vjeux
vjeux force-pushed the create-resizable-factory branch from 73ca4d9 to a8a8a2f Compare November 1, 2026 15:58
@github-actions github-actions Bot removed the needs:spec-owner-review Current knowledge records await owner approval label Nov 1, 2026

@astracat-bot astracat-bot Bot 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.

Approval waits on an engineering owner's decision: no current spec decides createResizable's isReversed, isDisabled and label options, the controller's update and subscribe, or a null handle, so an owner needs to name the record that governs them, or have them added to component:Resizable's public contract and record the approval by commenting /approve-spec <full head SHA> (the author may do so as an engineering owner). Independently of that decision, these need changes:

  • resizeGesture.ts and createResizable.ts re-implement ResizeHandle's input and useResizable's region state; route ResizeHandle through createResizeGesture and share the region logic so each concern has one implementation.
  • update() replaces every option, so a partial call reopens a collapsed region and drops its bounds, label and saving; merge the options or document that the full set is required.
  • The change is 2,325 authored lines, over the 2,000-line limit; recheck the size once the points above are settled.

[Automated review]

* region holding an embedded frame otherwise loses the drag the moment the
* pointer crosses the frame.
*/
export function createResizeGesture(

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.

This re-implements ResizeHandle's own handlers (ResizeHandle.tsx:518-688, with adjacentSnapPoint, toggleCollapse and the body cursor at :107-115 and :373-419), and ResizeHandle does not call it. Routing ResizeHandle through createResizeGesture here (its event types already accept React events) would leave one implementation of the splitter's input.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here: ResizeHandle's resizable path now goes through createResizeGesture, so the splitter's input has one implementation.

}

/** The region's state from its options, its basis and its selection. */
function compute(): void {

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.

compute(), commit(), persist() and the model below repeat useResizable's single-region state (useResizable.ts:278-291, :482-570, :628-653, :668-759, :792-805), so a clamping or saving fix in one misses the other. Sharing these decisions through resizableModel.ts, or having the hook delegate to this factory, keeps one path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here: useResizable(config) builds on this factory, so the region's state has one copy.

if (isDestroyed) {
return;
}
opts = next;

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.

update replaces the whole options object. On a collapsed, saved region with bounds 200–400, update({isDisabled: true}) reopens it at 300px, resets the bounds to 50 and unbounded, renames the handle "Resize handle", stops saving and drops onSizeChange, while the JSDoc and doc page say state is kept. Either merge next over the current options or document that the full set is required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented: update() takes the full set of options, as a new render does; an option left out returns to its default, and the region keeps its size and collapse state. A test pins it.

},
sidebar: {
flexShrink: 0,
padding: spacingVars['--spacing-3'],

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.

With content-box sizing this padding stays when width is 0px, so the collapsed sidebar keeps a 24px strip with a clipped "S" and "0". Padding an inner element avoids it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9372: the sidebar pads an inner element, so collapsed to 0 it leaves no strip.

@vjeux
vjeux force-pushed the create-resizable-factory branch from a8a8a2f to 86812b8 Compare November 1, 2026 17:31
@vjeux vjeux changed the title feat(Resizable): createResizable makes a plain element a window splitter that resizes a region, without React feat(Resizable): createResizable, internal to the package, makes a plain element a window splitter that resizes a region, without React Nov 1, 2026

@astracat-bot astracat-bot Bot 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 requested: the internal factory looks correct and its tests pass, but resizeGesture.ts and createResizable.ts copy ResizeHandle's input and useResizable's single-region state instead of extracting them, so after this merges every splitter fix has to land twice, and the copies already differ (a disabled handle's tab stop, and which document gets the drag cursor). Please extract rather than copy, as #9197 did for the model: route ResizeHandle's handlers through createResizeGesture, and share the region decisions through resizableModel.ts (or land the factory together with the stacked move of useResizable onto it). No spec decision is needed for this cut, since nothing exports the factory, and the earlier notes about update() and the story no longer apply.

[Automated review]

@vjeux vjeux changed the title feat(Resizable): createResizable, internal to the package, makes a plain element a window splitter that resizes a region, without React refactor(Resizable): one internal createResizable factory, which useResizable's configuration-object form and ResizeHandle's resizable path go through Nov 1, 2026
@vjeux
vjeux force-pushed the create-resizable-factory branch from ee538dd to f76c7f8 Compare November 3, 2026 06:29
@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 3, 2026

@astracat-bot astracat-bot Bot 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.

This needs an engineering owner's spec decision before it can be approved, and two changes either way.

Because this branch is stacked on #9529, it amends component:Resizable FR1 (the Grab zone stays centred on the Handle, the painted grip is its own grab target, and the zone shrinks on a short pane). An engineering owner approved that same text on #9529, but this pull request's head has no approval, so an engineering owner records the decision by commenting /approve-spec <full head SHA> here (the author may, as an engineering owner), or this waits until #9529 lands and FR1 is current on main.

Required either way:

  • A grip shown only on hover (isAlwaysVisible={false}) and drawn outside the Grab zone hides as the pointer moves onto it, so the resizable SideNav can no longer be dragged from its grip in right-to-left (it still can in left-to-right); please keep that grip reachable from the zone and add browser cases for an overlay Handle with a hover-revealed grip in left-to-right start and right-to-left end.
  • At 4,367 changed lines, 1,579 of them from #9680 and #9529, please land those first (with the fix above in #9529), or take #9529 out from under this branch, so this pull request ships only the factory and the hook moved onto it.

The factory, the hook migration and the three [fix] entries look right: the Resizable, Layout, SideNav and AppShell suites pass, and each new fix test fails on main.

[Automated review]

Comment on lines +548 to +552
onPointerLeave: () => {
if (!isDragging) {
setIsHovered(false);
}
},

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.

In right-to-left, SideNav's overlay Handle draws its end grip 4px outside this zone. Leaving the zone to reach it clears the hover, so pillHidden (:231) hides the grip and drops its pointer events before the pointer arrives; a drag from the grip then does nothing (260px on this head, 300px on main). Keeping a hover-revealed grip reachable from the zone, and a browser case for isAlwaysVisible={false} in LTR start and RTL end, would close it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9529 (b79aa8e), which this carries: a hover-revealed pill stays reachable from the zone, with a browser case in left-to-right start and right-to-left end.

@astracat-bot astracat-bot Bot 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.

This needs an engineering owner's spec decision before it can be approved, and three changes either way.

Because this branch is stacked on #9529, it amends component:Resizable FR1 with text an engineering owner approved on #9529 but not on this pull request, and #9529 has since added a 150 ms delay before a hover-revealed grip hides, which FR1 does not state. An engineering owner records the decision by commenting /approve-spec <full head SHA> here and saying whether it covers that delay (the author may, as an engineering owner), or this waits until #9529 lands and FR1 is current on main.

Required either way:

  • useResizable skips update() when it is given the same configuration object it was created with, so switching between two stable objects (WIDE → NARROW → WIDE) keeps NARROW's bounds (useResizable.ts:551); please compare against the options last passed to the factory and add a test for that switch.
  • In right-to-left, SideNav's hover-revealed grip sits 4px outside the Grab zone and hides once the pointer has been off the zone for 150 ms, so a slower approach can no longer drag from it, while left-to-right and main have no time limit; please keep the grip reachable from the zone with no time limit, and add a browser case that waits in the gap.
  • At 3,653 changed lines, 841 of them from #9529, please land #9529 first (with the fix above), or take it out from under this branch, so this pull request ships only the factory and the hook moved onto it.

The factory, the hook migration and the two hydration [fix] entries look right, and both hydration tests fail on main.

[Automated review]

);
useIsomorphicLayoutEffect(() => {
const entry = slot.get();
if (entry != null && options !== entry.createdWith) {

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.

createdWith is the object the factory was created with and is never refreshed, so a caller that switches WIDE → NARROW → WIDE (two stable objects) skips the last update() and keeps NARROW's bounds; the base follows WIDE again. Comparing against the options last passed to the factory would fix it, with a test for that switch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 63ff73a: the hook compares each configuration with the options the factory has now, set at every update. 6344cc7 adds the WIDE, NARROW, WIDE case, which fails before the fix.

Comment on lines +565 to +570
onPointerLeave: () => {
if (!isDragging && hoverExitRef.current == null) {
hoverExitRef.current = setTimeout(() => {
hoverExitRef.current = null;
setIsHovered(false);
}, PILL_HOVER_GRACE_MS);

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.

In right-to-left SideNav the end pill sits 4px outside the zone, so this 150 ms delay is the only thing keeping it reachable: a pointer that rests in the gap for 400 ms finds the pill hidden and a drag from it does nothing, while left-to-right (pill inside the zone) and main have no time limit. Keeping the pill reachable from the zone with no time limit, plus a browser case that waits in the gap, would close it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9529 (f9a1753), which this carries: there is no time limit, because the pill's hit area spans the gap to the zone. Its browser case rests 300 ms in the gap.

@astracat-bot astracat-bot Bot 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.

This needs an engineering owner's spec decision before it can be approved, and two changes either way.

Because this branch is stacked on #9529, it amends component:Resizable FR1 (the Grab zone centred on the Handle for every pillPlacement, the painted grip as its own grab target, the zone shrinking on a short pane) with text an engineering owner approved on #9529 but not on this pull request, and #9529 has since added an extension of a hover-revealed grip's hit area that FR1 does not state. An engineering owner records the decision by commenting /approve-spec <full head SHA> here and saying whether it covers that extension (the author may do so as an engineering owner), or this waits until #9529 lands and FR1 is current on main.

Blocking either way:

  • A hover-revealed grip's extension (ResizeHandle.tsx:225-244, applied at :658-662) covers the whole upper half of a pane under 24px across the grip's width, so a 12px item dragged from near its top resizes instead of moving, against the changeset's "a short item … keeps its own drag"; limit the extension to the gap it bridges (or clamp it on a short pane, as the zone is) and add a short-items browser case with isAlwaysVisible={false}.
  • The diff is 3,679 authored lines, 855 of them #9529's separately reviewable Grab zone change; land #9529 first or take it out from under this branch, leaving this pull request's own 2,824 lines.

The configuration-object regression and the right-to-left SideNav grip from the last round are fixed, and the factory, the hook migration and both hydration fixes match the current records.

[Automated review]

Comment on lines +658 to +662
!isAlwaysVisible &&
effectiveSide !== 'center' &&
(isHorizontal
? styles.pillBridgeHorizontal
: styles.pillBridgeVertical),

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.

This extension is added on both sides of a hover-revealed grip, also where the grip already touches the zone. On a pane under 24px the far side covers the pane's upper half: on a 12px item with isAlwaysVisible={false}, after the pointer touches the zone, every point of the item's upper half at the grip's column hits the grip, and a drag there resizes it from 12 to 6px instead of reaching the item's own drag. Extending only toward the Handle, and only where the grip sits outside the zone (or clamping it on a short pane as the zone is), would keep the short-item drag the changeset describes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #9529, which this carries: the bridge spans only the gap toward the Handle (ec69381), d8960ff checks the point past the pill's far side, and FR1 states it in the owner's words (6693aa4).

@astracat-bot astracat-bot Bot 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.

This needs an engineering owner's spec decision before it can be approved, and one change either way.

Because this branch is stacked on #9529, it amends component:Resizable FR1 (the Grab zone centred on the Handle for every pillPlacement, the painted grip as its own grab target, the zone shrinking on a short pane, and a hover-revealed grip taking the pointer across the gap to the zone) with text an engineering owner approved on #9529 but not on this pull request. An engineering owner records the decision by commenting /approve-spec <full head SHA> here (the author may do so as an engineering owner), or this waits until #9529 lands and FR1 is current on main.

Blocking either way:

  • The diff is 3,735 authored lines, 909 of them #9529's separately reviewable Grab zone change; land #9529 first or take it out from under this branch, leaving this pull request's own 2,826 lines.

The hover-revealed grip no longer takes a short pane's upper half, and the factory, the hook migration and both hydration fixes match the current records.

[Automated review]

Comment on lines +379 to +381
// =============================================================================
// The resize factory in React
// =============================================================================

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.

Non-blocking: this banner restates the code under it. Dropping it, as the branch already did for createResizable.ts, leaves the code to explain itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. The banner moves through the four pull requests stacked on this one, so I drop it at the top of the stack in its next round rather than restack all five for a comment.

vjeux added 7 commits November 3, 2026 08:04
…ct, the configuration-object useResizable builds on it, and a server-rendered region hydrates with the server markup

- createResizable: collapse, expand and resize on request, a gesture
  through the region's model as a ResizeHandle drives it, bounds and snap
  points, a percentage against its container, saving once per gesture,
  update() with the full set of options, subscribe() and destroy, and an
  import that does no DOM work. They fail here: the module does not exist.
- With a stand-in factory, useResizable(config) creates one, from the
  configuration, passes later options through update(), destroys it on
  unmount and returns what it reports. They fail here.
- A region with an autoSaveId whose size is saved hydrates over server
  markup with the default, and a '30%' default with no container hydrates
  over markup the server resolved at its 1200px basis. Both should
  hydrate cleanly, then take the client's size. Both fail here: the first
  logs a hydration mismatch and keeps 200, the second keeps 360.
- Guards that pass here and after: the configuration-object form reports
  in the commit of onCollapseChange, controlled or not; it keeps its size
  and collapse state across an <Activity>; collapse(), expand() and
  resize() apply before its first layout effect and while hidden; its
  warnings start useResizable: in the words they always used; it follows
  a container swapped behind its ref; and a '30%' default resolves against
  its container once measured, after an <Activity> hid and showed it.
…and the configuration-object useResizable builds on it

createResizable, internal to the package, keeps a resizable region with
no React and no element: it resolves, clamps, snaps, collapses and saves
the size through the model in resizableModel.ts, follows a percentage's
basis, and reports through subscribe and onSizeChange. ResizeHandle's
resizable path drives it through resizeGesture.ts's model.

The configuration-object useResizable is that region in React, with one
factory, so the region's state has one copy. Its props carry what the
factory reports, and its signature, props, warnings and one-render-pass
mounts are unchanged. While its component is unmounted or an <Activity>
hides it, the factory lets go of the page and keeps the region, so
collapse(), expand() and resize() apply and report whenever they are
called. Its server snapshot is the state the server renders, so a
hydrating render matches the server's markup, and a saved size or a
percentage default resolved against the window applies in the next
commit.

The comment-provenance baseline drops the useResizable.ts sites this
removes. A [chore] changeset records the move, and two [fix] changesets
record the hydration behaviors.
On an <Activity> reveal, follow() resumes the same factory; none is made
for the reveal. The comment now says so.
…s the factory again after another one

A caller that switches between two stable configuration objects, WIDE,
then NARROW, then WIDE, must get WIDE's bounds back.

It fails here: the hook compares each configuration with the one it
created the factory with, so the last switch never reaches update().
…e factory has now

The hook kept the options it created the factory with and compared every
later configuration against them. So WIDE, then NARROW, then WIDE skipped
the last update() and kept NARROW's bounds. The slot entry now holds the
options last passed to the factory, at creation or at an update.
@vjeux
vjeux force-pushed the create-resizable-factory branch from 1bc43bc to dd345f4 Compare November 3, 2026 16:27
@github-actions github-actions Bot removed needs:design-review Affects visuals — Design should review needs:spec-owner-review Current knowledge records await owner approval labels Nov 3, 2026

@astracat-bot astracat-bot Bot 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.

Recommended for approval. Moving the configuration-object useResizable onto one internal createResizable factory leaves the public API and the existing Resizable, Layout, LayoutPanel, SideNav and AppShell suites unchanged, and both hydration fixes reproduce on main and are fixed here.

Non-blocking suggestions:

  • readInitialResizableState repeats compute()'s starting-state steps (createResizable.ts:127-210 and :413-519); one shared pure resolver would stop a future sizing fix from making the first render or the server markup disagree with the factory.
  • A held region's collapse(), expand() or resize() after unmount now writes autoSaveId storage, which no changeset mentions, and the comment at useResizable.ts:496-499 says the calls apply "as React state did"; a changeset line and a corrected comment would match the code.

[Automated review]

* rendered, for a render that hydrates it: the 1200px basis for a viewport
* percentage too, and nothing saved.
*/
export function readInitialResizableState(

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.

Non-blocking: this repeats compute()'s starting-state steps (:413-519): saved size, basis, bounds, default, clamp, collapse and reported maximum. A sizing fix made only in compute() would make the first render or the server markup disagree with the factory's first snapshot (an extra render on mount, or a jump after hydration). One pure resolver over options, basis and saved state, called by both, would keep one copy.

* Before its first commit (the server, the first render, a hydrating render)
* it returns the state the factory starts in. While it is unmounted or an
* `<Activity>` hides it, the factory lets go of the page and keeps the region.
* So collapse, expand and resize apply and report whenever they are called, as

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.

Non-blocking: "as React state did" holds for an <Activity>-hidden region, but after unmount the base left autoSaveId storage unchanged, while the suspended factory now writes it (a resize(333) after unmount stores 333 here, 200 on main). Saying the calls apply even after unmount, and adding a line to a changeset, would match the code.

@vjeux
vjeux marked this pull request as ready for review November 3, 2026 20:19
@cixzhang
cixzhang marked this pull request as draft November 3, 2026 21:09
@cixzhang
cixzhang marked this pull request as ready for review November 3, 2026 21:09
@cixzhang
cixzhang merged commit f9a6965 into meta:main Nov 3, 2026
164 of 165 checks passed
@vjeux
vjeux deleted the create-resizable-factory branch November 3, 2026 22:13

This branch was successfully deployed

1 active deployment
Preview — dd345f49 Deployed Nov 3, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants