Skip to content

fix(core): page scroll and selection locks are one per page across Core loads (AST-110 FR3) - #10822

Merged
cixzhang merged 12 commits into
mainfrom
fix/scroll-lock-shared
Nov 10, 2026
Merged

fix(core): page scroll and selection locks are one per page across Core loads (AST-110 FR3)#10822
cixzhang merged 12 commits into
mainfrom
fix/scroll-lock-shared

Conversation

@cixzhang

@cixzhang cixzhang commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Stack: #10817 → #10822 → #10831

Review only this PR's own commits until the ones below merge.

User impact

Two places keep page-lock state per load of Core:

  • Page scroll lock. When Core's scroll-lock modules load twice on one page (two bundles of Core, or a copy that carries them), each load keeps its own holder count and saved page styles. With an overlay from each load open, closing the overlay that opened first unlocks the page while the other overlay is still open. Swizzled components import useScrollLock from @astryxdesign/core/hooks, so they share the package's lock already; this covers the two-load case.
  • Text-selection lock. utils/selectionLock is carried by swizzled copies. A copy's release restored the selection styles it had saved, which could be the package's lock, so the package's selection lock could end while it was still held.

After this change both keep one count and one set of saved styles per page.

Problem and solution fit

  • Why does the page unlock early? useScrollLock keeps its holder count and active lock in module variables, and rootScrollStyles keeps its claim stacks, saved page values and lock count in module variables. A second load of either module keeps its own.
  • Why does that matter? spec:AST-110 FR3: a module whose state coordinates components keeps one store per page, so a copy and the package component behave as the package alone does.

The state moves into the shared-realm helpers from #10817:

  • rootScrollStyles: claims, held targets, the claim counter, the lock count and its listeners → one sharedState('hooks/root-scroll-styles/state', 1, …).
  • useScrollLock: holder count and active lock → sharedState('hooks/use-scroll-lock/lock', 1, …).
  • pageHold: the per-document holds → sharedWeakMap('gestures/page-hold/holds-by-document', 1).
  • selectionLock: locked elements and their saved styles → sharedWeakMap('utils/selection-lock/locks', 1); a swizzled copy carries this module, so a copy's release no longer restores a selection lock the package still holds.

The source-check baseline loses its nine F3 Scroll lock entries.

Expected behavior and authority

spec:AST-110 FR3 (approved, authority: draft): coordinating state lives in one store per document under a registered, versioned key, held through Core's shared-realm helpers.

Evidence

  • Red first (hooks/useScrollLock.shared.test.ts, first commit): with a lock from each of two module loads, releasing the first-opened one unlocks the page (overflow back to '') while the other still holds.
  • After: both pass, along with the hooks, gestures, selection-lock, Dialog, Drawer and BottomSheet suites (121 files, 2801 tests) and Core's tsc.
  • scripts/shared-identity.test.mjs passes with the nine entries removed.

Scope

Components: every overlay that uses useScrollLock, and every component that locks text selection during a drag

  • One defect group is restored; no API change.
  • Public text contains no internal Meta context.

Testing

  • pnpm exec vitest run --project ui packages/core/src/hooks packages/core/src/gestures packages/core/src/utils/selectionLock packages/core/src/Dialog packages/core/src/Drawer packages/core/src/BottomSheet
  • pnpm exec vitest run --project node scripts/shared-identity.test.mjs
  • tsc --noEmit -p packages/core

@vercel

vercel Bot commented Nov 9, 2026 •
edited
Loading

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 10, 2026 5:18am UTC

Request Review

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

astracat-bot Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 10, 2026, 5:26 AM UTC

@cixzhang

cixzhang commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec ed19691

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Nov 9, 2026
This was referenced Nov 9, 2026
Merged
Merged
@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch from ed19691 to d93c4de Compare November 9, 2026 16:16
@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch from 0b3df6a to 55777ee Compare November 9, 2026 16:22
@cixzhang

cixzhang commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec 798a582

@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch 2 times, most recently from 083a46c to 622e578 Compare November 9, 2026 16:44
@cixzhang

cixzhang commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec 622e578

@cixzhang cixzhang mentioned this pull request Nov 9, 2026
Merged
2 tasks done
@github-actions

