Review 3935
#3935 feat(core): add isSticky/stickyOffset props to Stack family by jiunshinn (bucket: contributor)
f4408f06200d3e447011548f6d7f3f5e47647ce6
REFRESH: merged current main e6507a95fb13cae83f94e2d4b58009eaf584f1c6 in a normal merge commit. The only textual conflict was append-only drift in Stack.test.tsx; the resolution kept both main's padding tests and the PR's sticky tests. No author logic was changed.
LOOP VERSION: 1.6.0 AUDIT RUBRIC: 1.13
LANE: full WHY: new permanent public API, intended layout behavior, a 44-day refresh conflict, and multiple real-container checks.
WHY 1: product-detail, docs-sidebar, and filter-rail layouts need one column to remain visible while a taller neighbour scrolls. WHY 2: builders currently leave the design system and write three custom declarations for every instance. WHY 3: repeated custom layout CSS makes templates harder to theme and prevents the layout primitives from expressing a common relationship. USER-FACING PROBLEM: a builder composing a two-column product or navigation layout cannot express the sticky rail through Astryx layout APIs. PROBLEM SEVERITY: missing capability — the demonstrated product-detail template requires custom CSS.
VERDICT: clear
A Stack can opt into sticky positioning and choose a token-based block-start offset. The responsibility lives on Stack and is inherited by HStack/VStack. The product-detail template then replaces its three-declaration workaround with those props.
SOLUTION (2 decisions · ~38 runtime lines of 255)
-
isStickyenables sticky positioning and also forcesalign-self: flex-start;stickyOffsetselects a spacing-token inset — serves the sticky rail problem. - product-detail adopts the public props — proves one end-to-end consumer and removes its custom CSS.
BURDEN: medium — two permanent public props and one shared StyleX map; no state, Effects, listeners, timers, or observers. BURDEN MATCH: proportionate for the demonstrated rail class, but the bundled cross-axis alignment creates an ordinary layout footgun.
VERDICT: BLOCKS — the sticky switch also changes an independent sizing axis.
OWNER: Stack, as the public utility layout primitive selected in #3223.
TIER 1: Stack family + spacing tokens; no new shared behavior system.
TIER 2: none.
SEAMS: Stack/HStack/VStack inheritance; grid and row/column flex parents; internal scroll containers; RTL; consumer xstyle precedence.
BEHAVIOR UNIT: inline — two static StyleX declarations and an O(1) token lookup are small and directly testable.
| seam | driven result |
|---|---|
| previous product-detail workaround vs new props | byte-identical pixels and identical 150×106 geometry after 180px scroll |
| scroll container | sticks 33px from the pane edge for stickyOffset={8} (32px token + 1px border) |
| HStack in a column flex parent | sticks, but width changes from 302px to 173.98px |
xstyle={{alignSelf: 'stretch'}} equivalent |
sticks and restores 302px content width |
| nested RTL rail | computed direction rtl; sticky positioning remains active |
| 200% text zoom | focus remains reachable; focusing the action scrolls to the pane's 498px maximum |
Stack ownership and the isSticky / stickyOffset names are already reserved by the maintainer's layout standard. The open problem is implementation scope: isSticky currently owns both sticky positioning and cross-axis alignment.
VERDICT: BLOCKS — an orthogonal layout axis changes as a side effect of enabling sticky behavior.
Existing Stack/HStack/VStack callers are unchanged until they opt in. The migrated product-detail rail renders identically to its old workaround. A builder using the new API for an HStack header inside a normal column layout gets a content-width header instead of the parent's full width; the measured 302px box becomes 173.98px.
VERDICT: BLOCKS — an ordinary opt-in composition visibly collapses.
<VStack gap={0} isSticky stickyOffset={8}>
<ProductInfo />
</VStack>| change | public? | class | doc'd? | verdict | |
|---|---|---|---|---|---|
+ |
Stack.isSticky?: boolean = false (inherited by HStack/VStack) |
yes — core Stack/Layout barrels | product-detail, Outline side rail, AppShell side rail | HStack/VStack en+zh; unified Stack entry arrived later on main | BLOCK — changes alignment as well as position |
+ |
Stack.stickyOffset?: SpacingStep = 0 (inherited by HStack/VStack) |
yes — core Stack/Layout barrels | same sticky rail class | HStack/VStack en+zh; current-main literal-union guard red | ok after mechanical refresh cleanup |
OSSIFICATION: the class and names are established by #2613 and #3223, and utility-layout knobs are consistent with API Conventions. The meaning of isSticky would also permanently promise a cross-axis alignment reset, though, so that side effect must be resolved before release.
Current-main drift, not contributor debt: the unified Stack docs and literal-union guard landed after this PR opened. The refreshed branch now needs the Stack entry plus four inline SpacingStep unions before it can pass current CI.
VERDICT: BLOCKS — the public switch is not orthogonal in a builder's hands.
No theme target, component variable, or token is added. stickyOffset consumes the existing spacing scale. The consumer xstyle remains last in the StyleX merge; the alignment override control restored the full 302px width while preserving stickiness.
VERDICT: clear
BEHAVIOR: existing callers unchanged; opt-in isSticky adds position, block-start inset, and cross-axis alignment.
API: additive only.
VISUAL: product-detail migration is byte-identical to the previous workaround; constrained column composition shrinks 302px → 173.98px.
THEME: no target/token/override changes; spacing tokens continue to resolve.
VERDICT: BLOCKS — the opt-in visual contract collapses an ordinary full-width header.
EFFECTS: zero added, changed, kept, or moved.
RENDER: one static conditional StyleX merge; no state or extra render pass. LISTENERS/OBSERVERS: none. LAYOUT: no JS reads/writes or forced reflow; browser-native sticky layout only. BUNDLE: no dependency; one 11-entry spacing-token style map.
VERDICT: clear
VISUAL CHECK: manual frames required WHY: the PR adds opt-in layout/CSS behavior, changes the product-detail template, and has no landed story covering sticky endpoints.
| Previous custom CSS | New props |
|---|---|
 |
 |
