Repository navigation
spec(ContextMenu, List): ContextMenu on the headless menu; ListItem row menus attach to the li - #8648
Conversation
…istItem row menus attach to the li (FR10, FR11)
|
/approve-spec f5dcbbe |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
spec-auto-merge
…y hold opens the menu
|
/approve-spec 52860ba |
PR Analysis Report
The exact-head preview is not available yet. Generated by PR Enrichment workflow | View current CI run |
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes. The ContextMenu and List direction is owner-approved, but this head should not merge yet: both new claims cite spec:AST-107 FR8, which only lands with #8600, and List FR11's last clause describes a reorder state that current AST-103 rules out. After updating, the new head needs a fresh /approve-spec.
- Land #8600 first and rebase, so the spec auto-merge path cannot merge this ahead of it.
- Remove "a row that reorders without a handle shows one when it also has a menu" and its "whole-row reorderable row with a menu" verification state, or amend AST-103 to define handle-less reordering.
- Fill the "Mutation or failure expectation" and "Audit section" cells of the three new verification rows.
- Settle the plain row's button name (include the row's label; Core's existing overflow trigger says "More options") and which element gets focus after a right click or long press opens a plain row's menu.
[Automated review]
| | FR7 | Each row's retained inset MUST come from the same private density- and theme-derived source as Item's applied inline padding. A theme `item.paddingInline` override takes precedence over every treatment's row padding, as it does over density. A consumer override of row padding through `xstyle`, `className`, or `style` is outside the alignment guarantee. | Shared two-owner geometry contract | Accepted contract | | ||
| | FR8 | `outset` and `bleed` are valid only under a host state whose published inline inset matches its applied padding: a visually neutral host, such as a Layout region with explicit padding or a padded ScrollableArea, or the content area of a Card, Section (any variant), Dialog, or BottomSheet. Those hosts bound their padding with a border, background, or clip; FR6 keeps row paint inside the padding edge, so it never covers the border or changes the host's outline. Automatic LayoutContent/LayoutPanel states with a known publication mismatch are not eligible until that gap is fixed, and Drawer's content area is not admitted until it has its own evidence. | `architecture:container-padding/INV8–INV11`; real-browser evidence in each admitted host | Accepted contract | | ||
| | FR9 | The list element containing the rows MUST clip in the inline axis with `overflow-inline: clip`, never `hidden`. Under `outset` and `bleed` its inline edges MUST reach the host's padding edge, so the clip never cuts resting row paint and a swiped row paints no further than that edge. | `spec:AST-057/FR9` | Verified by `List.test.tsx` and real-browser geometry | | ||
| | FR11 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), the row takes the press paint and lifts as the menu grows from it, and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly; a row with neither shows a visible "More actions" button, its keyboard and screen-reader path to the menu. When a row is both reorderable and has a row menu, a hold on the reorder handle picks the row up and a hold anywhere else on the row opens the menu; a row that reorders without a handle shows one when it also has a menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
The last clause, "a row that reorders without a handle shows one when it also has a menu", presupposes handle-less reordering. Current AST-103 FR3 says "a drag starts from a row's handle", and FR4 gives each reorderable row a handle. Remove the clause, or amend AST-103 in this PR to define that state.
| | FR1–FR7, FR9 | List, Item, and theme-registry tests | every treatment; all densities; static and interactive rows; header; deprecated alias alone and with `edge`; ListItem outside a List | Removing context propagation, using one host edge for both inline edges, zeroing padding without the shared inset source, or dropping the list's reach or clip fails | `audit:List/layout` | | ||
| | FR2–FR6, FR8–FR9, AR1–AR2 | Real-browser List geometry evidence | each treatment in a Card at two paddings, a Section, an inline Dialog with and without Layout, a BottomSheet, a padded ScrollableArea scrolling vertically and in both axes, a LayoutContent with explicit padding, and standalone; header and dividers; LTR/RTL; hovered and selected rows; a swiped row under `outset` and `bleed` | Header movement, content off the expected line, paint past the host's padding edge, clipped resting paint, or a changed row box fails | `audit:List/visual` | | ||
| | FR11 | List tests and real-browser tests of a row menu with a mouse, a finger, and the keyboard | a plain row, a row with `onClick`, a row with `href`; a right click, a long press, Shift+F10, the context-menu key; close by Escape and by choice | | | | ||
| | FR11 reorder | Real-browser tests with a finger and a mouse on a reorderable row with a menu | a hold on the handle, a hold on the row body, a whole-row reorderable row with a menu | | | |
There was a problem hiding this comment.
"A whole-row reorderable row with a menu" cannot be built under current AST-103, where every drag starts from the handle. This row and the FR11 row above also leave the failure-expectation and audit cells empty.
| | FR7 | Sub-menu rows follow `component:DropdownMenu` FR12 inside the pointer menu: ContextMenu mounts the same drill-in view stack, named after its `label`, so a sub-menu drills in on a phone here too. | `component:DropdownMenu/DEC-2` | Proposed; verified in jsdom; owner decision pending | | ||
| | FR8 | In the pointer menu ArrowDown on the last enabled row wraps to the first and ArrowUp on the first to the last; PageDown and PageUp page as in DropdownMenu FR6; a held key's auto-repeat does not activate; typeahead matches the row's label element alone. | Current source and tests; shared `useListFocus` and `useTypeahead` suites | Proposed; verified in jsdom; owner decision pending | | ||
| | FR9 | While the Pointer menu surface is open, it participates in `family:overlay-dismissal` through the shared stack and provides descendant depth. One Escape press affects only the top-most present nested surface; ContextMenu closes only when it is that surface. | `family:overlay-dismissal` FR1–FR6; `ContextMenu.test.tsx` nested-layer case | Current family projection; verified in jsdom | | ||
| | FR10 | ContextMenu is built on the public headless context menu (`spec:AST-107` FR8): its invocation, open state, and cleanup are the factory's, with no behavior of its own beside the factory's. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Decided | |
| | Layer relationships | `ContextMenu.test.tsx` nested-layer case and current layer/dismissal architecture records | Outside click, Escape, nested context layers, sheet | A Pointer menu bypassing the shared stack or dismissing with its nested child fails. | `audit:ContextMenu/behavior` | | ||
| | FR5 | `ContextMenu.test.tsx` press model case; `module:DropdownMenu/useMenuPress` map | Finger slide across rows, stray click | A row acting on the press row or the stray click fails. | `audit:ContextMenu/behavior` | | ||
| | FR6 | `ContextMenu.test.tsx` keyboard case; shared `useListFocus` and `useTypeahead` suites | wrap at both ends | Clamping arrows fail. | `audit:ContextMenu/behavior` | | ||
| | FR10 | ContextMenu tests over `createContextMenu` and a source check | a right click, a long press, Shift+F10; open, close, unmount while open | | | |
There was a problem hiding this comment.
Please fill "Mutation or failure expectation" and "Audit section", for example: "a ContextMenu that opens, closes or cleans up outside the factory, or a listener left after unmount, fails".
…n name and focus pinned; verification cells filled
|
/approve-spec 21b4846 |
There was a problem hiding this comment.
Requesting changes. The ContextMenu and List claims are owner-approved, and the earlier reorder, verification-cell, button-name and focus points are addressed, but both new claims still cite spec:AST-107 FR8, which only #8600 adds, and the spec-only auto-merge path can land this head first. Please land #8600 first and rebase, or keep this pull request out of auto-merge (for example as a draft) until it lands; the new head then needs a fresh /approve-spec.
[Automated review]
| | FR7 | Each row's retained inset MUST come from the same private density- and theme-derived source as Item's applied inline padding. A theme `item.paddingInline` override takes precedence over every treatment's row padding, as it does over density. A consumer override of row padding through `xstyle`, `className`, or `style` is outside the alignment guarantee. | Shared two-owner geometry contract | Accepted contract | | ||
| | FR8 | `outset` and `bleed` are valid only under a host state whose published inline inset matches its applied padding: a visually neutral host, such as a Layout region with explicit padding or a padded ScrollableArea, or the content area of a Card, Section (any variant), Dialog, or BottomSheet. Those hosts bound their padding with a border, background, or clip; FR6 keeps row paint inside the padding edge, so it never covers the border or changes the host's outline. Automatic LayoutContent/LayoutPanel states with a known publication mismatch are not eligible until that gap is fixed, and Drawer's content area is not admitted until it has its own evidence. | `architecture:container-padding/INV8–INV11`; real-browser evidence in each admitted host | Accepted contract | | ||
| | FR9 | The list element containing the rows MUST clip in the inline axis with `overflow-inline: clip`, never `hidden`. Under `outset` and `bleed` its inline edges MUST reach the host's padding edge, so the clip never cuts resting row paint and a swiped row paints no further than that edge. | `spec:AST-057/FR9` | Verified by `List.test.tsx` and real-browser geometry | | ||
| | FR11 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), the row takes the press paint and lifts as the menu grows from it, and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly; a row with neither shows a visible "More actions" button named with the row's label, such as "More actions for Inbox", its keyboard and screen-reader path to the menu. When a menu opens, focus moves into it; when it closes, focus returns to the row, or to the "More actions" button on a row with neither `onClick` nor `href`. A reorderable row always has its handle (`spec:AST-103` FR3), so when it also has a row menu, a hold on the handle picks the row up and a hold anywhere else on the row opens the menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
This Basis, like ContextMenu FR10, cites spec:AST-107 FR8, which has no record on main yet; #8600 adds it. With exact-head owner approval in place, the spec-only auto-merge path can land this first and leave a current claim delegating to a missing record. Please land #8600 first and rebase, or keep this pull request out of auto-merge until it does.
github-actions
Bot
removed
the
spec-auto-merge
…menus # Conflicts: # packages/core/src/List/List.spec.md
|
/approve-spec 8100a24 |
github-actions
Bot
added
the
spec-auto-merge
There was a problem hiding this comment.
Requesting changes. The ContextMenu and List claims are owner-approved and their basis, AST-107 FR8, is now on main, but List's "Contract at a glance" was not updated for FR13: its Builder impact row still calls animatedLayout "the one new choice", and Behavior, End-user impact, Compatibility/readiness and Governing rules leave the row menu out. Please add FR13 to those rows (marking it specified, not implemented yet), add spec:AST-107 to Governing rules and system_specs, add a row-menu bullet to Owns, and name how a caller gives a ListItem a row menu so Builder impact can state that choice. While editing, consider saying whether an interactiveRef row shows the More actions button and which side dismisses ContextMenu on an outside click after FR10; the new head then needs a fresh /approve-spec.
[Automated review]
| | FR10 | Under a coarse primary pointer (`spec:AST-094` FR13), every ListItem row MUST offer a hit target at least 44 px tall, through its padding or an invisible hit area, with no visual change, where its neighbors leave room: the hit area never overlaps a neighboring row's or control's target, and between close neighbors it is clamped to half the gap; row height and geometry stay as they are at every density. A fine pointer keeps today's hit target. | Owner decision, 2026-10-26: no row-height change, a 44 px touch target | Accepted contract | | ||
| | FR11 | Under a coarse primary pointer, an interactive row's press MUST follow the cell profile (`spec:AST-094` FR11): it paints the `--color-overlay-pressed` token above the row's background and below its content without adding an element, and its release fades over the press model's release duration and curve on the compositor (`spec:AST-094` FR5). At rest the row's paint and stacking are as they were. | `spec:AST-094` FR5, FR11 | Accepted contract | | ||
| | FR12 | List MUST take an `animatedLayout` handle (`spec:AST-105` FR1) and report each row inserted, removed, or moved between renders. With a handle, an inserted row enters, a removed row collapses in place, inert and hidden from assistive technology until it is gone, and a row crossing rows moving the other way rides above them on the list's surface, while the rows after a change glide to their new places (`spec:AST-105` FR3), all starting on one frame on the shared motion role (`spec:AST-105` FR4). Without a handle the list changes at once (`spec:AST-105` FR2). A row's identity is its `itemKey` (`spec:AST-103` DEC-4); a row without one changes at once. | `spec:AST-105` FR1–FR5; `spec:AST-103` DEC-4 | Accepted contract | | ||
| | FR13 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), the row takes the press paint and lifts as the menu grows from it, and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly; a row with neither shows a visible "More actions" button named with the row's label, such as "More actions for Inbox", its keyboard and screen-reader path to the menu. When a menu opens, focus moves into it; when it closes, focus returns to the row, or to the "More actions" button on a row with neither `onClick` nor `href`. A reorderable row always has its handle (`spec:AST-103` FR3), so when it also has a row menu, a hold on the handle picks the row up and a hold anywhere else on the row opens the menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
List's Contract at a glance (lines 46-52) still leaves this row menu out: Builder impact calls animatedLayout "the one new choice", Compatibility/readiness does not mark FR13 specified and not implemented, and Governing rules and system_specs omit spec:AST-107; Owns has no row-menu bullet either. Please bring those up to FR13 and name how a caller gives a ListItem a row menu, so Builder impact can state the choice.
| | FR7 | Sub-menu rows follow `component:DropdownMenu` FR12 inside the pointer menu: ContextMenu mounts the same drill-in view stack, named after its `label`, so a sub-menu drills in on a phone here too. | `component:DropdownMenu/DEC-2` | Proposed; verified in jsdom; owner decision pending | | ||
| | FR8 | In the pointer menu ArrowDown on the last enabled row wraps to the first and ArrowUp on the first to the last; PageDown and PageUp page as in DropdownMenu FR6; a held key's auto-repeat does not activate; typeahead matches the row's label element alone. | Current source and tests; shared `useListFocus` and `useTypeahead` suites | Proposed; verified in jsdom; owner decision pending | | ||
| | FR9 | While the Pointer menu surface is open, it participates in `family:overlay-dismissal` through the shared stack and provides descendant depth. One Escape press affects only the top-most present nested surface; ContextMenu closes only when it is that surface. | `family:overlay-dismissal` FR1–FR6; `ContextMenu.test.tsx` nested-layer case | Current family projection; verified in jsdom | | ||
| | FR10 | ContextMenu is built on the public headless context menu (`spec:AST-107` FR8): its invocation, open state, and cleanup are the factory's, with no behavior of its own beside the factory's. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Decided | |
There was a problem hiding this comment.
With open state the factory's "with no behavior of its own", this record still owns "open-state coordination" (line 66) and says ContextMenu "retains local outside-click handling" (line 199). Worth saying which side closes the menu on an outside click.
|
/approve-spec bfbad4c |
There was a problem hiding this comment.
Requesting changes. Last round's projection updates are in and both claims are owner-approved, but List FR13 now has the caller pass the ListItem's ref to useContextMenu with no ListItem prop, while AST-107 FR8 returns focus to that element on close, the row's <li>, which never takes focus, so FR13's return to the row's control or its More actions button has no owner. Please reconcile the two (amend FR8's focus return, or say in FR13 or OQ2 who moves focus and keep "focus return" out of Owns until then), mark the More actions button as open in Owns and Builder impact until OQ2 is ruled, and state ContextMenu's outside-click close in AST-107 FR8 or keep it ContextMenu's own. The new head will then need a fresh /approve-spec.
[Automated review]
| | FR10 | Under a coarse primary pointer (`spec:AST-094` FR13), every ListItem row MUST offer a hit target at least 44 px tall, through its padding or an invisible hit area, with no visual change, where its neighbors leave room: the hit area never overlaps a neighboring row's or control's target, and between close neighbors it is clamped to half the gap; row height and geometry stay as they are at every density. A fine pointer keeps today's hit target. | Owner decision, 2026-10-26: no row-height change, a 44 px touch target | Accepted contract | | ||
| | FR11 | Under a coarse primary pointer, an interactive row's press MUST follow the cell profile (`spec:AST-094` FR11): it paints the `--color-overlay-pressed` token above the row's background and below its content without adding an element, and its release fades over the press model's release duration and curve on the compositor (`spec:AST-094` FR5). At rest the row's paint and stacking are as they were. | `spec:AST-094` FR5, FR11 | Accepted contract | | ||
| | FR12 | List MUST take an `animatedLayout` handle (`spec:AST-105` FR1) and report each row inserted, removed, or moved between renders. With a handle, an inserted row enters, a removed row collapses in place, inert and hidden from assistive technology until it is gone, and a row crossing rows moving the other way rides above them on the list's surface, while the rows after a change glide to their new places (`spec:AST-105` FR3), all starting on one frame on the shared motion role (`spec:AST-105` FR4). Without a handle the list changes at once (`spec:AST-105` FR2). A row's identity is its `itemKey` (`spec:AST-103` DEC-4); a row without one changes at once. | `spec:AST-105` FR1–FR5; `spec:AST-103` DEC-4 | Accepted contract | | ||
| | FR13 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the caller passes the ListItem's ref to `useContextMenu` (`spec:AST-107` FR8), with no ListItem prop of its own; the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), the row takes the press paint and lifts as the menu grows from it, and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly, and a row with an `interactiveRef` takes them on that element; neither shows a "More actions" button; a row with none of these shows a visible "More actions" button named with the row's label, such as "More actions for Inbox", its keyboard and screen-reader path to the menu. When a menu opens, focus moves into it; when it closes, focus returns to the row, or to the "More actions" button on a row that shows one. A reorderable row always has its handle (`spec:AST-103` FR3), so when it also has a row menu, a hold on the handle picks the row up and a hold anywhere else on the row opens the menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
This focus return conflicts with AST-107 FR8. The menu attaches to the ListItem's ref, the row's <li>, and FR8 returns focus "to the element on close"; that <li> never takes focus (an onClick row's target is Item's invisible button, an href row's its anchor), and with no ListItem prop nothing else is assigned to move focus to the row's control or the More actions button. Amend FR8's focus return, or say here or in OQ2 who moves focus.
| - Its rows' press paint under a coarse pointer (FR11). | ||
| - Its rows' update motion with an `animatedLayout` handle: entering, exiting, | ||
| and crossing (FR12). | ||
| - Its rows' menus: attaching to the row's own `<li>`, the More actions button, |
There was a problem hiding this comment.
Owns lists the More actions button and focus return, but OQ2 says the button has no owner yet, and its option (a) makes it the caller's. Until OQ2 is ruled, please say the button is open here and in Builder impact (line 50), which names passing the ref as the caller's only step.
| - `architecture:layer-runtime` owns the current context-mode `useLayer` host and | ||
| cursor-anchor positioning. ContextMenu retains local outside-click handling and | ||
| joins shared Escape ordering through `useLayerDismissal`; touch hosting delegates | ||
| cursor-anchor positioning. The factory closes the menu on an outside click |
There was a problem hiding this comment.
AST-107 FR8, which specifies the factory, lists Escape but no outside-click close, and FR10 leaves ContextMenu no behavior of its own beside the factory's. Please state the outside-click close (with today's skip of the finger press's compatibility mouse events) in AST-107 FR8, or keep it ContextMenu's own and say so in FR10.
|
/approve-spec d6977f1 |
There was a problem hiding this comment.
Requesting changes. The ContextMenu and List direction carries exact-head owner approval, and the earlier focus-return, Owns and outside-close points are addressed, but two new clauses override current AST-094 as written. After updating, the new head needs a fresh /approve-spec.
- List FR13: scope "the row takes the press paint and lifts as the menu grows from it" to a held finger (AST-094 FR31 and FR33, List FR11 for the press paint), keep a mouse, the keyboard and
presentation: 'popover'on the released menu (AST-094 FR41, public-component-api INV16), name who opts the row into the lift, and add those states to the FR13 verification row. - AST-107 FR8: add "under a finger, as AST-094 FR32, FR33 and FR35 say" to the new outside-press clause, and an outside press with a mouse and with a finger to its verification row.
[Automated review]
| | FR10 | Under a coarse primary pointer (`spec:AST-094` FR13), every ListItem row MUST offer a hit target at least 44 px tall, through its padding or an invisible hit area, with no visual change, where its neighbors leave room: the hit area never overlaps a neighboring row's or control's target, and between close neighbors it is clamped to half the gap; row height and geometry stay as they are at every density. A fine pointer keeps today's hit target. | Owner decision, 2026-10-26: no row-height change, a 44 px touch target | Accepted contract | | ||
| | FR11 | Under a coarse primary pointer, an interactive row's press MUST follow the cell profile (`spec:AST-094` FR11): it paints the `--color-overlay-pressed` token above the row's background and below its content without adding an element, and its release fades over the press model's release duration and curve on the compositor (`spec:AST-094` FR5). At rest the row's paint and stacking are as they were. | `spec:AST-094` FR5, FR11 | Accepted contract | | ||
| | FR12 | List MUST take an `animatedLayout` handle (`spec:AST-105` FR1) and report each row inserted, removed, or moved between renders. With a handle, an inserted row enters, a removed row collapses in place, inert and hidden from assistive technology until it is gone, and a row crossing rows moving the other way rides above them on the list's surface, while the rows after a change glide to their new places (`spec:AST-105` FR3), all starting on one frame on the shared motion role (`spec:AST-105` FR4). Without a handle the list changes at once (`spec:AST-105` FR2). A row's identity is its `itemKey` (`spec:AST-103` DEC-4); a row without one changes at once. | `spec:AST-105` FR1–FR5; `spec:AST-103` DEC-4 | Accepted contract | | ||
| | FR13 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the caller passes the ListItem's ref to `useContextMenu` (`spec:AST-107` FR8), with no ListItem prop of its own; the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), the row takes the press paint and lifts as the menu grows from it, and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly, and a row with an `interactiveRef` takes them on that element; neither shows a "More actions" button; a row with none of these shows a visible "More actions" button named with the row's label, such as "More actions for Inbox", its keyboard and screen-reader path to the menu. When a menu opens, focus moves into it; when it closes, focus returns to the row's focus target, or to the "More actions" button on a row that shows one. Who moves focus there, since `useContextMenu` returns focus to the `<li>`, which takes none, is part of OQ2. A reorderable row always has its handle (`spec:AST-103` FR3), so when it also has a row menu, a hold on the handle picks the row up and a hold anywhere else on the row opens the menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
This clause lifts the row and paints the press on every context request. Current AST-094 confines both to a held finger: FR31 grows a menu from its trigger "Under a finger, on every touch device", FR33 lifts a block trigger that "a held finger opens" and keeps presentation="popover" "without the lift", FR41 keeps a mouse at "its released behavior", and List FR11 paints the press "Under a coarse primary pointer". Suggest: "under a finger, the row takes the press paint (FR11) and lifts as the menu grows from it (spec:AST-094 FR31, FR33); a mouse, the keyboard and presentation: 'popover' keep the released menu", plus naming who opts the row into the lift (AST-107 FR8's factory, or OQ2) and adding a finger long press and a mouse right click to the FR13 verification row.
| move between rows, Home and End jump to the ends, Escape closes, a press | ||
| outside the menu closes it (skipping the compatibility mouse events a | ||
| finger's press sends), and focus returns to the element on close; `destroy` removes every listener it added. `useContextMenu(ref, options) → |
There was a problem hiding this comment.
Under a finger, AST-094 decides this differently: FR32 closes an anchored menu "as a touch outside it begins to pan, and at the lift of a tap or hold outside it", FR33 says a tap outside "reaches nothing beneath it", and FR35 keeps a tap that lands while the menu is still growing from closing it. ContextMenu now hands its outside close to this clause, so suggest adding "under a finger, as spec:AST-094 FR32, FR33 and FR35 say" here, and an outside press with a mouse and with a finger to FR8's verification row.
|
/approve-spec 43a9bbf |
There was a problem hiding this comment.
Requesting changes. The ContextMenu and List row-menu claims carry exact-head owner approval and the earlier lift and outside-press points are addressed, but List FR13 promises a plain row's press paint under a finger's hold that nothing can paint: List FR11 paints only an interactive row's press, Item counts only rows with onClick, href or interactiveRef as interactive, FR13 allows no ListItem prop, and AST-107 FR8 gives the factory no press paint. Please name the plain row's press paint in OQ2 (and Owns) beside the More actions button and focus return, or give it to the factory in AST-107 FR8; the new head then needs a fresh /approve-spec. The pull request description also still calls the List claim FR11 and does not mention OQ2.
[Automated review]
| | FR10 | Under a coarse primary pointer (`spec:AST-094` FR13), every ListItem row MUST offer a hit target at least 44 px tall, through its padding or an invisible hit area, with no visual change, where its neighbors leave room: the hit area never overlaps a neighboring row's or control's target, and between close neighbors it is clamped to half the gap; row height and geometry stay as they are at every density. A fine pointer keeps today's hit target. | Owner decision, 2026-10-26: no row-height change, a 44 px touch target | Accepted contract | | ||
| | FR11 | Under a coarse primary pointer, an interactive row's press MUST follow the cell profile (`spec:AST-094` FR11): it paints the `--color-overlay-pressed` token above the row's background and below its content without adding an element, and its release fades over the press model's release duration and curve on the compositor (`spec:AST-094` FR5). At rest the row's paint and stacking are as they were. | `spec:AST-094` FR5, FR11 | Accepted contract | | ||
| | FR12 | List MUST take an `animatedLayout` handle (`spec:AST-105` FR1) and report each row inserted, removed, or moved between renders. With a handle, an inserted row enters, a removed row collapses in place, inert and hidden from assistive technology until it is gone, and a row crossing rows moving the other way rides above them on the list's surface, while the rows after a change glide to their new places (`spec:AST-105` FR3), all starting on one frame on the shared motion role (`spec:AST-105` FR4). Without a handle the list changes at once (`spec:AST-105` FR2). A row's identity is its `itemKey` (`spec:AST-103` DEC-4); a row without one changes at once. | `spec:AST-105` FR1–FR5; `spec:AST-103` DEC-4 | Accepted contract | | ||
| | FR13 | A row menu on a ListItem attaches to the row's own `<li>`, keeping `ul > li` with no wrapper element: the caller passes the ListItem's ref to `useContextMenu` (`spec:AST-107` FR8), with no ListItem prop of its own; the whole row is its hit area, it opens on the context request (a long press, a right click, or Shift+F10), under a finger, the row takes the press paint (FR11) and lifts as the menu grows from it, `useContextMenu` lifting the element it is attached to (`spec:AST-094` FR31, FR33), while a mouse, the keyboard, and `presentation: 'popover'` open the released menu without the lift (`spec:AST-094` FR41), and focus returns to the row's focus target when the menu closes. A row that is already focusable (`onClick` or `href`) takes Shift+F10 and the context-menu key directly, and a row with an `interactiveRef` takes them on that element; neither shows a "More actions" button; a row with none of these shows a visible "More actions" button named with the row's label, such as "More actions for Inbox", its keyboard and screen-reader path to the menu. When a menu opens, focus moves into it; when it closes, focus returns to the row's focus target, or to the "More actions" button on a row that shows one. Who moves focus there, since `useContextMenu` returns focus to the `<li>`, which takes none, is part of OQ2. A reorderable row always has its handle (`spec:AST-103` FR3), so when it also has a row menu, a hold on the handle picks the row up and a hold anywhere else on the row opens the menu. | Owner decision, 2026-10-26; `spec:AST-107` FR8 | Accepted contract | |
There was a problem hiding this comment.
Under a finger this gives every row with a menu "the press paint (FR11)", but FR11 paints only "an interactive row's press", and Item applies its press layer only when onClick, href or interactiveRef is set (packages/core/src/Item/Item.tsx:612, :879). With "no ListItem prop of its own" and no press paint in AST-107 FR8, nothing can paint a plain row's press. Please name it in OQ2 (and Owns) beside the More actions button and focus return, or state in AST-107 FR8 that the factory paints the press on the element it attaches to, as this clause already has it lift that element.
| | FR12 | List update-motion tests and real-browser frame evidence | an insert, a removal, and a reorder, with and without a handle; reduced motion; a mouse | A staying row that jumps with a handle, motion without one, a collapsing removed row that takes focus, pointer input or an accessibility-tree node, a removed row left after its collapse ends, or layout or paint per frame fails | `audit:List/visual` | | ||
| | Migration | `astryx upgrade` codemod tests | static, aliased, already-migrated, dynamic, and computed-`edge` callsites; a second run | A dynamic value or a computed `edge` rewritten, a static one missed, or a second run that changes output fails | `audit:List/docs` | | ||
| | Documentation | `List.doc.mjs`, Storybook, and `scripts/check-knowledge.mjs` | public prop, values, default, deprecation, and representative composition | Missing or stale contract, docs, or example fails | `audit:List/docs` | | ||
| | FR13 | List tests and real-browser tests of a row menu with a mouse, a finger, and the keyboard | a plain row, a row with `onClick`, a row with `href`; a right click, a long press, Shift+F10, the context-menu key; close by Escape and by choice; a finger's hold, a mouse's right click, the keyboard, and `presentation: 'popover'` | A wrapper element around the `<li>`, a row a context request does not open, focus left in a closed menu or not returned to the row or its button, a plain row without a named "More actions" button, or a focusable row that ignores Shift+F10 fails A lift or press paint under a mouse, the keyboard, or `presentation: 'popover'`, or a finger's hold without them, fails. | `audit:List/layout` | |
There was a problem hiding this comment.
This row fails "a finger's hold without them" on a plain row, which becomes testable once the press-paint owner above is named. Nit: a period is missing before "A lift or press paint".
|
/approve-spec 0e6cced |
There was a problem hiding this comment.
Recommending approval: every changed claim in the ContextMenu, List and AST-107 records carries exact-head spec-owner approval, none contradicts a current record, and check:knowledge passes at this head. Two optional tightenings, each of which would need a fresh /approve-spec:
- List FR13's verification row could add "a row with
interactiveRef", since FR13 gives that row its own clause. - AST-107 FR8's verification row lists an outside press as a state but names no outside-press failure.
[Automated review]
| | FR12 | List update-motion tests and real-browser frame evidence | an insert, a removal, and a reorder, with and without a handle; reduced motion; a mouse | A staying row that jumps with a handle, motion without one, a collapsing removed row that takes focus, pointer input or an accessibility-tree node, a removed row left after its collapse ends, or layout or paint per frame fails | `audit:List/visual` | | ||
| | Migration | `astryx upgrade` codemod tests | static, aliased, already-migrated, dynamic, and computed-`edge` callsites; a second run | A dynamic value or a computed `edge` rewritten, a static one missed, or a second run that changes output fails | `audit:List/docs` | | ||
| | Documentation | `List.doc.mjs`, Storybook, and `scripts/check-knowledge.mjs` | public prop, values, default, deprecation, and representative composition | Missing or stale contract, docs, or example fails | `audit:List/docs` | | ||
| | FR13 | List tests and real-browser tests of a row menu with a mouse, a finger, and the keyboard | a plain row, a row with `onClick`, a row with `href`; a right click, a long press, Shift+F10, the context-menu key; close by Escape and by choice; a finger's hold, a mouse's right click, the keyboard, and `presentation: 'popover'` | A wrapper element around the `<li>`, a row a context request does not open, focus left in a closed menu or not returned to the row or its button, a plain row without a named "More actions" button, or a focusable row that ignores Shift+F10 fails. A lift or press paint under a mouse, the keyboard, or `presentation: 'popover'`, or a finger's hold on a focusable row without them, fails. | `audit:List/layout` | |
There was a problem hiding this comment.
Optional: FR13 gives a row with interactiveRef its own clause (Shift+F10 and the context-menu key on that control, no More actions button), but the representative states list only a plain row, a row with onClick and a row with href. Adding "a row with interactiveRef" would cover it.
| | FR4 | Export check of `@astryxdesign/core` and its hooks | the public exports | a recognizer, the arena, the internal animator module, or the resolver reachable from a public export fails | | ||
| | FR6 | Unit tests of each factory and its hook, server-render and import checks, and listener counts after `destroy` | each factory without React; each hook; a server import; `destroy` mid-drag | a hook with behavior its factory lacks, a listener left after `destroy`, or DOM work at import fails | | ||
| | FR7 | Source boundary check over Core and Lab against the known public-API gap list in `architecture:react-component-runtime` INV13 | every Core and Lab component that drags, snaps, animates one element, reorders, or animates layout | a component reaching internals for a job the public layer covers, or an internal use whose job is not in the gap list, fails | | ||
| | FR8 | Unit and browser tests of `createContextMenu` and `useContextMenu`, and ContextMenu's tests over them | a long press, a right click, and Shift+F10 on a plain element; `open(at)`; `close`; `destroy` while open; `items` and `render`; an outside press with a mouse and with a finger | a context request that opens nothing, a listener left after `destroy`, a hook with behavior of its own, or a ContextMenu that bypasses the factory fails | |
There was a problem hiding this comment.
Optional: the new state "an outside press with a mouse and with a finger" has no matching failure in the last cell, for example "a menu an outside press leaves open, or a finger's compatibility mouse events that close it".
… or finger (spec:AST-107 FR8) meta#8648's FR8 states that a press outside the menu closes it, skipping the compatibility mouse events a finger's press sends, and its verification names an outside press with a mouse and with a finger. A mouse press outside closes the factory's menu; the opening finger's own compatibility mousedown leaves it open, and a later press outside closes it. Both pass: they cover behavior the menu already has.
Main's meta#8648 added ContextMenu FR10 (ContextMenu is built on the public headless context menu), taking the number this branch proposed. Its text and verification row are kept as merged; this branch's open-focus rule becomes proposed FR11, and FR5's closing sentence points at FR11.
…nu sub-menu regions Main's meta#8648 put ContextMenu on the public headless context menu (FR10) and rewrote the record's ownership lines. Her text is kept as merged; this branch adds back only its own surface-host wording (the ownership line, FR2 and its verification row) and proposed FR12 after FR10.
…pec; the factory closes on a press outside, mouse or finger) into ContextMenu on the factory createContextMenu.test.tsx: both sides add cases at its end; this keeps both, and the finger's outside-press cases take the file's shared clock epoch, since the touch-model cases before them have moved the gesture host's clock past a fresh 100 s.
…extMenu's source attaches no request (component:ContextMenu FR10) meta#8648 makes ContextMenu FR10 current: ContextMenu's invocation, open state and cleanup are the factory's. Its verification row names an unmount while open and a source check. Unmounting a ContextMenu while its menu is open removes the menu, the factory's container and the trigger's request (the browser keeps its own menu after); ContextMenu.tsx makes no request attachment call. The source check fails on this PR's first version (883bf8e), whose ContextMenu attached its trigger's request itself; both pass on the factory.
packages/core/src/ContextMenu/ContextMenu.spec.md: main's text as meta#8648 merged it (FR10, its verification row and the layer note), with this PR's DEC-2 verification row re-applied after it.
Decided (Cindy, 2026-10-26 20:46, option (c)).
User impact
A list row can carry a context menu without breaking the list's
ul > limarkup. The whole row is the hit area; a long press, right click, or Shift+F10 opens it; the row shows its press paint and the menu grows from it; and focus comes back to the row on close. Keyboard and screen-reader users always have a path: focusable rows take Shift+F10 and the context-menu key, and plain rows show a visible "More actions" button.Records
component:ContextMenuFR10: ContextMenu is built on the public headless context menu (spec:AST-107FR8, in spec(AST-107): public gesture hooks — usePanGesture, useDragToSnapPoints, useAnimator #8600). Invocation, open state, and cleanup belong to the factory. The ownership line names it, and there's a verification row.component:ListFR13: a ListItem row menu attaches to the row's own<li>, with no wrapper element: the caller passes the ListItem's ref touseContextMenu(spec:AST-107FR8). Under a finger the row takes the press paint and lifts; a mouse, the keyboard, andpresentation: 'popover'open the released menu.component:ListOQ2 (open, to Cindy): who draws a plain row's More actions button, returns focus to the row's target, and paints a plain row's press.onClick/href) takes Shift+F10 and the context-menu key directly. A plain row shows a visible "More actions" button, because List has no roving focus.This merges after #8600 (AST-107 FR8).
check:knowledgepasses. List FR10 is #8571's, so whichever merges second takes the other's row.Writing rubric results