github-actions Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 4cbecd6

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
LayerProvider 4.3 kB (+1 B) · 20 B CSS (0) — ✓ · ✓ · ✗
SideNav 114.9 kB (+202 B) · 11.4 kB CSS (0) — — · — · —
DateRangeInput 93.1 kB (+198 B) · 12.5 kB CSS (0) — — · — · —
179 more components
Component Size LoC Visual · A11y · RTL
charts/Chart 204.8 kB (+189 B) · 494 B CSS (0) — — · — · —
Drawer 50.9 kB (+189 B) · 8.3 kB CSS (0) — — · — · —
PowerSearch 288.7 kB (+189 B) · 24.2 kB CSS (0) — — · — · —
lab/ChatReactionBar 107.5 kB (+189 B) · 1.1 kB CSS (0) — — · — · —
DateInput 187.2 kB (+187 B) · 17.0 kB CSS (0) — — · — · —
lab/ChatEmojiPicker 106.3 kB (+184 B) · 773 B CSS (0) — — · — · —
charts/ChartTooltip 182.2 kB (+182 B) · 423 B CSS (0) — — · — · —
CommandPalette 73.5 kB (+177 B) · 12.7 kB CSS (0) — — · — · —
Lightbox 122.8 kB (+177 B) · 10.6 kB CSS (0) — — · — · —
lab/CommentComposer 152.4 kB (+176 B) · 1.9 kB CSS (0) — — · — · —
lab/Comments 352.3 kB (+175 B) · 2.5 kB CSS (0) — — · — · —
AlertDialog 105.7 kB (+172 B) · 14.6 kB CSS (0) — — · — · —
DateTimeInput 208.9 kB (+172 B) · 18.7 kB CSS (0) — — · — · —
richtext/RichTextEditorToolbar 278.9 kB (+170 B) · 454 B CSS (0) — — · — · —
lab/CommentThread 345.3 kB (+168 B) · 2.4 kB CSS (0) — — · — · —
TimeInput 162.7 kB (+165 B) · 16.0 kB CSS (0) — — · — · —
AppShell 91.7 kB (+161 B) · 13.5 kB CSS (0) — — · — · —
TopNav 72.5 kB (+158 B) · 10.9 kB CSS (0) — — · — · —
BottomSheet 68.4 kB (+155 B) · 7.7 kB CSS (0) — — · — · —
MobileNav 70.9 kB (+145 B) · 10.6 kB CSS (0) — — · — · —
BottomSheetSwitcher 18.0 kB (+143 B) · 3.4 kB CSS (0) — — · — · —
Dialog 24.1 kB (+129 B) · 6.9 kB CSS (0) — — · — · —
Selector 100.4 kB (+68 B) · 17.4 kB CSS (0) — — · — · —
Table 125.7 kB (+65 B) · 18.3 kB CSS (0) — — · — · —
lab/TourStep 97.5 kB (+65 B) · 590 B CSS (0) — — · — · —
ButtonGroup 41.6 kB (+64 B) · 3.7 kB CSS (0) — — · — · —
ContextMenu 109.0 kB (+64 B) · 17.4 kB CSS (0) — — · — · —
Carousel 70.1 kB (+60 B) · 10.9 kB CSS (0) — — · — · —
Pagination 110.5 kB (+57 B) · 17.8 kB CSS (0) — — · — · —
richtext/RichTextEditor 298.4 kB (+55 B) · 2.6 kB CSS (0) — — · — · —
lab/Schedule 106.4 kB (+54 B) · 3.9 kB CSS (0) — — · — · —
lab/NavigationStack 82.7 kB (+53 B) · 1.5 kB CSS (0) — — · — · —
Markdown 188.4 kB (+52 B) · 20.6 kB CSS (0) — — · — · —
lab/NavigationScreen 82.7 kB (+52 B) · 1.5 kB CSS (0) — — · — · —
Popover 91.1 kB (+48 B) · 14.2 kB CSS (0) — — · — · —
TableCell 110.3 kB (+47 B) · 17.9 kB CSS (0) — — · — · —
ContextMenuItem 29.3 kB (+46 B) · 6.3 kB CSS (0) — — · — · —
Switch 103.9 kB (+46 B) · 9.9 kB CSS (0) — — · — · —
ChatLayout 83.1 kB (+44 B) · 10.5 kB CSS (0) — — · — · —
CheckboxInput 64.4 kB (+44 B) · 9.2 kB CSS (0) — — · — · —
TableHeaderCell 109.8 kB (+43 B) · 17.7 kB CSS (0) — — · — · —
CheckboxListItem 71.5 kB (+42 B) · 11.4 kB CSS (0) — — · — · —
lab/TabBar 77.0 kB (+42 B) · 1.8 kB CSS (0) — — · — · —
DropdownMenu 97.3 kB (+40 B) · 17.6 kB CSS (0) — — · — · —
ListItem 70.5 kB (+40 B) · 11.4 kB CSS (0) — — · — · —
Tokenizer 91.6 kB (+40 B) · 18.8 kB CSS (0) — — · — · —
lab/PageControl 75.3 kB (+40 B) · 709 B CSS (0) — — · — · —
charts/ChartLegend 49.6 kB (+37 B) · 224 B CSS (0) — — · — · —
ToggleButton 86.9 kB (+37 B) · 10.1 kB CSS (0) — — · — · —
lab/ChatVirtualizer 55.4 kB (+37 B) · 144 B CSS (0) — — · — · —
richtext/RichTextView 276.5 kB (+37 B) · 2.3 kB CSS (0) — — · — · —
NumberInput 71.9 kB (+36 B) · 11.4 kB CSS (0) — — · — · —
Slider 109.4 kB (+36 B) · 8.8 kB CSS (0) — — · — · —
Toast 98.3 kB (+36 B) · 10.3 kB CSS (0) — — · — · —
lab/TransferList 102.3 kB (+34 B) · 1.2 kB CSS (0) — — · — · —
lab/TransferListSelector 109.8 kB (+34 B) · 1.4 kB CSS (0) — — · — · —
ToggleButtonGroup 39.2 kB (+33 B) · 3.7 kB CSS (0) — — · — · —
MoreMenu 97.6 kB (+29 B) · 17.6 kB CSS (0) — — · — · —
RadioListItem 67.6 kB (+29 B) · 10.1 kB CSS (0) — — · — · —
MultiSelector 95.2 kB (+27 B) · 16.8 kB CSS (0) — — · — · —
SegmentedControl 96.3 kB (+18 B) · 6.9 kB CSS (0) — — · — · —
lab/CodeEditor 14.4 kB (+7 B) · 743 B CSS (0) — — · — · —
DropdownMenuRadioItem 22.8 kB (+6 B) · 6.6 kB CSS (0) — — · — · —
CollapsibleGroup 6.8 kB (+3 B) · 3.3 kB CSS (0) — — · — · —
InfoTip 46.8 kB (+3 B) · 5.3 kB CSS (0) — — · — · —
Layout 7.0 kB (+3 B) · 2.3 kB CSS (0) — — · — · —
List 18.6 kB (+3 B) · 790 B CSS (0) — — · — · —
TabList 44.0 kB (+3 B) · 6.5 kB CSS (0) — — · — · —
TreeList 44.9 kB (+3 B) · 5.6 kB CSS (0) — — · — · —
lab/LineClamp 14.0 kB (+3 B) · 168 B CSS (0) — — · — · —
ChatMessageList 28.1 kB (+2 B) · 7.0 kB CSS (0) — — · — · —
CodeBlock 75.5 kB (+2 B) · 11.3 kB CSS (0) — — · — · —
DropdownMenuCheckboxItem 22.8 kB (+2 B) · 6.6 kB CSS (0) — — · — · —
SelectableCard 18.2 kB (+2 B) · 7.5 kB CSS (0) — — · — · —
SideNavSection 6.9 kB (+2 B) · 3.7 kB CSS (0) — — · — · —
Timestamp 33.2 kB (+2 B) · 10.2 kB CSS (0) — — · — · —
Token 28.5 kB (+2 B) · 4.7 kB CSS (0) — — · — · —
BreadcrumbItem 84.8 kB (+1 B) · 12.1 kB CSS (0) — — · — · —
ChatMessage 17.1 kB (+1 B) · 3.7 kB CSS (0) — — · — · —
ChatMessageBubble 7.5 kB (+1 B) · 3.7 kB CSS (0) — — · — · —
ChatToolCalls 32.0 kB (+1 B) · 8.3 kB CSS (0) — — · — · —
CommandPaletteList 16.3 kB (+1 B) · 3.4 kB CSS (0) — — · — · —
DropdownMenuItem 22.0 kB (+1 B) · 6.3 kB CSS (0) — — · — · —
Field 55.1 kB (+1 B) · 7.2 kB CSS (0) — — · — · —
MetadataList 17.1 kB (+1 B) · 3.7 kB CSS (0) — — · — · —
Spinner 24.7 kB (+1 B) · 6.2 kB CSS (0) — — · — · —
Tooltip 29.7 kB (+1 B) · 4.4 kB CSS (0) — — · — · —
TopNavItem 13.7 kB (+1 B) · 4.0 kB CSS (0) — — · — · —
Avatar 43.4 kB (0) · 5.6 kB CSS (0) — — · — · —
AvatarStatusDot 7.5 kB (0) · 3.5 kB CSS (0) — — · — · —
BaseTypeahead 71.2 kB (0) · 10.9 kB CSS (0) — — · — · —
ChatComposer 78.6 kB (0) · 12.0 kB CSS (0) — — · — · —
ChatComposerDrawer 17.9 kB (0) · 4.7 kB CSS (0) — — · — · —
ChatComposerInput 76.3 kB (0) · 11.3 kB CSS (0) — — · — · —
ChatComposerTokenElement 76.3 kB (0) · 11.3 kB CSS (0) — — · — · —
ChatMessageMetadata 20.2 kB (0) · 4.1 kB CSS (0) — — · — · —
Citation 20.5 kB (0) · 4.4 kB CSS (0) — — · — · —
ClickableCard 18.0 kB (0) · 7.3 kB CSS (0) — — · — · —
Collapsible 27.8 kB (0) · 4.5 kB CSS (0) — — · — · —
CommandPaletteFooter 17.4 kB (0) · 3.7 kB CSS (0) — — · — · —
CommandPaletteInput 31.7 kB (0) · 7.5 kB CSS (0) — — · — · —
CommandPaletteItem 13.0 kB (0) · 3.6 kB CSS (0) — — · — · —
DropdownMenuSubMenu 70.4 kB (0) · 9.7 kB CSS (0) — — · — · —
EmptyState 6.8 kB (0) · 485 B CSS (0) — — · — · —
FieldStatus 9.8 kB (0) · 3.6 kB CSS (0) — — · — · —
FormLayout 6.8 kB (0) · 3.5 kB CSS (0) — — · — · —
InputGroup 56.7 kB (0) · 7.8 kB CSS (0) — — · — · —
Item 17.5 kB (0) · 5.5 kB CSS (0) — — · — · —
LayoutFooter 6.1 kB (0) · 1.6 kB CSS (0) — — · — · —
LayoutPanel 21.3 kB (0) · 4.9 kB CSS (0) — — · — · —
MetadataListItem 6.9 kB (0) · 3.5 kB CSS (0) — — · — · —
NavHeadingMenu 16.3 kB (0) · 3.5 kB CSS (0) — — · — · —
Overlay 22.3 kB (0) · 3.9 kB CSS (0) — — · — · —
ResizeHandle 21.5 kB (0) · 4.3 kB CSS (0) — — · — · —
StatusDot 31.4 kB (0) · 5.3 kB CSS (0) — — · — · —
Step 30.9 kB (0) · 5.6 kB CSS (0) — — · — · —
TableRow 7.0 kB (0) · 4.0 kB CSS (0) — — · — · —
Toolbar 43.9 kB (0) · 8.1 kB CSS (0) — — · — · —
TopNavMegaMenu 69.9 kB (0) · 11.8 kB CSS (0) — — · — · —
TopNavMenu 74.7 kB (0) · 11.2 kB CSS (0) — — · — · —
lab/ChatTypingIndicator 25.6 kB (0) · 612 B CSS (0) — — · — · —
lab/DialogHeroHeader 83.4 kB (0) · 372 B CSS (0) — — · — · —
lab/ListInput 88.4 kB (0) · 922 B CSS (0) — — · — · —
lab/TabBarItem 3.8 kB (0) · 1.8 kB CSS (0) — — · — · —
AvatarGroup 46.1 kB (−1 B) · 5.7 kB CSS (0) — — · — · —
Breadcrumbs 16.4 kB (−1 B) · 3.4 kB CSS (0) — — · — · —
CheckboxList 68.6 kB (−1 B) · 7.7 kB CSS (0) — — · — · —
DropdownMenuRadioGroup 6.5 kB (−1 B) · 3.3 kB CSS (0) — — · — · —
HoverCard 28.0 kB (−1 B) · 4.4 kB CSS (0) — — · — · —
Icon 8.0 kB (−1 B) · 681 B CSS (0) — — · — · —
LinkProvider 3.6 kB (−1 B) · 20 B CSS (0) — — · — · —
Outline 24.8 kB (−1 B) · 4.1 kB CSS (0) — — · — · —
ProgressBar 11.7 kB (−1 B) · 6.3 kB CSS (0) — — · — · —
RadioList 56.5 kB (−1 B) · 7.3 kB CSS (0) — — · — · —
SelectorOption 20.9 kB (−1 B) · 6.1 kB CSS (0) — — · — · —
StackItem 16.3 kB (−1 B) · 219 B CSS (0) — — · — · —
Tab 14.4 kB (−1 B) · 4.2 kB CSS (0) — — · — · —
Text 11.8 kB (−1 B) · 5.4 kB CSS (0) — — · — · —
TopNavMegaMenuItem 13.7 kB (−1 B) · 4.2 kB CSS (0) — — · — · —
lab/CommentPin 57.3 kB (−1 B) · 1.2 kB CSS (0) — — · — · —
AvatarGroupOverflow 49.1 kB (−2 B) · 5.6 kB CSS (0) — — · — · —
Banner 62.4 kB (−2 B) · 10.5 kB CSS (0) — — · — · —
CheckIndicator 8.2 kB (−2 B) · 681 B CSS (0) — — · — · —
FieldLabel 51.2 kB (−2 B) · 5.7 kB CSS (0) — — · — · —
FileInput 69.3 kB (−2 B) · 10.9 kB CSS (0) — — · — · —
Heading 12.4 kB (−2 B) · 5.4 kB CSS (0) — — · — · —
Kbd 17.1 kB (−2 B) · 3.6 kB CSS (0) — — · — · —
LayoutContent 17.7 kB (−2 B) · 1.7 kB CSS (0) — — · — · —
LayoutHeader 6.2 kB (−2 B) · 1.6 kB CSS (0) — — · — · —
TableSelectionToolbar 70.7 kB (−2 B) · 13.2 kB CSS (0) — — · — · —
Timer 14.0 kB (−2 B) · 6.0 kB CSS (0) — — · — · —
TopNavMegaMenuFeaturedCard 7.3 kB (−2 B) · 3.6 kB CSS (0) — — · — · —
TextArea 62.5 kB (−3 B) · 9.3 kB CSS (0) — — · — · —
Thumbnail 64.8 kB (−3 B) · 11.6 kB CSS (0) — — · — · —
lab/SelectableCardGroup 64.6 kB (−3 B) · 162 B CSS (0) — — · — · —
Typeahead 79.7 kB (−4 B) · 12.4 kB CSS (0) — — · — · —
SegmentedControlItem 17.3 kB (−6 B) · 4.3 kB CSS (0) — — · — · —
Calendar 71.4 kB (−7 B) · 10.6 kB CSS (0) — — · — · —
ComplexSelector 71.1 kB (−7 B) · 11.8 kB CSS (0) — — · — · —
Stepper 62.0 kB (−7 B) · 10.1 kB CSS (0) — — · — · —
DrawerHeader 63.4 kB (−8 B) · 12.9 kB CSS (0) — — · — · —
ChatSendButton 60.8 kB (−9 B) · 10.0 kB CSS (0) — — · — · —
IconButton 60.5 kB (−9 B) · 10.0 kB CSS (0) — — · — · —
SideNavItem 70.6 kB (−9 B) · 11.1 kB CSS (0) — — · — · —
TopNavHeading 75.3 kB (−9 B) · 11.2 kB CSS (0) — — · — · —
Button 60.4 kB (−10 B) · 10.0 kB CSS (0) — — · — · —
DialogHeader 63.2 kB (−10 B) · 11.5 kB CSS (0) — — · — · —
MobileNavToggle 60.7 kB (−10 B) · 10.0 kB CSS (0) — — · — · —
SideNavCollapseButton 61.2 kB (−10 B) · 10.0 kB CSS (0) — — · — · —
TabMenu 71.9 kB (−10 B) · 11.1 kB CSS (0) — — · — · —
lab/EmojiPicker 87.1 kB (−11 B) · 751 B CSS (0) — — · — · —
ChatDictationButton 61.0 kB (−12 B) · 10.0 kB CSS (0) — — · — · —
ChatLayoutScrollButton 60.9 kB (−12 B) · 10.2 kB CSS (0) — — · — · —
Link 53.2 kB (−12 B) · 7.0 kB CSS (0) — — · — · —
lab/DiffViewer 87.6 kB (−12 B) · 2.1 kB CSS (0) — — · — · —
TextInput 67.9 kB (−15 B) · 11.2 kB CSS (0) — — · — · —
SideNavHeading 75.7 kB (−16 B) · 11.3 kB CSS (0) — — · — · —
layer — — — · ✓ · —
Layer — — ✓ · ✓ · —

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

