Repository navigation
spec(family): action groups — press, slide across, act on lift (AST-094 FR26–FR28) - #8622
Conversation
…(AST-094 FR26–FR28, DEC-17)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
/approve-spec d88563a |
|
AI review status for this pull request.
|
github-actions
Bot
added
the
spec-auto-merge
|
/approve-spec 99d6101 |
|
/approve-spec 3369b14 |
|
/approve-spec f4f87d2 |
|
That The gate only accepts the exact current head, so post this instead: A new commit invalidates the command, so repeat it after any push. |
…one passive listener (FR19/FR24 exception scoped to action groups)
|
/approve-spec 5d22a75 |
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression57 of 504 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 37 more. Generated by PR Enrichment workflow | Storybook | Sandbox | 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.
Thanks for pulling press, slide and act-on-lift into one family contract; the direction is owner-approved, but a few current records still contradict it, so changes are requested before this lands. Each fix is an edit to the records in this pull request:
spec:AST-094FR11 and its verification row still say a control activates only through the browser's click, which never reaches the lifted button after a slide.- FR5's fine-pointer budget leaves menus no room for the post-release click swallower that
module:DropdownMenu/useMenuPressPR2 and FR5 rely on. component:ButtonGroup,component:Bannerandfamily:buttonsFR11 keep ownership statements the family overrides, and AST-094's Current-state impact does not name the change.membersneedscomponent:DateTimeInput, the date wheels need an answer for their drag-to-scroll, and DEC-17 should drop "a segmented control".
[Automated review]
| FR26–FR28 are the shared rule of `family:action-groups`: every member, not | ||
| only menus, follows them under the family's scope, governed by the one | ||
| internal `actionGroupTracker` (DEC-17). |
There was a problem hiding this comment.
This makes FR26 bind every member, and FR26 ends with "the browser's click for that gesture acts on nothing". FR11 (lines 184–208) still says a control "activates by the browser's click on a lift inside … its activation stays the browser's click", and the FR11/FR12 verification row fails "a control activated other than by the browser's click". After a slide no browser click reaches the lifted button, so ButtonGroup, Toolbar, AlertDialog, Banner and Toast buttons cannot meet family FR1 without breaking FR11. Suggest amending FR11 and that row so a control inside a family:action-groups member activates at the lift through the group's tracker, the way FR11 already gives menu rows the cell profile.
| **Decider:** `cixzhang`, `2026-10-26` | ||
|
|
||
| A person who presses one action and slides to another expects the same result | ||
| in a menu, a segmented control, an alert's buttons, or a button group. One |
There was a problem hiding this comment.
family:action-groups makes SegmentedControl a named exception that "this family's rule does not bind", but this sentence lists a segmented control among the groups that behave the same, and the next sentence says one tracker governs every group. Suggest "a menu, an alert's buttons, a button group, or a toolbar", and scoping the next sentence to the family's members.
| - **FR5 — Every pointer, one target rule.** The behavior applies to every web | ||
| pointer, mouse and pen as well as a finger. Under a fine pointer a group | ||
| keeps one passive `pointerdown` listener on its element and adds document | ||
| listeners only during a press, removing them at the release; nothing else | ||
| is idle (`spec:AST-094` FR19, FR24). The 44 px target is each member's hit |
There was a problem hiding this comment.
Menus are members, and useMenuPress keeps a window capture click listener installed once per document (its PR2) to swallow the browser's click, which arrives after the release (its FR5); the shipped hook also cancels a finger's pointerdown (packages/core/src/hooks/useMenuPress.ts line 813). "Removing them at the release; nothing else is idle" and a passive pointerdown leave no room for either. Suggest allowing one per-document click swallower armed from the release until the gesture's click, and a cancelable pointerdown where a member needs it, or amending useMenuPress PR2 and FR5 in this pull request.
| component:ContextMenu, | ||
| component:Selector, | ||
| component:MultiSelector, | ||
| component:DateInput, |
There was a problem hiding this comment.
The membership rule names the date wheels of DateInput and DateTimeInput (lines 66–67), but only component:DateInput is listed. pnpm specs:for routes this family to DateInput and the shared Wheel.tsx, while DateTimeInput.tsx and TouchDateTimeField.tsx get only the five global FR routes; TimeInput's TouchTimeField.tsx mounts the same Wheel and is neither listed nor excluded. Separately, a wheel is a scroll container whose mouse drag scrolls it (usePointerDragScroll.ts), which FR1 and FR2 under FR5 would turn into tracking; consider an allowed variation (a drag scrolls, a tap acts) or moving the wheels to the exclusions.
| | Menus, Selector, MultiSelector, date wheels | Track through `actionGroupTracker`; `useMenuPress` converges onto it. | | ||
| | AlertDialog's actions | Track the actions only. | | ||
| | ButtonGroup, ToggleButtonGroup, Toolbar | Track sibling buttons; a field inside is skipped; toggles follow FR4. | | ||
| | Banner and Toast action rows | Track only a row with two or more sibling actions. | |
There was a problem hiding this comment.
These adoptions contradict current records this pull request leaves as they are: component:ButtonGroup does not own "Each member's own surface, naming, activation, pending, and disabled rendering", component:Banner does not own "Button geometry, interaction layers, tooltip painting, or consumer-supplied endContent", and family:buttons FR11 says "The group owns only the behavior and surface declared by its group mode." AST-094's Current-state impact names neither this family nor its members. Suggest amending those statements, saying where focus goes after a slide, and adding the Current-state impact entries.
meta-cla
Bot
added
the
CLA Signed
|
/approve-spec 1e17099 |
There was a problem hiding this comment.
The action-groups contract and its amendments are owner-approved for this head, and the earlier Toolbar, InputGroup and navigation-menu findings are fixed, but one membership gap needs an edit before this lands. The family lists SideNavHeading's product menu as a member, while TopNavHeading renders the same popover with its own role="menu" around the caller's menu (TopNavHeading.spec.md FR7) and appears nowhere in members, the membership rule, the adoption row, the gap sentence or INV13's gap note; please add it beside SideNavHeading, or exclude it with a reason. Two smaller points: TopNavMegaMenu's panel also meets the membership rule and is on neither list, and ToggleButtonGroup's verification row still names "the absence of group listeners" as PR1's test gap.
[Automated review]
| today: | ||
|
|
||
| - menus: DropdownMenu, ContextMenu, and their sub-menus, the navigation | ||
| menus of TopNav (TopNavMenu), SideNavHeading, and NavHeadingMenu; and |
There was a problem hiding this comment.
TopNavHeading is missing here. Its current record gives it the same product menu as SideNavHeading's: FR7 "a role="menu" region named by heading … that holds menu" (packages/core/src/TopNav/TopNavHeading.spec.md line 125), and TopNavHeading.tsx line 554 renders it exactly as SideNavHeading.tsx line 536 does. As written, pnpm specs:for packages/core/src/TopNav/TopNavHeading.tsx does not reach this family, and the same NavHeadingMenu would track a held pointer in a SideNav header but not in a TopNav header. Suggest adding component:TopNavHeading to members and naming TopNavHeading's menu here, in the adoption row (line 171), the gap sentence (lines 177–179) and INV13's gap note, or excluding it with a reason.
|
/approve-spec 2ebd152 |
There was a problem hiding this comment.
The action-groups contract and its amendments are owner-approved for this head, and the TopNavHeading, TopNavMegaMenu and ToggleButtonGroup verification-row fixes hold, but the membership still needs one edit before this lands. Three current components match the membership rule and appear on neither list: TabMenu's role="menu" of menuitemradio options (TabMenu.spec.md FR4), DateRangeInput's labeled group of preset buttons (DateRangeInput.spec.md FR2), and SideNavItem's collapsed-rail flyout (SideNavItem.spec.md FR9). Please name each as a member, with its adoption row and INV13's gap note where it applies, or exclude it with a reason, as you did for TopNavHeading and TopNavMegaMenu. The description also still calls FR5's listener passive and says useMenuPress converges onto the tracker, which the records no longer say.
[Automated review]
| A component is a member when it renders a set of two or more sibling | ||
| momentary, toggle, or option actions that a person chooses one of. Members | ||
| today: |
There was a problem hiding this comment.
Three current components meet this rule and appear neither in the list below nor under "These are not members": TabMenu's role="menu" of menuitemradio options (packages/core/src/TabList/TabMenu.spec.md FR4, TabMenu.tsx lines 405 and 425), DateRangeInput's presets, "an independent button in one labeled group" (DateRangeInput.spec.md FR2), and SideNavItem's collapsed-rail flyout (SideNavItem.spec.md FR9). pnpm specs:for packages/core/src/TabList/TabMenu.tsx does not reach this family. Suggest naming each as a member (with members, its adoption row and INV13's gap note where it applies) or excluding it with a reason, as was done for TopNavHeading and TopNavMegaMenu.
|
/approve-spec fe7bf01 |
…oups # Conflicts: # docs/architecture/interaction-modality.md
|
/approve-spec 514dd56 |
There was a problem hiding this comment.
The action-groups contract and its amendments are owner-approved for this head, and the TabMenu, DateRangeInput and SideNavItem membership gap is closed, but one record line still needs an edit before this lands. spec:AST-094's Current-state impact names the family's members by hand (docs/specs/AST-094-touch-presentations/spec.md lines 605–609) and leaves out DateRangeInput's presets, which this head makes a member; please add them, or point that line at the family's membership so the two cannot drift. The description's member and exclusion lists also predate the latest membership edit.
[Automated review]
| - `family:action-groups` members (menus, Selector's and MultiSelector's | ||
| options, the date and time wheels, AlertDialog's actions, ButtonGroup, | ||
| ToggleButtonGroup, Toolbar's buttons, and Banner's and Toast's action rows): | ||
| a press that slides across sibling actions acts on the action it lifts on | ||
| (FR26–FR28), under every pointer. |
There was a problem hiding this comment.
This list names the family's members by hand and leaves out DateRangeInput's presets, which docs/families/action-groups.md now makes a member (line 83, adoption row at line 187). Add "DateRangeInput's presets", or point this line at the family's membership so the two cannot drift.
|
/approve-spec 63b7b09 |
There was a problem hiding this comment.
Full review of 63b7b09; the incremental delta could not be read: the merge of main rewrote two spec records in its conflict resolution.
The action-groups contract and its amendments are owner-approved for this head, and the earlier gap is closed: spec:AST-094's Current-state impact now points at the family's membership rule, so DateRangeInput's presets are covered and nothing blocks. Two optional cleanups remain: run Prettier on docs/architecture/interaction-modality.md, whose INV13 verification row lost its padding in the merge, and consider amending component:AlertDialog's draft PR1 ("only listener is the small-screen media query") before its actions adopt the family.
[Automated review]
| | INV10 | Source boundary check over Core and Lab, a Lab-to-Core comparison on one recorded stream, and release-filter unit tests over recorded iOS pan and scroll-view streams | A Core component with its own sample buffer, velocity math, or release filter; a Lab component with its own sample buffer, velocity math, or release filter; a Lab component reporting a different velocity than Core for the same stream; an export of the tracker; a non-zero release after a still pause | | ||
| | INV11 | Evidence gap: not implemented yet; the shared hover recognizer and its adopters' move onto it are pending. Then: unit tests of the hover recognizer's intent and browser tests of its adopters with a mouse, a pen, a finger, and the keyboard | A show without the owner's entry delay, a missed handoff within a group, a delayed keyboard show, a hover-shown surface that closes before the pointer reaches it, a finger served by it, or a second hover-timing implementation fails | | ||
| | INV12 | Unit tests of native scroll's progress and real-browser proofs of its adopters | Progress routed through React state, a missing edge, travel under Reduce Motion, use where native scroll-driven animation is supported, or a public export fails | | ||
| | INV13 | Source boundary check over Core and each family member's pointer-stream tests | A member outside menus and Selector's and MultiSelector's options that tracks a held pointer across its actions with its own code, or a public export of the tracker, fails | |
There was a problem hiding this comment.
Nit: this row lost its padding in the merge from main, so prettier --check now flags the file (it passes on main), and the lint-staged prettier --write hook will re-pad the whole row as churn in the next commit that touches it. Running Prettier on the file fixes it.
|
/approve-spec 63b7b09 |
… base interaction-modality.md and AST-094 take main's text: the branch's versions were meta#8622's own, which main now carries as merged.
…o ButtonGroup press tracking
…leButtonGroup press tracking
Decided (Cindy, 2026-10-26: D32, then the full scope: "capture all potentially related components so they get the behavior consistently").
User impact
A person who presses one action and slides to another gets the same result in a menu, an alert's buttons, a button group, or a toolbar. The action they lift on runs, sliding off runs nothing, and in a toggle group only the lifted item toggles. Keyboard and screen-reader users are unchanged.
Records
family:action-groups(docs/families/action-groups.md):spec:AST-094FR26–FR28 rule); FR4 a toggle group toggles only the lifted action.pointerdownlistener on its element, cancelable where a member needs it, and adds document listeners only during a press. The 44 px target is each member's hit expansion under a coarse pointer only. Keyboard and AT are unchanged.actionGroupTrackerfor every member but menus and Selector's and MultiSelector's options, which stay onuseMenuPress; when they converge is open (INV13, OQ1).LayoutFooterget the behavior by composing ButtonGroup.spec:AST-094: FR26–FR28 become the family's shared rule. FR19 lets a member track its own group as FR24 allows (one element listener and press-time document listeners), and FR24 counts its release-armed click swallower as press work, and DEC-17 records the decision. Its number assumes DEC-15 (spec(AST-108): theme motion is three knobs — tempo, character, reducedMotion #8613) and DEC-16 (spec(AST-094): every touch presentation holds pointer input while it enters (FR35) #8618).pnpm check:knowledgepasses.Writing rubric results