fix(ui-dialog): cancel the pending focus region activation on close - #2697
fix(ui-dialog): cancel the pending focus region activation on close#2697matyasf wants to merge 1 commit into
Conversation
|
Visual regression reportCypress suite: ✅ Passing Visual diff:
Accessibility (axe): ✅ No violations. 📊 View full report — click a screenshot's ⚠ badge to see each violation boxed on the image, with the offending element named and contrast failures shown as color swatches. Diff images (1)badge-canvas.png — 1573 pixels differBaselines come from the |
balzss
left a comment
balzss
left a comment
There was a problem hiding this comment.
although the code looks good, the 2 visual regression test changes look interesting. the table one seems unrelated which is even weirder, but the menu one might has something to do with the changes. it looks like the dark theme now not highlights the first menu option? also the other themes are not consistent with highlighting/not-highlighting the first item. do you think it's related to this change?
I've re-ran the VRT and the changes are gone.. |
Dialog activates its FocusRegion in a requestAnimationFrame callback. When the Dialog closed before that frame ran (e.g. a Tray that is opened and closed within the same frame, which happens on a loaded machine where rAF callbacks land late), close() found no region to blur and left the frame scheduled. The callback then activated a region for an already closed Dialog, which nothing ever blurred: componentWillUnmount only closes while open. The leaked region kept a document keydown listener that scoped every later tab press to an element that is not rendered anymore, so scopeTab called preventDefault and Tab stopped working. close() now cancels any scheduled activation and clears _focusRegion after blurring. This is what made the Tray "should handle focus properly in complex cases" browser test flaky on CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
48622de to
0fbf026
Compare
|
VRT is flaky, now I run it again, and now it shows a change in |
3 participants
Summary
Dialog.close()cancels any still-scheduledrequestAnimationFrameregion activation and clears_focusRegionafter blurring. Without it, a Dialog closed before that frame ran (e.g. a Trayopened and closed within one frame, which happens when rAF callbacks land late under load)
activated a region nothing ever blurred — its document
keydownlistener then ranscopeTabonan unrendered element,
preventDefaulting every later Tab press.asserts Tab still moves focus outside the Dialog.
Test Plan
should handle focus properly in complex casesbrowsertest. To reproduce the old failure, stub
window.requestAnimationFrameto fire ~150ms late andrun
packages/ui-tray/src/Tray/__tests__/Tray.test.tsx— it fails on the firstuserEvent.tab()without this fix and passes with it.
focus return and tab order are unchanged.
🤖 Generated with Claude Code