API changes: none in this pull request

Committed API report behind this build: cli 29 rows, core 36 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[bot]
astracat-bot Bot requested changes Nov 9, 2026
View reviewed changes

@astracat-bot astracat-bot Bot left a comment •
edited
Loading

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 scroll-lock change matches spec:AST-110 FR3, and its new test fails before the fix and passes after it, but the release note and description misstate what it fixes:

  • The changeset (.changeset/scroll-lock-swizzle-shared.md) and the User impact section say that closing a package Dialog over an open swizzled Drawer let the page scroll and that the page could stay overflow: hidden after both close, but on main both cases already behave correctly; name the case this fixes, closing the overlay that opened first while the other is still open, and drop the stuck-hidden sentence.
  • Miss: the title, changeset and User impact say that swizzling an overlay splits the lock, but a swizzled Dialog, Drawer, MobileNav or BottomSheet imports useScrollLock from @astryxdesign/core/hooks and already shares the package's lock on main, so describe the case as two loads of Core's scroll-lock modules on one page (for example two installed copies of Core).

This review covers this pull request's own commits; the spec:AST-110 wording raised earlier belongs to #10807 and #10817 and is judged with them. A pull request in this stack lands only when it and every pull request below it are approved.

[Automated review]

Comment thread docs/specs/AST-110-swizzle-output/spec.md Resolved
Comment thread docs/specs/AST-110-swizzle-output/spec.md Resolved
Comment thread .changeset/scroll-lock-swizzle-shared.md Resolved
Comment thread docs/specs/AST-110-swizzle-output/spec.md Resolved
@cixzhang
cixzhang marked this pull request as draft November 10, 2026 03:15
@cixzhang
cixzhang marked this pull request as ready for review November 10, 2026 03:15
@cixzhang cixzhang mentioned this pull request Nov 10, 2026
Merged
2 tasks done
@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch from 622e578 to 64ea212 Compare November 10, 2026 03:43
@cixzhang

