Repository navigation
test(TabList): TabMenu's arrow-key tests wait for the open's focus to land - #8493
Conversation
… land Opening the TabMenu moves focus to its first item a frame later (requestAnimationFrame, APG menu-button focus-on-open). Two keyboard tests pressed ArrowDown right after the click and expected it to land on the first item, which holds only while that frame has not fired: with nothing focused, ArrowDown focuses the first item; once focus is there, it moves on to the second. On a loaded CI runner the frame fired first and the test failed with focus on Reports. Both tests now wait for focus on the first item before pressing arrows, and expect ArrowDown to reach the second item and ArrowUp to come back.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
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.
This looks good to merge: both TabMenu arrow-key tests now wait for the open's focus to land on the first item, so they pass whether that frame fires before or after the first key, and no shipped code changes. The fix matches the failure it targets: when the frame fires first, the previous assertions fail with focus on Reports while the new ones pass, and the new ones also fail if focus-on-open ever regresses. No changes are needed.
[Automated review]
There was a problem hiding this comment.
This looks good to merge: both TabMenu arrow-key tests now wait for the open's focus to land on the first option, so they pass whichever way that frame fires, and no shipped code changes. When the frame fires before the first key, the previous assertions fail with focus on Reports while the new ones pass, and the new ones also fail if focus-on-open, which TabMenu.spec.md FR3 promises, is ever removed. No changes are needed.
[Automated review]
cixzhang
added
the
ci:run
…import kept with waitFor)
There was a problem hiding this comment.
Looks good to merge: both TabMenu arrow-key tests now wait for the menu's focus to land on the first option, so they pass whichever way that frame fires, and no shipped code changes. Letting the frame fire before the first key fails both tests on the base branch and passes them here, and removing focus-on-open makes the new waits fail, so the tests still catch a broken menu. Nothing further is needed from you.
[Automated review]
Review only this PR's change: vjeux/astryx@2b0f9a1...bcac705
Why
Two TabMenu keyboard tests in
TabList.test.tsxfail now and then on loaded CI runners, which fails the UI test shard and the requiredtestcheck of pull requests that never touch TabList (seen on #8045, whose diff is the interaction harness alone). The failure reads "Received element with focus: Reports".What led to it
We traced it while keeping our own runs green. Opening a TabMenu moves focus to its first item a frame later (
requestAnimationFrame(() => focusFirst()), the APG menu-button focus-on-open). The two tests press ArrowDown right after the click and expect it to reach the first item. That holds only while the frame has not fired: with nothing focused, ArrowDown focuses the first item; once focus is on it, ArrowDown moves on to the second. On a busy runner the frame fires first.Options and tradeoffs
Why this choice
Test-only, and it asserts what a user sees: the menu opens with its first item focused, then ArrowDown and ArrowUp move from there. The other TabMenu tests (Home/End, Enter, Tab, Escape) don't depend on when that frame fires. The file's newer pointer test (#8791) is unchanged.
How to check it
pnpm exec vitest run --project ui packages/core/src/TabList/TabList.test.tsx: every test passes.await new Promise(r => setTimeout(r, 50));after the click in "moves focus between items with ArrowDown and ArrowUp" and in "moves the roving tab stop with arrow navigation": on main both fail every time, with focus on Reports; here both pass.Evidence