Repository navigation
feat(sandbox): the motion lab previews TopNav's mega-menu exit with the missing Reduce Motion branch - #8607
feat(sandbox): the motion lab previews TopNav's mega-menu exit with the missing Reduce Motion branch#8607cixzhang wants to merge 7 commits into
Conversation
…he missing Reduce Motion branch The live previews page gets a TopNav mega-menu card: core's real TopNav, opened and closed on a loop through its own trigger, beside the same component carrying the reduced-motion branch it lacks as a sandbox prototype in CSS. Under Reduce Motion (or the lab's Degrade) the panel only fades, within 200 ms; under Delete it cuts. The panes open half a loop apart so their auto popovers don't close each other. An enter-and-exit slider tunes the duration core reads. Core's TopNav is unchanged; the card notes that animator retargeting lands with useAnimator. Co-authored-by: Ernest <ernesttien@meta.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
|
The lab's sliders tune its semantic tokens, and TopNav's panel reads the primitive duration, so the enter-and-exit slider read 0 ms and moved nothing. Co-authored-by: Ernest <ernesttien@meta.com>
There was a problem hiding this comment.
Changes requested: the TopNav mega-menu exit card is a useful comparison, but its loop and its Proposed branch need fixes first.
- The loop's scripted
trigger.click()arrives withdetail: 0, which core'suseMenuHovertreats as Enter or Space, so each open moves focus into the panel (about every 1.6 s, anywhere on the page) and the scheduled close never closes it; with Show set to Today or Proposed, or with Loop off, no exit plays. - Under Reduce Motion the Proposed branch fades for 200 ms with any pointer, but
design:reduced-motionDR3 requires an immediate change when the primary pointer is not coarse, so keep the fade for(pointer: coarse)and cut otherwise. - Smaller fixes: the card ignores the rail's slow-motion speed, "the only component in core that animates a popover exit" overlooks
useKeyboardHint's fading hint, and the AST-107 note presents an open spec PR as already landing.
[Automated review]
| 'button[aria-expanded]', | ||
| ); | ||
| if (trigger != null && (trigger.ariaExpanded === 'true') !== isOpen) { | ||
| trigger.click(); |
There was a problem hiding this comment.
HTMLElement.click() dispatches a click with detail: 0, which core's useMenuHover treats as Enter or Space (useMenuHover.ts:347-362): each open moves focus to the panel's first link, and the scheduled close only refocuses the panel instead of closing it. On the previews page this pulls focus into the card about every 1.6 s, and with Show set to Today or Proposed (or Loop off) the panel never closes. Close with a pointer-style click (detail: 1), and keep focus where it was on open: hover opens keep focus on the trigger, or restore the previously focused element.
| than the 200 ms cross-fade (design:reduced-motion DR2). The panel stays a | ||
| DOM descendant in the top layer, so the scope reaches it. The lab's | ||
| Degrade applies the same branch; Delete cuts. */ | ||
| @media (prefers-reduced-motion: reduce) { |
There was a problem hiding this comment.
This fades for every pointer. design:reduced-motion DR3 requires an immediate change, with no animated property, when the primary pointer is not coarse; DR2's 200 ms cross-fade is for coarse pointers only. Consider @media (prefers-reduced-motion: reduce) and (pointer: coarse) for the fade and transition-duration: 0s otherwise, and update the pane label and hint to match.
| CSS. Animator retargeting lands with <code>useAnimator</code>{' '} | ||
| (public, AST-107, #8600). |
There was a problem hiding this comment.
spec:AST-107 is not on main yet; #8600 is an open spec PR. Suggest wording this as proposed rather than landing.
…and its branch changes at once with a mouse The loop's click() sent detail 0, which the menu reads as Enter or Space, so each open moved focus into the panel and the scheduled close only refocused it. It now dispatches a pointer-style click (detail 1) and puts focus back where it was. The prototype's Reduce Motion branch fades only under a coarse pointer and changes at once with a fine one (design:reduced-motion DR2, DR3); the pane label and hint say so, and the card words useAnimator as proposed (AST-107 is an open spec PR). Co-authored-by: Ernest <ernesttien@meta.com>
Restoring focus to the body is a no-op, so after a loop click the trigger kept the focus the menu gave it. When nothing had focus, the loop now blurs whatever the click focused. Co-authored-by: Ernest <ernesttien@meta.com>
…ng had focus The previous commit's message described this change, but its edit didn't apply; this commit carries it. Restoring focus to the body is a no-op, so the trigger kept the focus the menu gave it. Co-authored-by: Ernest <ernesttien@meta.com>
There was a problem hiding this comment.
Changes requested: the TopNav mega-menu exit card now closes on schedule, leaves focus where it was, and its Proposed branch meets the reduced-motion rules for both touch and mouse, but two small fixes remain.
- The card says TopNav is core's only popover exit (
previews/page.tsx:202,LabDemos.tsx:1004), but the arrow-key hint that SegmentedControl, TabList and Toolbar show also fades out over 150 ms, so say TopNav's is the only one that travels. - The Preview plan's
topnavrow still hasstatus: 'specified'andrig: null(previewPlan.ts:585-586), so the plan page lists this preview as unbuilt; mark it built withTopNavExitRig.
[Automated review]
|
|
||
| <DemoCard | ||
| title="TopNav mega-menu exit" | ||
| question="The only component in core that animates a popover exit, and it never checks Reduce Motion." |
There was a problem hiding this comment.
Core has a second animated popover exit: the arrow-key hint that SegmentedControl, TabList and Toolbar show through useKeyboardHint fades out over 150 ms (opacity, display, overlay with allow-discrete, packages/core/src/hooks/useKeyboardHint.tsx:130-137). Saying TopNav's is the only popover exit that travels would keep this accurate; the Today hint at LabDemos.tsx:1004 makes the same claim.
…hat travels, and the plan lists it as built Core has a second popover exit, the keyboard hint's 150 ms fade, so the card, its Today hint and the preview plan now say TopNav's is the only popover exit that travels. The plan's TopNav row is built, on TopNavExitRig. Co-authored-by: Ernest <ernesttien@meta.com>
There was a problem hiding this comment.
Changes requested: the TopNav mega-menu exit card now meets the reduced-motion rules for both touch and mouse and the plan lists it as built, but one copy fix remains. "The only popover exit in core that travels" (previews/page.tsx:202, LabDemos.tsx:1004, previewPlan.ts:574) is still inaccurate, because a non-modal Drawer (hasScrim={false}) is a manual popover whose exit slides out (packages/core/src/Drawer/Drawer.tsx:564), so please scope it, for example to "the only anchored popover exit in core that travels".
[Automated review]
|
|
||
| <DemoCard | ||
| title="TopNav mega-menu exit" | ||
| question="The only popover exit in core that travels, and it never checks Reduce Motion." |
There was a problem hiding this comment.
"The only popover exit in core that travels" is still not accurate: a non-modal Drawer (hasScrim={false}) opens as a <dialog popover="manual"> (packages/core/src/Drawer/Drawer.tsx:564) and slides out before that popover closes (Drawer.tsx:30-31, :554-555). Scoping it, for example to "The only anchored popover exit in core that travels", keeps it true; the Today hint (LabDemos.tsx:1004) and the plan's gap line (previewPlan.ts:574) carry the same sentence.
A non-modal Drawer's popover exit slides too, so the card, its Today hint and the plan row describe TopNav's anchored panel exit without calling it the only one. Co-authored-by: Ernest <ernesttien@meta.com>
There was a problem hiding this comment.
Changes requested: the TopNav mega-menu exit card's Proposed branch meets the reduced-motion rules at this head, but core shipped the same branch for the mega-menu panel in #8612, so on main the Today pane no longer rises under Reduce Motion. Once merged, the card's Today label, hint and question and the Preview plan's gap line (previews/page.tsx:202-222, LabDemos.tsx:1004, previewPlan.ts:574) would describe a gap that no longer exists, with both panes behaving identically. Please rebase onto main and rework the card for the shipped branch, for example by comparing a copy of the old 4 px rise with core's real panel, or by retiring the card and updating its plan row.
[Automated review]
| panes={[ | ||
| { | ||
| tone: 'before', | ||
| label: 'Today — the same rise under Reduce Motion', |
There was a problem hiding this comment.
On main, #8612 gives core's mega-menu panel this same reduced-motion branch, so once this merges the Today pane no longer rises under Reduce Motion: in this head's CI sandbox build (merged with main) both panes open identically. This label, the question on line 202, the body on lines 220-222, the hint at LabDemos.tsx line 1004 and the plan's gap line need rewriting for the shipped branch.
The motion lab's live previews page gets a TopNav mega-menu exit card. TopNav is the only component in core that animates a popover exit, and it never checks Reduce Motion.
useAnimator(public, AST-107, spec(AST-107): public gesture hooks — usePanGesture, useDragToSnapPoints, useAnimator #8600).Evidence
Real Chrome (headless, Linux), rendering this PR's CI sandbox build at 0c058e4: desktop with a mouse (fine pointer), phone with touch (coarse pointer). The two panes open half a loop apart.
nonewith a 0.2 s opacity fade under touch, and 0 s (at once) with a mouse.