cixzhang commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec 64ea212

astracat-bot[bot]
astracat-bot Bot requested changes Nov 10, 2026
View reviewed changes

@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 shared scroll-lock change matches spec:AST-110 FR3 and component:Drawer FR5, and its new test fails before the fix and passes after it, but the title, release note and description still misstate what it fixes:

  • Miss: the title, changeset and User impact say that swizzling an overlay splits the page scroll lock, but swizzled Dialog, Drawer, BottomSheet, MobileNav and Lightbox copies import useScrollLock from @astryxdesign/core/hooks and already share the package's lock, so describe the case as two loads of Core's scroll-lock modules on one page (for example two installed copies of Core).
  • The changeset (.changeset/scroll-lock-swizzle-shared.md) says closing a package Dialog over an open swizzled Drawer (or the reverse) let the page scroll, and the User impact says the page could keep overflow: hidden after both close, but before this change closing the overlay on top already kept the page locked and both orders ended with the page's own overflow, so name the case this fixes, closing the overlay that opened first while the other is still open, and drop the stuck-hidden sentence.

This review covers this pull request's own commits; #10948 and #10817 are judged in their own reviews. A pull request in this stack lands only when it and every pull request below it are approved.

[Automated review]

Comment thread .changeset/scroll-lock-swizzle-shared.md Resolved
Comment thread packages/core/src/hooks/rootScrollStyles.ts
Comment on lines +71 to +72
// Every copy of Core uses this one state, so a swizzled overlay and the
// package's stack their claims and locks on the same page.