The PNGs are byte-identical (f940f24fe2eb7a67292fa00b022aef8754f4e792c4e82350095df71b7e36c7af). Both receipts: exact head f4408f0; neutral/light; LTR; 900×600@1; 180px scroll; position: sticky; 32px inset; align-self: flex-start; focused action retained; 150×106 subject; fonts loaded; zero Storybook/page errors.
| Constrained column + RTL | 200% text zoom | Alignment override control |
|---|---|---|
 |
 |
 |
The constrained receipt measures the normal stretched baseline at 302px and the sticky HStack at 173.98px, while a nested RTL rail remains sticky. The zoom receipt verifies 32px computed text, focus retention, and a reachable action at maximum scroll. The override receipt verifies align-self: stretch, sticky top 17px in the padded pane, and restored 302px width. All use exact head f4408f0, neutral/light, 900×600@1, settled fonts, and zero capture errors. The exact temporary story delta is archived as measurement-arm.diff.
VERDICT: BLOCKS — the constrained neighbour proves an unintended width change on the new API.
REMEDY SEARCH: not triggered — author owns HOW; acceptance criteria are sufficient
The settled outcome is: preserve the Stack's existing cross-axis sizing in a column parent, while the demonstrated grid/rail case still sticks at its token offset. The implementation is not prescribed.
No role, ARIA, string, event, or focus-order code changes. Real Chromium kept the button inside the sticky rail focused through scrolling in LTR, nested RTL, 200% text zoom, and the alignment override control. At 200% zoom the lower action becomes fully visible at the pane's scroll end, and focusing it scrolls there. Remote pr-a11y and pr-rtl both passed on the exact head; no a11y baseline entry was added.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | BLOCKS — sticky also changes cross-axis sizing |
| ARCHITECTURE | BLOCKS — orthogonal layout axis is bundled |
| IMPACT | BLOCKS — ordinary column composition collapses |
| API | BLOCKS — public switch promises the side effect |
| THEMING | clear |
| BREAKING | BLOCKS — opt-in visual contract |
| PERFORMANCE | clear |
| VISUAL | BLOCKS — 302px → 173.98px |
| A11Y & I18N | clear |
GOAL: partly met — the demonstrated product rail is byte-identical to the workaround, but the same public API visibly narrows an ordinary column-parent header.
REMOTE CI: lint, build, Storybook, sandbox, docsite, smoke, a11y, RTL, theme layers, and stable visual regression passed. Full test was 12,788 pass / 1 fail / 5 skipped; the sole failure is the four current-main SpacingStep doc literals reproduced locally. Vercel failed separately; review-required and visual-acceptance remain pending.
DISPOSITION: bundled alignment side effect → blocks now; refreshed-doc/literal-union drift → maintainer-owned cleanup on this PR after the API shape settles, not contributor debt.
ADVICE: bounded outcome criteria — preserve existing cross-axis sizing while keeping the demonstrated rail sticky at its token offset.
AUTHOR CAN PROCEED: yes — ownership, public names, constraints, and acceptance checks are settled; implementation HOW is open.
WORST OUTCOME: “a builder using the new API for an HStack header inside a normal column layout gets a content-width header instead of the parent's full width” → request changes.
JUDGEMENT NEEDED: none — #3223 already assigns the capability and names to Stack; this is an implementation-contract defect.
request changes
-
isStickyunconditionally addsalign-self: flex-start→ a builder's full-width HStack header collapses from 302px to 173.98px in a column parent ·sticky.stylex.ts:31CONFIRMATION: changing only the alignment through the documentedxstyleseam restores 302px while the element remains sticky.
Thanks for closing the sticky-layout gap. I merged current main and kept both appended Stack test blocks.
isSticky also changes cross-axis sizing. In a normal column layout, a full-width HStack collapses around its button; restoring its width needs the custom xstyle this API is meant to remove. Could we preserve the Stack's existing cross-axis sizing while keeping the rail case sticky?
If you'd rather talk it through with someone, we're in Discord.
[Reviewed by Robohands]
-
packages/core/src/Layout/sticky.stylex.ts:31— This also changes cross-axis sizing; a column-parent HStack shrinks 302px → 174px.
- The product-detail old/new screenshots are byte-identical, including geometry and focus state.
- Current main added the unified Stack docs and literal-union guard after this PR opened; the four local doc failures are refresh drift.
- Public AppShell/SideNav sticky owners add their own stacking/background treatment; the generic rail test does not need either because its sibling does not overlap it.
TIME total 28m setup/rules 5m loop 1.6.0, audit 1.13, trust guard, public guidance branch refresh 3m trial merge, one append-only conflict, normal push build/server 2m fast-install donor + guarded Storybook; warm main reused: yes browser/a11y 7m 5 receipt frames + 3 diagnostic re-measures focused tests 2m 381 focused tests on head; current-main control code/history 3m landed precedent, issues, drift chronology critique/wiki 6m 2 critic passes, durable frames and page CI wait 16m concurrent remote exact-head checks; no review slot held waste 3m (subset) favicon recapture, pane-padding probe correction, failed contact-sheet tooling
- Safari/WebKit rendering was unavailable in this run; the change uses standard logical sticky CSS and Chromium evidence is complete.
Posted exactly as drafted at the unchanged reviewed head: CHANGES_REQUESTED review. No body edits.
Current exit: open, MERGEABLE, CHANGES_REQUESTED at f4408f06200d3e447011548f6d7f3f5e47647ce6; merge remains held by the blocking cross-axis sizing finding, red full test, failed Vercel deployment, and pending visual-acceptance status. No merge or auto-merge action.