fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(ui-dialog): cancel the pending focus region activation on close - #2697

Merged
matyasf merged 1 commit into
masterfrom
fix_dialog_test
Sep 1, 2026
Merged

fix(ui-dialog): cancel the pending focus region activation on close#2697
matyasf merged 1 commit into
masterfrom
fix_dialog_test

Conversation

@matyasf

Copy link
Copy Markdown
Collaborator

Summary

  • Dialog.close() cancels any still-scheduled requestAnimationFrame region activation and clears
    _focusRegion after blurring. Without it, a Dialog closed before that frame ran (e.g. a Tray
    opened and closed within one frame, which happens when rAF callbacks land late under load)
    activated a region nothing ever blurred — its document keydown listener then ran scopeTab on
    an unrendered element, preventDefaulting every later Tab press.
  • Adds a Dialog regression test that flushes the activation frame manually after the close and
    asserts Tab still moves focus outside the Dialog.

Test Plan

  • This was the cause of the flaky Tray should handle focus properly in complex cases browser
    test. To reproduce the old failure, stub window.requestAnimationFrame to fire ~150ms late and
    run packages/ui-tray/src/Tray/__tests__/Tray.test.tsx — it fails on the first userEvent.tab()
    without this fix and passes with it.
  • Worth a manual keyboard pass on Modal/Popover/Tray/Menu: open, close, then Tab around to confirm
    focus return and tab order are unchanged.

🤖 Generated with Claude Code

@matyasfmatyasf self-assigned this Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-01 08:52 UTC

github-actionsBot pushed a commit that referenced this pull request Aug 25, 2026
@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Visual regression report

Cypress suite: ✅ Passing

Visual diff:⚠️Changes detected.

StatusCount
Unchanged95
Changed1
New0
Removed0

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 differ

Baselines come from the visual-baselines branch. They refresh on every merge to master. The Cypress suite line covers the a11y and console-error assertions — a ❌ there means the suite found real issues even if the visual diff is clean.

@balzssbalzss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

github-actionsBot pushed a commit that referenced this pull request Aug 27, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

@balzss the 2 visual regression test changes look interesting

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>
github-actionsBot pushed a commit that referenced this pull request Aug 28, 2026
@matyasf

Copy link
Copy Markdown
CollaboratorAuthor

VRT is flaky, now I run it again, and now it shows a change in Badge

@matyasf
matyasf requested a review from balzssAugust 28, 2026 11:54
@matyasf
matyasf merged commit 9b0467d into masterSep 1, 2026
11 of 13 checks passed
@matyasf
matyasf deleted the fix_dialog_test branch September 1, 2026 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@matyasf@balzss@joyenjoyer