@astracat-bot astracat-bot Bot Nov 10, 2026

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: a swizzled overlay already reaches this module through the package's useScrollLock, so this is not the reason for the shared state. Shortest rewrite: "Every copy of Core uses this one state, so locks from two copies stack on one page."

@cixzhang
test(core): each shared-realm shape gives the package and a copy one …
8dd1933 
…value (red)
@cixzhang
feat(core): shared-realm helpers hold every kind of cross-module state
6392e53 
sharedRealm.ts gains the shapes the FR3 fixes need, each a thin layer
over realmValue and sharedKey: sharedState for counters, flags and host
objects; sharedMap, sharedWeakMap and sharedWeakSet for registries and
per-document stores; sharedStore for a value and its listeners;
sharedBrand and the SharedBrand type for brands; sharedSequence for id
counters; and resetSharedStateForTest. The source check asserts that
every shared slot is named with a literal name and version, once. AST-110
FR3 states that cross-module state is held through these helpers.
@cixzhang
chore(changeset): note the internal shared-realm helpers
460632c
@cixzhang
feat(core): sharedForReact keeps a React-bound shared value one per R…
1677454 
…eact
@cixzhang
fix(core): reset shared state without a dynamic delete
9aca48d
@cixzhang
fix(core): a shared brand's type member is required
53b6023
@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch from 64ea212 to 33ab480 Compare November 10, 2026 04:37
@cixzhang

