Repository navigation
Conversation
|
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.
|
github-actions
Bot
added
the
needs:design-review
8fc6d73 to
7f17a8b
Compare
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsContextMenu (@astryxdesign/core) · View in Storybook
Component 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.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/ContextMenu | +40.2KB |
+12B |
| core/Markdown | +40.4KB |
+11B |
| core/Table | +40.1KB |
+13B |
| core/TableCell | +40.1KB |
+13B |
| core/TableHeaderCell | +40.2KB |
+19B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.9KB | 34.9KB | 0B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
57 of 504 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
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.
Approval is on hold for an engineering-owner decision: this pull request adds direction that no current spec states, namely draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2), the ContextMenu lift for touch-opened block triggers, and the new --color-background-backdrop dim that replaces the blurred scrim on Dialog, Drawer, MobileNav and BottomSheet, and its FR11 and FR14 would change the current design:user-states DR1 and spec:AST-025 FR14, which those records would then need to say. Any engineering owner, including the author as one, can record approval by commenting /approve-spec <full head SHA>, or reject or split any of these. Whatever is decided, these need fixing before approval:
- A block trigger without its own background lifts as a see-through copy over the still-visible trigger, so the ContextMenu Default story shows its label and border twice.
lintruns out of memory on this head (as on #8074 and #8125) while it passes onmain.- The range carries 26,730 authored lines because the stacked predecessors are unmerged; landing them first, or basing this pull request on its predecessor, leaves this commit's own change to review.
Smaller fixes (the close-path Effect, the unused safe-area clamp, right-to-left menu alignment, the zh guidance and a citation of the unmerged AST-090) are detailed in the review.
[Automated review]
| } | ||
|
|
||
| /** A copy of the trigger to lift: inert, unnamed, with no ids to collide. */ | ||
| function clonePreview(trigger: HTMLElement): HTMLElement { |
There was a problem hiding this comment.
The copy is cloned as is and the trigger stays visible under it, so a trigger without its own background (the Default story's dashed area) lifts as a see-through copy and its label and border show twice. Hiding the source trigger from lift() until drop() finishes, or giving the copy an opaque surface, would fix it; contextMenuPressFeedback.ts:72-75 already says the trigger "is hidden while its preview is up".
| // A popover reports itself open a task after the press asked (its toggle | ||
| // event), so only an open → closed change ends the touch-opened menu. | ||
| const wasOpenRef = useRef(isOpen); | ||
| useEffect(() => { |
There was a problem hiding this comment.
This Effect sets isTouchOpen and liftPlacement in reaction to isOpen, so the render after a close still carries the touch-open state. architecture:react-component-runtime INV2 asks for this work in the handler that causes it: useLayer's onHide (and updateOpenState(false) for the sheet).
| ); | ||
| } | ||
|
|
||
| function viewportOf(doc: Document): LiftViewport { |
There was a problem hiding this comment.
liftedBox() and menuPlacement() subtract safeTop and safeBottom, but this never sets them, so on a viewport-fit=cover page the copy can lift to 20 px from the screen edge, under the status bar.
| : 'above'; | ||
| const epsilon = 0.5; | ||
| const alignment = | ||
| lifted.x <= screenInset + epsilon |
There was a problem hiding this comment.
This picks start for the physical left edge, but the layer reads alignment logically (span-self-inline-end), so in a right-to-left page the menu is flush with the copy's inner edge instead of the edge it is held against.
| { | ||
| guidance: true, | ||
| description: | ||
| '块级触发区域周围请留出少量空间：按住时它会从中心放大约 6%，直到菜单关闭。', |
There was a problem hiding this comment.
The zh guidance still says the trigger stays grown until the menu closes; the English guidance and the code return it to its size when the copy lifts about 13 %.
7f17a8b to
1d09473
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the touch-presentation direction this pull request introduces: draft spec:AST-094, the iOS ContextMenu lift, the unblurred modal dim token and the new 744 / 500 px compact rule. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever the decision, these changes are needed:
- Rename
--color-background-backdrop, which breaks the colour token grammar (theme-tokens INV10) and failstest-ui, and regenerate the native Swift theme after rebasing onmain. - Clear the two strict-lint
flushSyncerrors atContextMenu.tsx:779and:790. - Split the 28,130-line range so that this pull request reviews the ContextMenu press and lift alone.
- Hide the source trigger under the lifted copy, move the close reset out of the Effect, feed the safe-area insets to the lift geometry, and fix right-to-left menu alignment.
- Correct the Chinese guidance in
ContextMenu.doc.mjsand the AST-090 citation in the spec.
[Automated review]
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
1d09473 to
e01d233
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
Converged in e01d233:
|
|
Addressed in e01d233:
|
…nd a blur-on frame
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on five pieces of direction that no current spec states: component:ContextMenu DEC-2 (the four elements the lift adds while open), the three context-menu spring rows added to spec:AST-094 FR4, the new architecture:layer-runtime INV13 (the exit copy), the page scroll lock while a menu a finger opened is up, and the public useLongPress stages option. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- The range carries the whole unmerged stack (37,674 authored lines); landing it first would leave this diff as the lift alone.
- Two repository guards that pass on
mainfail at this head: the press-coverage test, whose widened press arm now matcheshooks/actionGroupTracker.ts, and the touch-support bundle test's root-entry case. - The lift layer's
touch-action: none(liftFromTrigger.ts:530) blocks pinch-zoom while the menu is up, againstspec:AST-094FR10, FR32 and FR35, and no test pinches over the lift. - A pan outside a finger's menu does not close it (FR32), a trigger without its own background lifts see-through, the press spring is not an FR4 motion role, the lifted open calls
onOpenChange(true)from a layout effect, the lifted menu vanishes instead of shrinking back (FR31), and the AST-057 edit describes motion no code plays.
[Automated review]
| color: 'inherit', | ||
| overflow: 'visible', | ||
| pointerEvents: 'auto', | ||
| touchAction: 'none', |
There was a problem hiding this comment.
This layer covers the whole viewport while a trigger is lifted, so touch-action: none here stops page pinch-zoom for as long as the menu is up (probed: computed none over 390 × 664 on the lifted Default story). spec:AST-094 FR10, FR32 ("its surface never takes touch-action: none") and FR35 ("A pinch still zooms the page") ask for the opposite; touchAction: 'pinch-zoom' still keeps one-finger pans and taps off the page.
| return; | ||
| } | ||
| pendingLiftRef.current = null; | ||
| openLifted(lifted); |
There was a problem hiding this comment.
openLifted calls layer.show(), which runs onShow and with it the consumer's onOpenChange(true) from this layout effect. architecture:react-component-runtime INV2 wants the callback fired from the handler that caused the open (openFromFinger); the effect can keep the show, the measurement and the flip.
| // UIKit's context menu. A sheet locks the page itself. (The lifted trigger | ||
| // and the menu's anchor are fixed to the viewport in the top layer, so | ||
| // pinning the page moves neither.) | ||
| useScrollLock(isOpen && isTouchOpen && !usesBottomSheet); |
There was a problem hiding this comment.
No current record states this lock: spec:AST-094 FR19 allows one only under a modal presentation, and FR32 wants an anchored menu to close as a touch outside it begins to pan. Today a pan outside a finger's menu neither scrolls nor closes it (the outside handler listens for mousedown, which a pan never sends).
| @@ -63,7 +66,8 @@ const source = new Map( | |||
| files.map(file => [path.relative(SRC, file), fs.readFileSync(file, 'utf8')]), | |||
| ); | |||
|
|
|||
| const PRESS_ARM = /':active|'\[data-astryx-press=|interactionOverlayStyles\./; | |||
| const PRESS_ARM = | |||
| /':active|'\[data-astryx-press=|interactionOverlayStyles\.|getAttribute\(PRESSED_ATTRIBUTE\)/; | |||
There was a problem hiding this comment.
The widened arm also matches main's hooks/actionGroupTracker.ts (getAttribute(PRESSED_ATTRIBUTE) at lines 271 and 441), which has no usePressFeedback(), so this guard fails at this head and passes on the base. The body points the fix at #8638, which this head does not carry.
| - **Gesture tuning.** Axis-lock distance, commit ratio, fling velocity, | ||
| resistance past the panel: behavior constants in source, not public API and | ||
| not design tokens (the same ruling the touch press model received for its | ||
| clocks). The row's slide and spring motion plays on theme motion roles |
There was a problem hiding this comment.
No code here moves the row swipe onto motion roles: Item still settles on SWIPE_SETTLE_MS = 200 (useSwipeAction.ts:86, :368), and spec:AST-094 FR4 and DEC-11 keep touch motion roles internal, so no theme reaches them. This edit fits the pull request that puts the swipe on roles.
…tation has run Under the touch model the menu opens with a presentation, and the layer's input hold (spec:AST-094 FR35) drops every tap until that presentation completes, as UIKit drops a touch while one runs. The check tapped the shadow host two frames after the lift, so its tap landed on the hold's shield and the menu stayed open. It now taps once the page's animations have finished, and its assertions are unchanged.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on five pieces of direction that no current spec states: component:ContextMenu DEC-2 (the four elements the lift adds while open), the three context-menu spring rows added to spec:AST-094 FR4, the new architecture:layer-runtime INV13 (the exit copy), the page scroll lock while a menu a finger opened is up, and the public useLongPress stages option. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- The range carries the whole unmerged stack (37,748 authored lines); landing it first would leave this diff as the lift alone.
- Two repository guards that pass on
mainfail at this head: the press-coverage test, whose widened press arm now matcheshooks/actionGroupTracker.ts, and the touch-support bundle test's root-entry case. - The lift layer's
touch-action: none(liftFromTrigger.ts:530) blocks pinch-zoom while the menu is up, againstspec:AST-094FR10, FR32 and FR35, and no test pinches over the lift. - A trigger without its own background lifts see-through (its label drops to about 1.8:1 over the default tint), a pan outside does not close a finger's menu (FR32), the lifted menu vanishes instead of shrinking back (FR31), the lifted open calls
onOpenChange(true)from a layout effect, the press spring is not an FR4 motion role, and the AST-057 edit describes motion no code plays.
[Automated review]
| color: 'inherit', | ||
| overflow: 'visible', | ||
| pointerEvents: 'auto', | ||
| touchAction: 'none', |
There was a problem hiding this comment.
This viewport-wide layer takes every touch outside the menu while the trigger is lifted, and touch-action: none also takes pinch-zoom from the page (probed: computed none over 390 × 664 on both lifted stories); the input hold's pinch-zoom shield is appended inside it, so it cannot restore the pinch. spec:AST-094 FR10, FR32 ("its surface never takes touch-action: none") and FR35 keep pinch-zoom the browser's. touchAction: 'pinch-zoom' still keeps one-finger pans and taps off the page.
| 'padding-bottom', | ||
| 'padding-left', | ||
| 'color', | ||
| 'background-color', |
There was a problem hiding this comment.
Keeping the laid-out background-color leaves a trigger without its own background transparent over the tint: on the ContextMenu Default story its #6b7280 label goes from 4.83:1 on white to 1.21:1 in the captured frame (1.83:1 over Core's default light --color-overlay). An opaque surface under such a trigger, or a documented and tested requirement that a block trigger carry its own background, keeps the lifted item legible.
| return; | ||
| } | ||
| pendingLiftRef.current = null; | ||
| openLifted(lifted); |
There was a problem hiding this comment.
openLifted calls layer.show(), which runs onShow synchronously and with it the consumer's onOpenChange(true), from this layout effect. architecture:react-component-runtime INV2 has the callback fire from the handler that caused the open (openFromFinger); the effect can keep the show, the measurement and the flip.
| // UIKit's context menu. A sheet locks the page itself. (The lifted trigger | ||
| // and the menu's anchor are fixed to the viewport in the top layer, so | ||
| // pinning the page moves neither.) | ||
| useScrollLock(isOpen && isTouchOpen && !usesBottomSheet); |
There was a problem hiding this comment.
No current record states this lock: spec:AST-094 FR19 allows one only under a modal presentation, and FR32 wants an anchored menu to close as a touch outside it begins to pan. With it, a pan outside a finger's menu neither scrolls nor closes it (the outside handler listens for mousedown, which a pan never sends).
| attribute: 'data-astryx-context-menu-lift', | ||
| }); | ||
| }, | ||
| enterMenu(menu) { |
There was a problem hiding this comment.
The menu enters through presentSurface, but nothing calls the lift module's dismissSurface, and the lifted placement has no layerAnimations, so the menu disappears at once while the trigger drops back. spec:AST-094 FR31 has an anchored menu close by shrinking back, and layer-runtime INV15 plays the lift module's exit on the runtime's exit copy.
| * ContextMenu's touch code calls it before its first motion (it is | ||
| * idempotent). | ||
| */ | ||
| export function registerContextMenuTable(): void { |
There was a problem hiding this comment.
This registers the table with no roles, so the press feedback's growth and early return play contextMenu.press.feedbackSpring as a table value (contextMenuPressCurve.ts:73, contextMenuPressFeedback.ts:185). spec:AST-094 FR4: "A component MUST read touch motion through these roles and MUST NOT play a literal"; DEC-11 keeps roles internal, not absent.
| @@ -63,7 +66,8 @@ const source = new Map( | |||
| files.map(file => [path.relative(SRC, file), fs.readFileSync(file, 'utf8')]), | |||
| ); | |||
|
|
|||
| const PRESS_ARM = /':active|'\[data-astryx-press=|interactionOverlayStyles\./; | |||
| const PRESS_ARM = | |||
| /':active|'\[data-astryx-press=|interactionOverlayStyles\.|getAttribute\(PRESSED_ATTRIBUTE\)/; | |||
There was a problem hiding this comment.
The widened arm also matches main's hooks/actionGroupTracker.ts:271 (getAttribute(PRESSED_ATTRIBUTE)), which has no usePressFeedback(), so this guard fails at this head and passes on the base. The body points the fix at #8638, which this head does not carry.
| - **Gesture tuning.** Axis-lock distance, commit ratio, fling velocity, | ||
| resistance past the panel: behavior constants in source, not public API and | ||
| not design tokens (the same ruling the touch press model received for its | ||
| clocks). The row's slide and spring motion plays on theme motion roles |
There was a problem hiding this comment.
No code here moves the row swipe onto motion roles: Item still settles on SWIPE_SETTLE_MS = 200 (useSwipeAction.ts:86, :368), and spec:AST-094 FR4 and DEC-11 keep touch motion roles internal, so no theme reaches them. This edit fits the pull request that puts the swipe on roles.
… the presented start A zoom stage already drives its photo with an animator (pinch, pan, settle), and the zoom's grow is a rect morph on the same element: two animators would overwrite each other's channels. createRectMorph takes that animator in `animator` and plays on it; dispose leaves it to its owner. The morph's presentedRect then reads the stage's own transform, so a dismissal from a zoomed photo starts from the shown rect. A present or a dismissal also takes the animator's `start`: with 'presented', the element holds its first values until two frames have presented it, so a surface that appears late (a dialog's first frame in WebKit) plays the whole curve. Both new cases fail before this change and pass after.
…only moves Under Reduce Motion on a touch device, the morph treats a path with no opacity of its own as an entrance on a present and an exit on a dismissal, so the cross-fade lands it at rest (design:reduced-motion). An element that stays visible and only moves, such as a sub-menu's parent card, then cut out and faded in on present, and faded out and popped back on dismissal, with no way for its caller to say so: the animator's crossfadeAs names only an entrance or an exit. A present or a dismissal now takes crossfadeAs: 'move', which passes no cross-fade direction, so the animator plays its own dip: out, the travel lands, back in. The new case does not compile before this change (the option had no value for a move; an untyped 'move' reached the animator only by accident) and passes after.
…t morph's source copy AST-057's record takes main's text throughout (meta#8646 drops two-finger trackpad swipes from the non-goals); this branch keeps only its own change there: the row's slide and spring motion plays on theme motion roles (spec:AST-094 FR4), so its durations leave the behavior constants.
…er work in the lift's task A busy page does other work in the task that delivers the lift. The menu still shows in that task, before the next task and the next frame, but the check measured the time from the lift to the show against one frame (1000/60 ms) of wall clock, so it failed whenever the lift's task ran long: once per engine at a load of 67. With 40 ms of injected work it fails every time (49 ms in Chromium, 51 ms in WebKit). Red: the next commit measures the order instead.
…r, not a frame of wall clock The check now notes the lift at the finger's first pointer or touch event, queues a task and an animation frame there, and records the menu's show among them: the show comes first, in the lift's own task, before the next task and the next frame. That is what "before the next frame could paint" means, and it holds however long the lift's task runs: with the 40 ms of injected work it passes in both engines, three times in a row.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on five pieces of direction that no current spec states: component:ContextMenu DEC-2 (the four elements the lift adds while open), the three context-menu spring rows added to spec:AST-094 FR4, the new architecture:layer-runtime INV13 (the exit copy), the page scroll lock while a menu a finger opened is up, and the public useLongPress stages option. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- The range carries the whole unmerged stack (37,832 authored lines); landing it first would leave this diff as the lift alone.
- Two repository guards that pass on the base fail at this head: the press-coverage test, whose widened press arm now matches
hooks/actionGroupTracker.ts, and the touch-support bundle test's root-entry case. - The lift layer's
touch-action: none(liftFromTrigger.ts:530) blocks pinch-zoom while the menu is up, againstspec:AST-094FR10, FR32 and FR35, and no test pinches over the lift. - A trigger without its own background lifts see-through (its label drops to about 1.8:1 over the default tint), a pan outside does not close a finger's menu (FR32), the lifted menu vanishes instead of shrinking back (FR31), the lifted open calls
onOpenChange(true)from a layout effect, the press spring is not an FR4 motion role, and the AST-057 edit describes motion no code plays.
[Automated review]
| color: 'inherit', | ||
| overflow: 'visible', | ||
| pointerEvents: 'auto', | ||
| touchAction: 'none', |
There was a problem hiding this comment.
This layer covers the whole viewport while the trigger is lifted, so touch-action: none stops page pinch-zoom over the menu. spec:AST-094 FR10, FR32 and FR35 keep the pinch the browser's; 'pinch-zoom' still keeps one-finger pans and taps from reaching the page.
| 'padding-bottom', | ||
| 'padding-left', | ||
| 'color', | ||
| 'background-color', |
There was a problem hiding this comment.
Keeping the laid-out background-color means a trigger that takes its background from the page lifts see-through: the Default story's grey label drops from 4.83:1 to about 1.8:1 over the default light --color-overlay. An opaque surface under a transparent trigger (or a documented requirement) would keep it legible.
| return; | ||
| } | ||
| pendingLiftRef.current = null; | ||
| openLifted(lifted); |
There was a problem hiding this comment.
openLifted runs layer.show(), whose onShow calls the consumer's onOpenChange(true) from this layout effect. architecture:react-component-runtime INV2 wants it from the handler that caused it (openFromFinger); the effect can keep the show, the measurement and the flip.
| }); | ||
| }, | ||
| enterMenu(menu) { | ||
| lift.presentSurface(menu, null); |
There was a problem hiding this comment.
The menu enters through presentSurface, but nothing calls dismissSurface, so on close the menu disappears at once while the trigger drops back; spec:AST-094 FR31 wants it to shrink back into the trigger on the runtime's exit copy.
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.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on six pieces of direction that no current spec states: component:ContextMenu DEC-2 (the four elements the lift adds while open), the three context-menu spring rows added to spec:AST-094 FR4, the new architecture:layer-runtime INV13 (the exit copy), the page scroll lock while a menu a finger opened is up, the public useLongPress stages option, and whether the lift must first move into the createContextMenu factory that spec:AST-107 FR8 names. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- The range carries the whole unmerged stack (37,874 authored lines); landing it first would leave this diff as the lift alone.
- The lift layer's
touch-action: none(liftFromTrigger.ts:530) blocks pinch-zoom while the menu is up, againstspec:AST-094FR10, FR32 and FR35. - The press-coverage and touch-support bundle guards pass on the base and fail at this head; their fixes in #8638 and #8671 are not in this range.
- The lifted menu disappears at the tap instead of shrinking back through the exit copy (FR31, layer-runtime INV15), and the lifted open calls
onOpenChange(true)from a layout effect. - A trigger without its own background lifts see-through (its label drops from 4.83:1 to about 1.2:1 over the tint), ContextMenu's touch-only size needs its INV6 justification, and the AST-057 edit describes motion no code plays.
[Automated review]
| color: 'inherit', | ||
| overflow: 'visible', | ||
| pointerEvents: 'auto', | ||
| touchAction: 'none', |
There was a problem hiding this comment.
This viewport-wide layer takes every touch on the tint, so touch-action: none leaves the browser no pinch-zoom while the menu is lifted, against spec:AST-094 FR10, FR32 and FR35. touchAction: 'pinch-zoom' still keeps one-finger pans off the page; a lift case in the browser spec that pinches on the tint would pin it.
| 'padding-bottom', | ||
| 'padding-left', | ||
| 'color', | ||
| 'background-color', |
There was a problem hiding this comment.
Keeping the laid-out background-color means a transparent trigger (the Default story's dashed area) lifts see-through: its #6b7280 label drops from 4.83:1 on the page to about 1.2:1 over the tint. An opaque fallback surface when the trigger's background is transparent would keep the lifted item legible.
| attribute: 'data-astryx-context-menu-lift', | ||
| }); | ||
| }, | ||
| enterMenu(menu) { |
There was a problem hiding this comment.
enterMenu and drop are exposed, but nothing reaches the lift's dismissSurface, so a lifted menu hides at the tap with no exit copy, while spec:AST-094 FR31 and architecture:layer-runtime INV15 want it to shrink back through the runtime's exit copy (and the proposed DEC-2 says that copy joins). Calling dismissSurface(menu) while the menu still shows, before the layer hides it, would close the gap.
| return; | ||
| } | ||
| pendingLiftRef.current = null; | ||
| openLifted(lifted); |
There was a problem hiding this comment.
openLifted calls layer.show(), whose onShow runs onOpenChange?.(true) synchronously, so the consumer callback fires from this layout effect rather than from the handler (architecture:react-component-runtime INV2). Showing the layer from openFromFinger and keeping only the measurement and flip here would keep the callback in the handler.
| @@ -63,7 +66,8 @@ const source = new Map( | |||
| files.map(file => [path.relative(SRC, file), fs.readFileSync(file, 'utf8')]), | |||
| ); | |||
|
|
|||
| const PRESS_ARM = /':active|'\[data-astryx-press=|interactionOverlayStyles\./; | |||
| const PRESS_ARM = | |||
| /':active|'\[data-astryx-press=|interactionOverlayStyles\.|getAttribute\(PRESSED_ATTRIBUTE\)/; | |||
There was a problem hiding this comment.
Widening the arm to getAttribute(PRESSED_ATTRIBUTE) makes hooks/actionGroupTracker.ts on main match without the marker, so this guard fails at this head (test-ui (2/4)) while it passes on the base; the fix in #8638 is not in this range.
Depends on #8125 and #8485. It carries the static-import offer on #8125 (16dc55e) as a dependency; if #8125 lands differently, this stack follows her decision. Own commits: vjeux/astryx@16e1f21...e2edd39, vjeux@6ee4d62801, vjeux@5ea4de4f62, vjeux@bf71cc6f0d, vjeux@e3379dc53c, vjeux@0a5ad53a5a, vjeux@413b599e4a, vjeux@1271081b23, vjeux@a62d28d40d, vjeux@2c9794db08, vjeux@c997f735ee, vjeux@7ba52120e6, vjeux@e20a9343e8, vjeux@895b75d53f, vjeux@b8b636df5c, vjeux@3d038b9b72, vjeux@533e88113b, vjeux@fa073be1b5, vjeux@7df4c952d1
User need
Part of the UIKit-parity program. On iOS, a finger held on a view with a context menu lifts the view above a tinted page, the menu beside it; ContextMenu gives a phone the desktop menu at the finger (recordings below).
Problem and solution fit
On every touch device (
spec:AST-094FR13, FR33), as measured on iOS 26.2: the block trigger itself lifts with a shadow (26 pt larger on its longer side, at most 15%) while its spot stays held; a tint fades in, and tapping it closes the menu; while the lift grows, taps reach nothing (FR35); the menu sits 16 pt from it, below if it fits; Reduce Motion fades it in. Fine pointers, inline triggers,presentation="popover"and the sheet keep today's menu.Current specification
component:ContextMenu(draft) FR5,spec:AST-094FR31–FR33 and FR35,design:reduced-motion; the lift is #8485's module (architecture:layer-runtimeINV15). Owner decisions, pending:--color-overlay, the shadow--shadow-high.component:ContextMenu/DEC-2: open, the lift adds four elements, withinspec:AST-017FR60.Public delta and caller burden
No new token or prop. ContextMenu's JavaScript grows from 79,600 to 120,254 B gzip. Public-API gaps (
spec:AST-107FR7): the lift, its springs and UIKit tables, the input hold.Design and alternatives
A copy of the trigger is a second React tree that grows with its content; lifting the trigger itself costs the same whatever it holds and keeps its state, so it lifts. Open gap: a touch trigger keeps a
touchmovelistener at rest.Evidence
iOS 26.2 simulator, then Android 15:
Scope
Testing
In the story "Touch / long press", on a phone or the iOS simulator:
pnpm test:interaction ContextMenuLift ContextMenuLongPressruns these under Chromium's and WebKit's own finger. Unrelated failures: #8638, #8671.ContextMenuLongPress's lift-after-commit case used to time the menu against wall clock and failed under load; it now checks that the menu shows in the lift's own task, before the next task and frame, with 40 ms of other work in that task.Writing rubric results
R1: present tense. R2: the element counts are INV7's required evidence. R3: verification rows name tests and measurements.