Repository navigation
Conversation
…ointercancel A browser can read a finger that pans over the page as a tap, with no pointercancel, and its popover="auto" light dismiss then closes an open DropdownMenu or Popover. An open auto layer now cancels the lift (touchend) of a touch that started outside it and travelled the shared pan distance, so the browser makes no tap, no click and no light dismiss. A tap outside still closes the layer; a lift that ends a scroll is not cancelable, and a cancelled lift keeps the fling. Manual layers (ContextMenu) are unchanged. (architecture:layer-runtime INV7)
cixzhang
added
the
ci:run-a11y
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| SideNavItem | 73.8 kB (+1.4 kB) · 11.2 kB CSS (0) | — | — · — · — |
| ComplexSelector | 74.3 kB (+1.4 kB) · 11.9 kB CSS (0) | — | — · — · — |
| TopNavMegaMenu | 73.0 kB (+1.3 kB) · 11.9 kB CSS (0) | — | — · — · — |
150 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| lab/ChatEmojiPicker | 109.2 kB (+1.0 kB) · 773 B CSS (0) | — | — · — · — |
| MultiSelector | 98.0 kB (+1.0 kB) · 16.9 kB CSS (0) | — | — · — · — |
| lab/TourStep | 100.2 kB (+1.0 kB) · 590 B CSS (0) | — | — · — · — |
| Popover · Storybook | 94.0 kB (+1020 B) · 14.2 kB CSS (0) | 1,556 (+5) | — · ✓ · — |
| Selector | 103.3 kB (+1001 B) · 17.4 kB CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 110.3 kB (+1000 B) · 1.1 kB CSS (0) | — | — · — · — |
| Pagination | 113.4 kB (+987 B) · 17.8 kB CSS (0) | — | — · — · — |
| MoreMenu | 100.3 kB (+980 B) · 17.6 kB CSS (0) | — | — · — · — |
| DateRangeInput | 95.9 kB (+978 B) · 12.5 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 112.8 kB (+976 B) · 1.4 kB CSS (0) | — | — · — · — |
| DropdownMenu | 100.0 kB (+972 B) · 17.6 kB CSS (0) | — | — · ✓ · — |
| TabMenu | 74.8 kB (+955 B) · 11.2 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 281.9 kB (+955 B) · 454 B CSS (0) | — | — · — · — |
| TopNavHeading | 78.1 kB (+954 B) · 11.3 kB CSS (0) | — | — · — · — |
| SideNavHeading | 78.5 kB (+953 B) · 11.3 kB CSS (0) | — | — · — · — |
| TopNavMenu | 77.5 kB (+931 B) · 11.3 kB CSS (0) | — | — · ✓ · — |
| BreadcrumbItem | 87.6 kB (+915 B) · 12.1 kB CSS (0) | — | — · ✓ · — |
| BaseTypeahead | 74.6 kB (+794 B) · 11.0 kB CSS (0) | — | — · — · — |
| ChatComposerInput | 78.9 kB (+761 B) · 11.3 kB CSS (0) | — | — · — · — |
| Typeahead | 83.4 kB (+760 B) · 12.5 kB CSS (0) | — | — · — · — |
| ChatComposer | 81.2 kB (+759 B) · 12.1 kB CSS (0) | — | — · — · — |
| ChatComposerTokenElement | 78.9 kB (+759 B) · 11.3 kB CSS (0) | — | — · — · — |
| PowerSearch | 293.1 kB (+406 B) · 24.2 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 154.7 kB (+405 B) · 1.9 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 348.5 kB (+399 B) · 2.4 kB CSS (0) | — | — · — · — |
| DateInput | 189.8 kB (+398 B) · 17.1 kB CSS (0) | — | — · — · — |
| lab/Comments | 355.5 kB (+383 B) · 2.5 kB CSS (0) | — | — · — · — |
| DateTimeInput | 211.5 kB (+381 B) · 18.7 kB CSS (0) | — | — · — · — |
| Tokenizer | 94.8 kB (+364 B) · 18.8 kB CSS (0) | — | — · — · — |
| Markdown | 190.9 kB (+153 B) · 20.8 kB CSS (0) | — | — · — · — |
| Toolbar | 45.3 kB (+144 B) · 8.1 kB CSS (0) | — | — · — · — |
| TableCell | 112.3 kB (+136 B) · 17.9 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 111.9 kB (+135 B) · 17.7 kB CSS (0) | — | — · — · — |
| FileInput | 71.0 kB (+130 B) · 11.0 kB CSS (0) | — | — · — · — |
| ContextMenu | 111.1 kB (+129 B) · 17.5 kB CSS (0) | — | — · — · — |
| Slider | 111.0 kB (+127 B) · 8.8 kB CSS (0) | — | — · — · — |
| lab/Schedule | 108.1 kB (+127 B) · 3.9 kB CSS (0) | — | — · — · — |
| SegmentedControl | 98.0 kB (+123 B) · 6.9 kB CSS (0) | — | — · — · — |
| Table | 127.9 kB (+123 B) · 18.4 kB CSS (0) | — | — · — · — |
| Toast | 99.9 kB (+123 B) · 10.3 kB CSS (0) | — | — · — · — |
| RadioListItem | 69.2 kB (+120 B) · 10.2 kB CSS (0) | — | — · — · — |
| lab/DialogHeroHeader | 85.0 kB (+117 B) · 372 B CSS (0) | — | — · — · — |
| MobileNav | 72.6 kB (+116 B) · 10.6 kB CSS (0) | — | — · — · — |
| Carousel | 71.8 kB (+115 B) · 11.0 kB CSS (0) | — | — · — · — |
| ChatDictationButton | 62.6 kB (+115 B) · 10.1 kB CSS (0) | — | — · — · — |
| HoverCard | 29.3 kB (+115 B) · 4.4 kB CSS (0) | — | — · — · — |
| MobileNavToggle | 62.3 kB (+115 B) · 10.0 kB CSS (0) | — | — · — · — |
| ChatSendButton | 62.4 kB (+114 B) · 10.0 kB CSS (0) | — | — · — · — |
| Drawer | 52.5 kB (+114 B) · 8.3 kB CSS (0) | — | — · — · — |
| Lightbox | 124.6 kB (+114 B) · 10.6 kB CSS (0) | — | — · — · — |
| SideNavCollapseButton | 62.9 kB (+114 B) · 10.1 kB CSS (0) | — | — · — · — |
| ChatLayoutScrollButton | 62.5 kB (+113 B) · 10.2 kB CSS (0) | — | — · — · — |
| ListItem | 72.2 kB (+112 B) · 11.5 kB CSS (0) | — | — · — · — |
| IconButton | 62.1 kB (+111 B) · 10.0 kB CSS (0) | — | — · — · — |
| AvatarGroupOverflow | 50.6 kB (+110 B) · 5.7 kB CSS (0) | — | — · — · — |
| InfoTip | 48.3 kB (+110 B) · 5.4 kB CSS (0) | — | — · — · — |
| lab/ListInput | 90.0 kB (+110 B) · 922 B CSS (0) | — | — · — · — |
| AppShell | 93.7 kB (+109 B) · 13.5 kB CSS (0) | — | — · — · — |
| Banner | 64.0 kB (+109 B) · 10.5 kB CSS (0) | — | — · — · — |
| DrawerHeader | 65.0 kB (+109 B) · 13.0 kB CSS (0) | — | — · — · — |
| TextArea | 64.2 kB (+109 B) · 9.4 kB CSS (0) | — | — · — · — |
| lab/TransferList | 104.2 kB (+109 B) · 1.2 kB CSS (0) | — | — · — · — |
| Button | 62.0 kB (+108 B) · 10.0 kB CSS (0) | — | — · — · — |
| DropdownMenuSubMenu | 72.1 kB (+108 B) · 9.7 kB CSS (0) | — | — · ✓ · — |
| Switch | 105.5 kB (+107 B) · 10.0 kB CSS (0) | — | — · — · — |
| lab/CommentPin | 59.0 kB (+107 B) · 1.2 kB CSS (0) | — | — · — · — |
| AvatarGroup | 47.7 kB (+105 B) · 5.8 kB CSS (0) | — | — · — · — |
| DialogHeader | 64.8 kB (+105 B) · 11.5 kB CSS (0) | — | — · — · — |
| TableSelectionToolbar | 72.3 kB (+104 B) · 13.3 kB CSS (0) | — | — · — · — |
| TabList | 45.4 kB (+104 B) · 6.5 kB CSS (0) | — | — · ✓ · — |
| lab/DiffViewer | 89.2 kB (+104 B) · 2.1 kB CSS (0) | — | — · — · — |
| RadioList | 58.1 kB (+103 B) · 7.3 kB CSS (0) | — | — · — · — |
| Calendar | 73.0 kB (+101 B) · 10.6 kB CSS (0) | — | — · — · — |
| Thumbnail | 66.4 kB (+101 B) · 11.6 kB CSS (0) | — | — · — · — |
| TopNav | 74.3 kB (+101 B) · 11.0 kB CSS (0) | — | — · ✓ · — |
| lab/TabBarMoreList | 94.3 kB (+101 B) · 1.8 kB CSS (0) | — | — · — · — |
| charts/Chart | 207.2 kB (+100 B) · 516 B CSS (0) | — | — · — · — |
| CheckboxListItem | 73.1 kB (+100 B) · 11.5 kB CSS (0) | — | — · — · — |
| Link | 54.8 kB (+100 B) · 7.1 kB CSS (0) | — | — · — · — |
| NumberInput | 73.6 kB (+98 B) · 11.5 kB CSS (0) | — | — · — · — |
| lab/EmojiPicker | 88.8 kB (+98 B) · 751 B CSS (0) | — | — · — · — |
| Stepper | 63.7 kB (+96 B) · 10.2 kB CSS (0) | — | — · — · — |
| Field | 56.7 kB (+95 B) · 7.3 kB CSS (0) | — | — · — · — |
| StatusDot | 32.8 kB (+95 B) · 5.4 kB CSS (0) | — | — · — · — |
| ChatLayout | 84.8 kB (+94 B) · 10.6 kB CSS (0) | — | — · — · — |
| CodeBlock | 77.1 kB (+94 B) · 11.3 kB CSS (0) | — | — · — · — |
| InputGroup | 58.3 kB (+94 B) · 7.9 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditor | 300.3 kB (+94 B) · 2.6 kB CSS (0) | — | — · — · — |
| SideNav | 116.8 kB (+93 B) · 11.4 kB CSS (0) | — | — · ✓ · — |
| richtext/RichTextView | 278.5 kB (+93 B) · 2.3 kB CSS (0) | — | — · — · — |
| AlertDialog | 107.5 kB (+92 B) · 14.6 kB CSS (0) | — | — · — · — |
| Avatar | 44.9 kB (+92 B) · 5.6 kB CSS (0) | — | — · — · — |
| charts/ChartTooltip | 184.3 kB (+87 B) · 451 B CSS (0) | — | — · — · — |
| Tooltip | 31.1 kB (+87 B) · 4.4 kB CSS (0) | — | — · — · — |
| TextInput | 69.6 kB (+82 B) · 11.2 kB CSS (0) | — | — · — · — |
| lab/SelectableCardGroup | 66.1 kB (+81 B) · 162 B CSS (0) | — | — · — · — |
| CheckboxList | 70.5 kB (+80 B) · 7.8 kB CSS (0) | — | — · — · — |
| FieldLabel | 52.8 kB (+79 B) · 5.7 kB CSS (0) | — | — · — · — |
| TimeInput | 164.7 kB (+77 B) · 16.1 kB CSS (0) | — | — · — · — |
| ToggleButton | 88.5 kB (+76 B) · 10.1 kB CSS (0) | — | — · — · — |
| CheckboxInput | 66.1 kB (+74 B) · 9.2 kB CSS (0) | — | — · — · — |
| charts/ChartLegend | 53.5 kB (+55 B) · 224 B CSS (0) | — | — · — · — |
| ChatMessageList | 29.9 kB (+47 B) · 7.1 kB CSS (0) | — | — · — · — |
| CommandPaletteInput | 33.6 kB (+46 B) · 7.6 kB CSS (0) | — | — · — · — |
| lab/LineClamp | 15.7 kB (+46 B) · 168 B CSS (0) | — | — · — · — |
| Text | 13.5 kB (+41 B) · 5.5 kB CSS (0) | — | — · — · — |
| Heading | 14.1 kB (+40 B) · 5.5 kB CSS (0) | — | — · — · — |
| ChatToolCalls | 33.9 kB (+38 B) · 8.4 kB CSS (0) | — | — · — · — |
| Timestamp | 34.8 kB (+37 B) · 10.3 kB CSS (0) | — | — · — · — |
| Spinner | 26.5 kB (+33 B) · 6.2 kB CSS (0) | — | — · — · — |
| Timer | 15.7 kB (+33 B) · 6.0 kB CSS (0) | — | — · — · — |
| CommandPalette | 74.9 kB (+31 B) · 12.7 kB CSS (0) | — | — · — · — |
| lab/ChatVirtualizer | 55.5 kB (+2 B) · 144 B CSS (0) | — | — · — · — |
| CommandPaletteItem | 13.1 kB (+1 B) · 3.6 kB CSS (0) | — | — · — · — |
| DropdownMenuRadioItem | 23.4 kB (+1 B) · 6.6 kB CSS (0) | — | — · ✓ · — |
| ToggleButtonGroup | 39.4 kB (+1 B) · 3.7 kB CSS (0) | — | — · — · — |
| BottomSheet | 69.2 kB (0) · 7.7 kB CSS (0) | — | — · — · — |
| Breadcrumbs | 16.5 kB (0) · 3.4 kB CSS (0) | — | — · ✓ · — |
| ClickableCard | 18.1 kB (0) · 7.3 kB CSS (0) | — | — · — · — |
| Collapsible | 28.5 kB (0) · 4.5 kB CSS (0) | — | — · — · — |
| Dialog | 24.9 kB (0) · 6.9 kB CSS (0) | — | — · — · — |
| DropdownMenuCheckboxItem | 23.4 kB (0) · 6.6 kB CSS (0) | — | — · — · — |
| DropdownMenuDivider | 5.3 kB (0) · 548 B CSS (0) | — | — · ✓ · — |
| DropdownMenuGroup | 6.4 kB (0) · 3.4 kB CSS (0) | — | — · ✓ · — |
| DropdownMenuItem | 22.6 kB (0) · 6.3 kB CSS (0) | — | — · ✓ · — |
| DropdownMenuRadioGroup | 6.6 kB (0) · 3.3 kB CSS (0) | — | — · ✓ · — |
| LayerProvider | 4.4 kB (0) · 20 B CSS (0) | — | — · — · — |
| Outline | 24.9 kB (0) · 4.1 kB CSS (0) | — | — · — · — |
| ProgressBar | 12.0 kB (0) · 6.4 kB CSS (0) | — | — · — · — |
| SegmentedControlItem | 17.5 kB (0) · 4.3 kB CSS (0) | — | — · — · — |
| SelectableCard | 18.3 kB (0) · 7.5 kB CSS (0) | — | — · — · — |
| SelectorOption | 21.2 kB (0) · 6.1 kB CSS (0) | — | — · — · — |
| SideNavSection | 7.1 kB (0) · 3.7 kB CSS (0) | — | — · — · — |
| Step | 31.2 kB (0) · 5.6 kB CSS (0) | — | — · — · — |
| Tab | 14.5 kB (0) · 4.2 kB CSS (0) | — | — · — · — |
| Token | 28.7 kB (0) · 4.7 kB CSS (0) | — | — · — · — |
| TopNavItem | 14.1 kB (0) · 4.0 kB CSS (0) | — | — · — · — |
| TopNavMegaMenuFeaturedCard | 7.4 kB (0) · 3.6 kB CSS (0) | — | — · — · — |
| TopNavMegaMenuItem | 13.8 kB (0) · 4.2 kB CSS (0) | — | — · — · — |
| TreeList | 45.7 kB (0) · 5.6 kB CSS (0) | — | — · — · — |
| lab/NavigationScreen | 83.4 kB (0) · 1.5 kB CSS (0) | — | — · — · — |
| lab/NavigationStack | 83.4 kB (0) · 1.5 kB CSS (0) | — | — · — · — |
| lab/PageControl | 75.6 kB (0) · 709 B CSS (0) | — | — · — · — |
| lab/TabBar | 81.4 kB (0) · 1.8 kB CSS (0) | — | — · — · — |
| lab/TabBarItem | 3.9 kB (0) · 1.8 kB CSS (0) | — | — · — · — |
| Item | 17.6 kB (−1 B) · 5.5 kB CSS (0) | — | — · — · — |
| NavHeadingMenu | 16.9 kB (−1 B) · 3.5 kB CSS (0) | — | — · — · — |
| BottomSheetSwitcher | 18.6 kB (−2 B) · 3.4 kB CSS (0) | — | — · — · — |
| ButtonGroup | 42.2 kB (−2 B) · 3.7 kB CSS (0) | — | — · — · — |
| ContextMenuItem | 32.9 kB (−2 B) · 6.3 kB CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 7.30 MB packed (+4.1 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 67 rows, core 36 rows; run pnpm api:report.
Wide change (7 components): add ci:run-visual or ci:run-rtl to run them.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the intended behavior, and four changes are needed either way.
Decision needed: no current record says that a pan which the browser turns into a tap must leave a Popover, or another light-dismiss layer, open. architecture:layer-runtime INV7 covers only the same press reopening a layer, and spec:AST-094 FR32 says a Popover's dismissal is unchanged. An engineering owner records the decision by commenting /approve-spec with the full head commit SHA on this pull request, or by amending the governing record; the author may do so as an engineering owner.
Required changes:
- In Chromium 149, the build
@playwright/test1.61.1 runs, the new cases "a pan that the browser reads as a tap leaves it open" and "a 40 px pan with no pointercancel leaves it open" fail on this branch exactly as on main, because the layer closes atpointerupbefore thetouchendthat the guard cancels; keeping the release on the layer with pointer capture, asclaimPressdoes, as well as cancelling the tap would cover that order. - DropdownMenu must close as a pan outside begins (
spec:AST-094FR32,component:DropdownMenuFR21), so the guard and its "leaves it open" cases contradict a current requirement; keep menus out of the guard and mark those cases as the FR21 gap, as the ContextMenu cases already are, unless the records are amended. - The guard re-implements "a press outside that begins to pan" from
watchOutsidePress(Layer/outsidePress.ts) and thetouchendcancel fromarmClickGuard(utils/clickGuard.ts); extend those shared paths instead of adding a second one. - The changeset says a fling no longer closes "an open menu or popover", but ContextMenu still closes on the same pan and a fling that scrolls already left both components open, so name only the case and components that change.
Minor: isShownLayer returns true when :popover-open is unsupported, the opposite of its comment, so without the Popover API the guard keeps running after the layer closes.
[Automated review]
…NV7) A touch that starts outside an open auto layer and travels the shared pan distance is now claimed for the layer through Layer's outside-press watch (new onPan report) and claimPress (new cancelsTap). The pointer capture puts the release on the layer, for a browser that light-dismisses at pointerup, and the click guard cancels the touchend, for a browser that light-dismisses from the tap. The bespoke touch guard is removed. DropdownMenu opts out through the internal ownsOutsidePan option: an anchored menu closes as a pan outside begins (spec:AST-094 FR32), so its pan cases are fixme for the FR21 gap. architecture:layer-runtime INV7 states the rule, and its verification row lists the tests. The changeset names popovers and selector lists and the click it stops.
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the new architecture:layer-runtime INV7 sentence, and three changes are needed either way.
Decision needed: the head adds to INV7 that a one-finger touch that starts outside an open layer and pans 10 px is a gesture, not an outside press, even when the browser reads it as a tap, while spec:AST-094 FR32 still says a Popover's dismissal is unchanged. An engineering owner records the decision by commenting /approve-spec with the full head SHA (the author may do so as an engineering owner), or says what INV7 should state instead.
Required changes:
- A list row swiped while a Popover is open stops following the finger after about 10 px and stays part-way open, because the layer's claim takes the pointer the row captured (
useLayer.tsx:1350), againstspec:AST-057FR4 and FR5; leave a touch that a page control has captured to that control. - TabMenu and the Breadcrumbs overflow menu, both anchored menus built on
usePopover, now stay open after a 12 px pan beside them (base closes them), againstspec:AST-094FR32 and the new INV7 sentence; opt them out withownsOutsidePanas DropdownMenu does and cover them in the browser spec. - The changeset says a fling over the page no longer closes a popover or selector list, but a fling that scrolls the page already left them open; name the pan that changes, one the browser does not take as a scroll.
The pointer-capture rework fixes the earlier Chromium failure: the new browser spec passes on this head, and the DropdownMenu and shared-path points are resolved.
[Automated review]
|
/approve-spec 3230c70 |
github-actions
Bot
removed
the
needs:spec-owner-review
…n (INV7) TabMenu, BreadcrumbItem's menu, TopNavMenu, TopNavHeading's menu and SideNavHeading's menu render role="menu" on a Popover, so like DropdownMenu they opt out of the pan guard through the internal ownsOutsidePan option: an anchored menu closes as a pan outside begins (spec:AST-094 FR32). Each gets a fixme browser case for that gap. Navigation disclosures without role="menu" (TopNavMegaMenu, SideNavItem's flyout) keep Popover's rule and the guard. useLayer.test.tsx covers the claim and the opt-out.
There was a problem hiding this comment.
Changes are needed before approval. The new architecture:layer-runtime INV7 sentence is owner-approved at this head, and the opt-out now covers TabMenu, the Breadcrumbs menu, TopNavMenu, TopNavHeading and SideNavHeading as well as DropdownMenu.
Required changes:
- A list row swiped while a Popover is open stops following the finger after about 10 px and rests 24 px open, because the layer's claim takes the pointer the row captured (
useLayer.tsx:1350), againstspec:AST-057FR4 and FR5; leave a touch that a page control has captured to that control. - The changeset says a fling over the page no longer closes a popover or selector list, but a fling that scrolls the page already left them open, so it should name the pan that changes: one the browser does not take as a scroll.
- The description's table lists DateInput as guarded, but DateInput's calendar and DateTimeInput's time list set
hasLightDismiss: falseand still close on such a pan through their own outside-press watch, so drop them from that row or say that they still close.
Non-blocking: the five menus added at this head have only test.fixme browser cases, so a live "still closes" case for them would catch a lost opt-out.
[Automated review]
|
/approve-spec 2191fe7 |
A list row swiped while a Popover was open stopped after about 10 px and rested 20 px open: the layer's pan claim took back the pointer the row had captured (spec:AST-057 FR4, FR5). A claim now ends when another control takes the pointer (gotpointercapture), and it neither takes the pointer back nor cancels the tap. A test-only story (Core/Popover, Over Swipe Rows) carries the new Chromium case, which fails at the previous head and passes here. The five newly opted-out menus each get a live case: a pan the browser reads as a tap still closes them. The changeset names the pan that changes.
There was a problem hiding this comment.
Changes are needed before approval. The swipe-row cut-off, the changeset wording and the description's DateInput row are fixed, and the five other menus now have live "still closes" cases.
Required change:
- A ResizeHandle dragged by a finger while a Popover is open stops after about 16 px of an 80 px drag and the Popover stays open, because the handle captured the pointer at
pointerdown, before the layer's claim, andyieldToTakeronly listens from the claim on (gestureCounter.ts:181,:244-252), againstcomponent:ResizableFR3; leave a press that an element outside the layer already holds to that element, and add a browser case for it.
Non-blocking: the branch now conflicts with main in the verified_by list of docs/architecture/layer-runtime.md; keep both entries when rebasing.
[Automated review]
# Conflicts: # docs/architecture/layer-runtime.md
… keeps its drag A ResizeHandle dragged by a finger while a Popover was open stopped after about 10 px: the handle captured the pointer at pointerdown, before the layer's claim (component:Resizable FR3). Layer's outside-press watch now reports a pan only when touch-action from the press's target up leaves the pan's axis to the browser (utils/scrollView touchActionAllowsPan), so a drag on a control that keeps the pan (touch-action: none, or a sideways drag on pan-y) is never claimed. The capture yield stays for a control that takes the pointer later. The no-pointercancel cases now start while the main thread is busy, on the scrollable page. The test-only story is Core/Popover Over Page Controls, with a resize handle and swipe rows. Merges origin/main; layer-runtime verified_by keeps both new entries.
There was a problem hiding this comment.
Approved pending CI: typecheck, lint and the CLI smoke test were still running on this head, and the heavy test suites run before merge.
- The
architecture:layer-runtimeINV7 sentence has an engineering owner's approval for this head. - A finger drag on a ResizeHandle beside an open Popover now follows the finger all the way, and swipe rows still rest open.
- The new Chromium cases for a pan read as a tap and a 40 px pan with no
pointercancelfail on the base and pass here; the anchored menus still close on that pan, and a tap outside still closes the layer.
Non-blocking: the description's Testing line says 2 cases fail on main, but in the Chromium build Playwright 1.61.1 installs the two 40 px cases fail there too.
[Automated review]
There was a problem hiding this comment.
Changes are needed before approval. The ResizeHandle drag beside an open Popover now follows the finger, and the architecture:layer-runtime INV7 sentence has an engineering owner's approval for this head.
Required change:
- Miss: Layer's new imports (
utils/clickGuardatgestureCounter.ts:26, andoutsidePresswithmotion/uikit/gesturesatuseLayer.tsx:40) make a Carousel withouthasSnap={useSnapPaging()}bundle the click guard, the haptic switch and the UIKit gesture constants, soscripts/snap-paging-bundle.test.mjs"Carousel alone bundles no pager code" fails againstcomponent:CarouselFR8 (CI'stest-node (1/4)); keep those modules out of Layer's static imports, or get an owner's decision that narrows FR8's pager-code list and the test with it.
Non-blocking: the description's Testing line says 2 cases fail on main, but in the Chromium build Playwright 1.61.1 installs the two 40 px no-pointercancel cases fail there too.
[Automated review]
Layer's static imports of the click guard (through gestureCounter), the outside-press watch and the UIKit gesture constants made a Carousel without snap paging bundle them, against component:Carousel FR8 (scripts/snap-paging-bundle.test.mjs). The pan claim now lives in Layer/outsidePanClaim.ts. usePopover passes it to useLayer as watchesOutsidePans for a light-dismiss popover whose owner does not own its outside pan; useLayer starts and stops what it is given. claimPress takes an onRelease callback instead of arming the click guard itself. The bundle test fails at the previous head and passes here.
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the new architecture:layer-runtime INV7 sentence, and one change is needed either way.
Decision needed: the INV7 sentence that keeps a light-dismiss layer open after a touch pans outside it is new spec text, and no engineering owner has approved it on the current head. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. The decision should also settle which layers the rule covers.
Required change:
- INV7 says the pan rule holds for "an open layer", but only
usePopoverpasses the watch now (usePopover.tsx:495-496), so a layer made with the publicuseLayer({mode: 'context', lightDismiss: true})still closes and clicks the page on a 12 px pan in Chromium; narrow the INV7 sentence, its verification row and the title to light-dismiss popovers built onusePopover, or give publicuseLayerlight-dismiss layers the claim without a static import into Layer.
The Carousel bundle test passes again. The new Chromium cases fail on the base and pass on this head for Popover and Selector.
[Automated review]
| * owner (usePopover's `watchOutsidePans`). An anchored menu, which closes | ||
| * as such a pan begins, is given none. |
There was a problem hiding this comment.
Non-blocking: DropdownMenu does not close as such a pan begins yet (component:DropdownMenu FR21 gap), so this reads as current behavior that is not. Suggested: "An anchored menu owns what such a pan does, so it is given none."
| const dismissedByGestureRef = useRef<number | null>(null); | ||
| const forgetDismissalRef = useRef<(() => void) | null>(null); | ||
|
|
||
| // Stops the watch for pans outside the open layer (`watchesOutsidePans`). |
There was a problem hiding this comment.
Non-blocking: this restates stopOutsidePanWatch; it can go.
|
/approve-spec 0469438 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes are needed before approval. The architecture:layer-runtime INV7 outside-pan sentence now has an engineering owner's approval at this head, so one change remains.
Required change:
- The approved INV7 sentence covers "an open layer", but only
usePopoverpasses the watch (usePopover.tsx:495-496), so a layer made with the publicuseLayer({mode: 'context', lightDismiss: true})still closes and clicks the page on a 12 px touch pan in Chromium; give publicuseLayerlight-dismiss layers the claim while keeping the Carousel bundle test green, or narrow the INV7 sentence, its verification row and the title to light-dismiss popovers built onusePopoverand have an engineering owner approve that text at the new head.
Popover and Selector stay open with no page click after the pan, the six menus still close, and the new Chromium cases fail on the base and pass on this head.
[Automated review]
The outside-pan claim reaches popovers built on usePopover, not a layer made with the public useLayer, so Carousel stays free of it (component:Carousel FR8). INV7's sentence now names an open light-dismiss popover, the current-gaps list states that a public useLayer({lightDismiss: true}) caller gets no outside-pan claim, and the verification row names the Popover family and the menus built on it.
cixzhang
changed the title
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec acfdfbc |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Approved pending CI: the required build and test jobs did not run on this head because CI was full, so they need a re-run before merge.
The architecture:layer-runtime INV7 outside-pan sentence, now narrowed to light-dismiss popovers with the public useLayer path listed as a gap, has an engineering owner's approval at this head, which resolves the earlier scope finding. In Chromium, Popover and Selector stay open with no page click after a short outside pan, the six menus still close, and page-control drags still follow the finger.
Two optional comment tweaks remain in useLayer.tsx (lines 502-503 and 1331).
[Automated review]
# Conflicts: # docs/architecture/layer-runtime.md # packages/core/src/Layer/useLayer.tsx # packages/core/src/Popover/usePopover.tsx
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 7c990d4 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Approved pending CI: the required build, test and docsite-test jobs are deferred on this head and run in the pre-merge re-run.
The architecture:layer-runtime INV7 outside-pan rule has an engineering owner's approval for this head, and the new Chromium cases show Popover and Selector staying open, with no page click, after an outside pan that closes them on main. The six menus still close on that pan, and resize-handle and swipe-row drags keep following the finger. The two non-blocking comment notes on useLayer.tsx from the last round still stand.
User impact
On a touch phone, a person who opens a Popover or a Selector list and then flicks the page to scroll sometimes loses it: the layer closes under their finger, and the page element under the finger can get a click. This happens in Chromium on Android when the main thread is busy. The page sees
pointerdown→pointerupwith nopointercancel, and the browser'spopover="auto"light dismiss closes the layer. After this change a pan over the page leaves the layer open and clicks nothing. A tap outside still closes it. The change covers popovers built onusePopover(Popover, Selector's list and the other Popover-family layers); a layer made with the publicuseLayeris unchanged.Anchored menus are left out. An anchored menu is any anchored layer with menu semantics:
role="menu"with menu items, whatever it is built on.spec:AST-094FR32 says such a menu closes as a pan outside begins. That is DropdownMenu FR21 and ContextMenu FR5, both still implementation gaps.dialogornone, as the caller sets itlistboxhasLightDismiss: false)role="menu")role="menu")menumenumenumenumenumenumenu, manual popoverA caller's own Popover whose content is a
role="menu"keeps the guard: the layer cannot tell what its content is.Problem and solution fit
pointerup). Chrome 157 does it from the tap gesture, after the stand-inmouseupand beforeclick. When the browser takes a finger that travelled as a tap, or reaches a release outside the layer, the layer closes. No scroll began, so the page sees nopointercancel.beforetoggletoclosedcannot be cancelled, so the layer cannot veto the close after the fact.The fix acts while the touch is still in progress. While a non-menu auto layer is open, Layer's outside-press watch follows touches. A touch that starts outside the layer and travels the shared pan distance (
UIKIT_PAN.hysteresis, 10 px on either axis, the same "begins to pan" rule the watch already uses) is claimed for the layer with the existingclaimPress:armClickGuardwithcancelsTouchEnd) cancels the touch'stouchend, so the browser makes no tap, no click and no light dismiss.Measured in Chrome 157 (CDP touch, phone profile):
preventDefault()ontouchendstops the tap's light dismiss, and onpointerupit does not;Expected behavior and authority
architecture:layer-runtimeINV7, amended in this pull request: "A one-finger touch that starts outside an open light-dismiss popover and travels the shared pan distance is a gesture, not an outside press, even when the browser reads it as a tap; a tap outside still dismisses. An anchored menu instead closes as that pan begins (spec:AST-094FR32)." Its current-gaps list adds: "A caller of the publicuseLayer({lightDismiss: true})gets no outside-pan claim; Popover-based layers do (INV7)." The verification row names the Popover family and the menus built on it, andverified_bylists the new tests.family:overlay-dismissalAV4 leaves outside dismissal to each member; FR11 is unchanged: a tap still light-dismisses.role="menu"opts out through an internalownsOutsidePanoption onusePopoverInternal(no public API, no data attribute). ContextMenu is a manual popover and is untouched; its pan dismissal is in progress in feat(ContextMenu): open from a held finger on iOS 26's timeline #8125.Smallest restoration
Layer/outsidePress.ts:watchOutsidePresstakes anonPanoption. It reports a press outside once, as it begins a pan that the browser could take as a scroll, and then judges nothing else: no tap, no cancel, no dismissal count. "Could take as a scroll" meanstouch-actionfrom the press's target up leaves the pan's axis to the browser (utils/scrollView.ts's newtouchActionAllowsPan, on the existingparseTouchAction).Layer/gestureCounter.ts:claimPresstakes anonReleasecallback, run at the release of a claim still held then. A claim ends when another control takes the pointer, so a page control that captured the touch keeps it.Layer/outsidePanClaim.ts(new):watchOutsidePans(layer)joins the two. It starts the outside-press watch withonPan, which claims the touch for the layer, and the claim'sonReleasearms the click guard (armClickGuardwithcancelsTouchEnd).Layer/useLayer.tsx: an open auto layer starts the watch its owner passes in (watchesOutsidePans), and stops it when the layer closes or unmounts. Layer imports none of the watch, the click guard or the UIKit gesture constants statically. So a layer that never light-dismisses (Carousel) bundles none of them, andscripts/snap-paging-bundle.test.mjs"Carousel alone bundles no pager code" holds (component:CarouselFR8). That test fails at 994ed2e and passes here.Popover/usePopover.tsxpasseswatchOutsidePansfor a light-dismiss popover whose owner does not setownsOutsidePan. A caller of the publicuseLayer({lightDismiss: true})gets no pan claim. That keeps a layer that never light-dismisses (Carousel) free of the claim's code (component:CarouselFR8), and the record states it as an INV7 gap.ownsOutsidePanonusePopoverInternal.Evidence
pointeruptargets the layer and itstouchendis cancelled.pointercancel(it starts while the main thread is busy, so the browser takes no scroll) leaves Popover and Selector open, and so does a 40 px pan that scrolls the page (withpointercancel);touch-actionkeeps that axis from the browser is never claimed: a ResizeHandle (touch-action: none, capture atpointerdown) and a swipe row's sideways drag (pan-y). And a claim ends when another control takes the pointer (gotpointercapture); it then neither takes the pointer back nor cancels the tap.component:ResizableFR3). At the previous head it grew by 10 px.spec:AST-057FR4, FR5). Two heads back it rested 20 px open.Core/Popover"Over Page Controls".outsidePress.test.ts,outsidePanClaim.test.tsandgestureCounter.test.ts, removing theonPanreport, the click-guard arm inoutsidePanClaim.ts, theonReleasecall or the yield to another control fails the new cases.useLayer.test.tsxchecks that an open auto layer given the watch claims a pan outside, that a closed layer claims none, and that a layer given no watch claims none.Two DropdownMenu cases, and one case each for TabMenu, the BreadcrumbItem menu, TopNavMenu, the TopNavHeading menu and the SideNavHeading menu, are
test.fixme: "closes as a pan outside begins (FR21 gap)". On the phone profile, each of those triggers opens an auto popover withrole="menu". Each of the five also has a live case, "a pan that the browser reads as a tap still closes it", so a lost opt-out fails. Two ContextMenu cases aretest.fixmetoo, enabled by the ContextMenu pan-dismiss work. Whichever of this pull request and #8125 lands second turns the ContextMenu cases on. Chrome 157 never makes a tap from a 40 px pan, so the 40 px cases pass on main there. Chromium 149 closes at the release, and CI runs those cases in it.Scope
Components: Layer (useLayer, outsidePress, gestureCounter), Popover (usePopover), Selector (covered as a Popover-family layer), Item swipe rows (kept working; test-only story); opt-out only: DropdownMenu, TabMenu, BreadcrumbItem, TopNavMenu, TopNavHeading, SideNavHeading
Writing rubric results
Record touched:
architecture:layer-runtime(INV7 sentence, the INV7 current-gaps entry and itsspec:AST-003scope note, the INV6/INV7 verification row,verified_by).Testing
vitest runfor Layer, Popover, DropdownMenu, Selector, TabList, TopNav, SideNav, Breadcrumbs, ContextMenu, Carousel, Item, Resizable and utils: 165 files, 4006 tests pass. The earlier rounds also covered List, Typeahead, DateInput, DateTimeInput, Chat/useTriggerMenu and the gestures folder.scripts/snap-paging-bundle.test.mjs: 4 pass.pnpm -F @astryxdesign/core typecheck,pnpm check:knowledgeand ESLint on the changed files: clean.packages/core/src/Layer/__tests__/LayerPanLightDismiss.interaction.browser.spec.ts, in Chrome 157 (CDP touch, phone profile). On main, the tap-read pan fails for Popover and Selector, and the two page-control cases fail because their test-only story is new. Both 40 px no-pointercancelcases pass on main in Chrome 157, which never makes a tap from them. In Playwright 1.61.1's Chromium they fail on main, and CI runs them there. On this branch, 18 pass and 9 are fixme, in 2 of 2 runs.pnpm specs:for --diff origin/main: followed.