Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
|
/approve-spec 3a77795 |
There was a problem hiding this comment.
Requesting changes: the record now opens the touch menu at the finger's point, but spec:AST-094 FR33, which this pull request leaves unchanged, still says the menu shows beside the lifted trigger, and FR5 keeps "with the menu beside it" next to the new sentence. ORD1 keeps the finger's point inside the trigger, so a menu placed there covers the lifted trigger instead of sitting beside it. Please make the records agree in this pull request. Either amend FR33 and FR5 to the finger's-point placement and say whether the menu paints above the lifted trigger, or anchor the menu beside the trigger's in-place lifted rect; the no-dim and no-move decisions themselves need no change.
[Automated review]
| | FR2 | The `context-menu` target stays on the painted Pointer menu surface and alternative Touch menu surface, not on the Trigger area or cursor anchor. | `architecture:component-theming-surface`; current source | | ||
| | FR3 | Pointer action rows retain DropdownMenu ownership. The touch frame retains BottomSheet ownership, while data-driven touch lists and rows retain List ownership. | Current source and owner docs | | ||
| | FR4 | Compound `menuContent` remains a caller-supplied pointer-menu interior and may also render inside the touch frame; the data-driven touch path instead converts the same item data to List and ListItem presentation. | Current source and tests | | ||
| | FR5 | The Pointer menu surface carries `data-astryx-menu-press` and follows `module:DropdownMenu/useMenuPress` FR1–FR7: the row under the release acts, the highlight follows a held pointer, a mouse released outside closes and a finger leaves the menu open, the stray click never acts, and `touch-action` follows overflow. The long press that opened the menu does not close it as it lifts; a tap outside closes it, and a mouse press outside closes it at the press. Under a finger the menu MUST close as a pan that starts outside it begins (`spec:AST-094` FR32), MUST hand the held finger that opened it to the menu as the opening press, and MUST treat a pen's press outside as FR32 does (FR33). The lift, the grow and their springs apply only to a menu opened by touch (a touch-initiated open on a coarse pointer); a menu opened by mouse or keyboard keeps its released appearance and motion, with no lift and no grow. A touch-opened menu MUST grow out of the finger's point and shrink back into it (`spec:AST-094` FR31) as its own surface, by transform and opacity only (`spec:AST-094` FR5), with no second element such as a glass surface growing behind it, and on a block trigger MUST lift the trigger above the page with the menu beside it (FR33; DD1, DEC-2). The trigger MUST lift where it is: it never moves to make room for the menu, and the menu opens at the finger's point where it fits, placed as the released menu is (ORD1). The trigger lifts on `springs.contextMenuLift` and returns on `springs.contextMenuDismiss`, and the menu grows on `springs.contextMenuMorph` (`spec:AST-094` FR4). Implementation gap: it keeps the layer's own entrance and exit, a scroll that starts outside leaves it open, the opening finger is not handed to the menu so it cannot slide to a row, no trigger lift exists, and a pen press outside closes it at the press. Right-click, Shift+F10, long-press invocation, and keyboard navigation are as FR1 and FR8 state. | `module:DropdownMenu/useMenuPress`; `spec:AST-094` FR26–FR33 | |
There was a problem hiding this comment.
This cell now says both "lift the trigger above the page with the menu beside it" and "the menu opens at the finger's point where it fits, placed as the released menu is (ORD1)". ORD1 keeps the finger's point inside the trigger, so the second placement covers the lifted trigger, and spec:AST-094 FR33 (unchanged here) still says "show the menu beside it". Please make FR5, DEC-2 and FR33 say one placement, and say which paints on top if the menu may overlap the trigger.
…n-place box and never covers it
|
/approve-spec bae3635 |
There was a problem hiding this comment.
Requesting changes: dropping the page dim and keeping the trigger in place is owner-approved, but two things need fixing first. FR5 says the touch menu never covers the lifted trigger, yet it names the released placement, and spec:AST-059 FR4 shifts that placement over the trigger when neither side has room for the menu. Please state the no-room outcome, and the clearance from the grown trigger, in FR5 and its verification row. The description still describes the earlier finger's-point placement and says FR33 already rules out a tint and a shift, so please update it to match the record.
[Automated review]
| | FR2 | The `context-menu` target stays on the painted Pointer menu surface and alternative Touch menu surface, not on the Trigger area or cursor anchor. | `architecture:component-theming-surface`; current source | | ||
| | FR3 | Pointer action rows retain DropdownMenu ownership. The touch frame retains BottomSheet ownership, while data-driven touch lists and rows retain List ownership. | Current source and owner docs | | ||
| | FR4 | Compound `menuContent` remains a caller-supplied pointer-menu interior and may also render inside the touch frame; the data-driven touch path instead converts the same item data to List and ListItem presentation. | Current source and tests | | ||
| | FR5 | The Pointer menu surface carries `data-astryx-menu-press` and follows `module:DropdownMenu/useMenuPress` FR1–FR7: the row under the release acts, the highlight follows a held pointer, a mouse released outside closes and a finger leaves the menu open, the stray click never acts, and `touch-action` follows overflow. The long press that opened the menu does not close it as it lifts; a tap outside closes it, and a mouse press outside closes it at the press. Under a finger the menu MUST close as a pan that starts outside it begins (`spec:AST-094` FR32), MUST hand the held finger that opened it to the menu as the opening press, and MUST treat a pen's press outside as FR32 does (FR33). The lift, the grow and their springs apply only to a menu opened by touch (a touch-initiated open on a coarse pointer); a menu opened by mouse or keyboard keeps its released appearance and motion, with no lift and no grow. A touch-opened menu MUST grow out of the point on its edge nearest the finger and shrink back into it (`spec:AST-094` FR31) as its own surface, by transform and opacity only (`spec:AST-094` FR5), with no second element such as a glass surface growing behind it, and on a block trigger MUST lift the trigger above the page with the menu beside it (FR33; DD1, DEC-2). The trigger MUST lift where it is: it never moves to make room for the menu, and the menu opens beside the lifted trigger's in-place box where it fits, above or below as the released placement flips, and never covers the lifted trigger (FR33). The trigger lifts on `springs.contextMenuLift` and returns on `springs.contextMenuDismiss`, and the menu grows on `springs.contextMenuMorph` (`spec:AST-094` FR4). Implementation gap: it keeps the layer's own entrance and exit, a scroll that starts outside leaves it open, the opening finger is not handed to the menu so it cannot slide to a row, no trigger lift exists, and a pen press outside closes it at the press. Right-click, Shift+F10, long-press invocation, and keyboard navigation are as FR1 and FR8 state. | `module:DropdownMenu/useMenuPress`; `spec:AST-094` FR26–FR33 | |
There was a problem hiding this comment.
"Above or below as the released placement flips" and "never covers the lifted trigger" cannot both hold when neither side of the trigger has room for the menu. The menu grows to 300px, and spec:AST-059 FR4 shifts the released placement into the viewport, over the anchor. Please say what happens in that case here and in the verification row. Also measure the no-cover rule against the lift's grown box: the menu sits flush against the unscaled in-place box (DEC-2), so any growth overlaps it.
…very placement, per AST-059's anchor-clear order
|
/approve-spec 3b24dd4 |
There was a problem hiding this comment.
Requesting changes: dropping the page dim and lifting the trigger in place are owner-approved, and FR5 now keeps the lifted trigger clear in every placement. Two wording fixes remain. DEC-2 places the menu against the trigger's unscaled in-place box while FR5 measures from the box grown by the lift, so please add "grown by the lift" to DEC-2's anchor-box bullet. The description's DEC-2 bullet still says the anchor box sits at the finger's point; please update it to the trigger's in-place box.
[Automated review]
| - an empty box in the layer, at the lifted rect, is what the menu is placed | ||
| against (anchor positioning follows transforms, so the trigger's own cursor | ||
| anchor would move with the lifted trigger); | ||
| - an empty box in the layer, at the trigger's in-place box, is what the menu |
There was a problem hiding this comment.
This anchor box is the unscaled in-place box, but FR5 places the menu beside "the lifted trigger's in-place box, grown by the lift". A menu that keeps clear of this box still meets the grown trigger. Suggest: "an empty box in the layer, at the trigger's in-place box grown by the lift, is what the menu is placed against".
Maintainer impact
A person who holds a finger on a ContextMenu trigger sees the trigger lift where it is, and the page around it stays as it was: no dim, no blur, and the trigger never moves to make room for the menu. The menu opens beside the lifted trigger, above or below where it fits, and never covers it, growing out of its edge nearest the finger.
Exact decision needed
Whether a touch-opened ContextMenu dims the page and moves its trigger. The owner's answer is no to both.
Decision
The owner, 2026-11-02: "I'm thinking we skip darkening the scrim and shifting the position, the lift is fine"
Earlier, 2026-11-01: "context menu appearance on ios seems really touch specific. I don't feel like desktop should have it." and "I think context menu proposal A, we skip the blur"
Changed records
component:ContextMenuonly:spec:AST-059FR4 orders for a layer that keeps its anchor clear: above or below where it fits, then beside it, then the roomier side with the menu capped and scrolling. It never settles over the trigger, even when neither side has room. The verification row adds that case.::backdrop, not an element. The layer stays, painting nothing, because a tap outside must still reach nothing beneath (spec:AST-094FR33). The anchor box now sits at the finger's point, not at the lifted rect, so the menu does not follow the lift away from the finger. One rejected line is added.spec:AST-094is unchanged: FR33 places the menu beside the lifted trigger, which this record keeps. No motion role is removed.Evidence and implementation relationship
This changes what #8201 builds: no page tint and no move of the trigger to fit the menu. The menu is placed beside the lifted trigger and keeps it clear.
Writing rubric results
Rejected:line, inside DEC-2.Scope