cixzhang commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec 33ab480

astracat-bot[bot]
astracat-bot Bot requested changes Nov 10, 2026
View reviewed changes

@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 shared scroll-lock change matches spec:AST-110 FR3 and component:Drawer FR5, and its new test fails before the fix and passes after it, but the title, release note and description still misstate what it fixes:

  • Miss: the title, changeset and User impact say that swizzling an overlay splits the page scroll lock, but swizzled Dialog, Drawer, BottomSheet, Lightbox and MobileNav copies import useScrollLock from @astryxdesign/core/hooks and already share the package's lock, so describe the case as two loads of Core's scroll-lock modules on one page (for example two installed copies of Core).
  • The changeset (.changeset/scroll-lock-swizzle-shared.md) says closing a package Dialog over an open swizzled Drawer (or the reverse) let the page scroll, and the User impact says the page could keep overflow: hidden after both close, but before this change closing the overlay on top already kept the page locked and both orders ended with the page's own overflow, so name the case this fixes, closing the overlay that opened first while the other is still open, and drop the stuck-hidden sentence.

This review covers this pull request's own commits; #10817 is judged in its own review. A pull request in this stack lands only when it and every pull request below it are approved.

[Automated review]

@cixzhang
cixzhang force-pushed the fix/scroll-lock-shared branch from 33ab480 to 4cbecd6 Compare November 10, 2026 05:07
@cixzhang cixzhang changed the title fix(core): a swizzled overlay and the package's share one page scroll lock (AST-110 FR3) fix(core): page scroll and selection locks are one per page across Core loads (AST-110 FR3) Nov 10, 2026
@cixzhang

cixzhang commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

/approve-spec 4cbecd6

astracat-bot[bot]
astracat-bot Bot approved these changes Nov 10, 2026
View reviewed changes

@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.

Approved pending CI: CI's test, build and docsite-test jobs are deferred on this head and run before merge.

Sharing the page scroll lock's state per page restores page scroll locking when Core loads twice on one page (component:Drawer FR5): the new test fails before the fix and passes after it, and the title, changeset and description now describe that case accurately.

Non-blocking:

  • Add a two-load test for the selection lock, which this change also fixes but no test covers.
  • The comment at packages/core/src/hooks/rootScrollStyles.ts lines 71-72 and the new test's title still credit a swizzled overlay, which already shares the package's lock.

This review covers this pull request's own commits; #10817 is judged in its own review. A pull request in this stack lands only when it and every pull request below it are approved.

[Automated review]

@cixzhang
cixzhang marked this pull request as draft November 10, 2026 06:21
@cixzhang
cixzhang marked this pull request as ready for review November 10, 2026 06:21
@cixzhang
cixzhang merged commit 0840fcc into main Nov 10, 2026
127 of 131 checks passed
@github-actions
github-actions Bot deleted the fix/scroll-lock-shared branch November 11, 2026 06:05

This branch was successfully deployed

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

Reviewers

@astracat-bot astracat-bot[bot] astracat-bot[bot] approved these changes
@imdreamrunner imdreamrunner Awaiting requested review from imdreamrunner imdreamrunner is a code owner
@josephfarina josephfarina Awaiting requested review from josephfarina josephfarina is a code owner
@cvkxx cvkxx Awaiting requested review from cvkxx
@ernestt ernestt Awaiting requested review from ernestt
@kentonquatman kentonquatman Awaiting requested review from kentonquatman
@rubyycheung rubyycheung Awaiting requested review from rubyycheung

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@cixzhang