Repository navigation
[toast] Fix timers not resuming after window regains focus - #4438
Conversation
The handleWindowFocus guard `getTarget(event) === win` caused an early return when the window itself was the focus target, which is exactly the alt-tab-back scenario. The `event.relatedTarget` check alone is sufficient to filter intra-page focus moves.
commit: |
Bundle size report
Check out the code infra dashboard for more information about this PR. |
flaviendelangle
left a comment
flaviendelangle
left a comment
There was a problem hiding this comment.
PR #4438 Review: [toast] Fix timers not resuming after window regains focus
Author: kyrregjerstad | Diff: +1 / -1 in 1 file | Closes: #4439
Overview
A one-line fix that removes an overly aggressive guard in handleWindowFocus. The original condition:
if (event.relatedTarget || getTarget(event) === win) {
return;
}was short-circuiting the exact scenario it needed to handle — when the user alt-tabs back into the browser, event.target is the window object, so the guard caused an early return, preventing resumeTimers() and setIsWindowFocused(true) from ever being called.
Analysis
Correctness — looks right. There's a clear symmetry issue in the current code:
handleWindowBlur(line 83–90) acts whengetTarget(event) === win(pauses timers)handleWindowFocus(line 92) was skipping whengetTarget(event) === win(blocking resume)
The fix restores logical symmetry. The remaining event.relatedTarget check is sufficient: when focus moves within the page, relatedTarget is non-null (the element that lost focus), so intra-page focus moves are still filtered out. When focus arrives from outside the page (alt-tab back), relatedTarget is null, allowing the handler to proceed.
Scope of change — narrow. The only behavioral difference is for events where getTarget(event) === win && !event.relatedTarget, which is specifically the window-regains-focus scenario. All other paths are unchanged.
Concerns
-
No regression test. The PR author acknowledges this is a manual-only verification. This is understandable — synthesizing a true window-level focus/blur event in JSDOM is not realistic, and even Chromium tests would need tricky alt-tab simulation. Still, it would be nice to have a unit test that directly calls
handleWindowFocuswith a synthetic event whererelatedTarget = nullandtarget = window, if possible via the store/timer APIs. -
Original intent is unclear. The
getTarget(event) === winguard was presumably added deliberately. It would be helpful if a maintainer who wrote the original logic confirms this was indeed a mistake and not protecting against a different edge case. The git blame would be informative here.
Verdict
| Dimension | Assessment |
|---|---|
| Correctness | The fix is logically sound and restores symmetry with the blur handler |
| Style / Conventions | Clean, minimal change — follows project patterns |
| Performance | No impact |
| Test coverage | No new automated test (hard to test this scenario) |
| Risk | Low — change is narrow and only affects the alt-tab-back path |
Confidence: 4/5
The fix is correct, minimal, and addresses a real user-facing bug. The -1 is for the lack of an automated regression test and the fact that the original guard's intent should ideally be confirmed by whoever wrote it. But the logical analysis is solid and the risk is very low — this is safe to merge after a maintainer sanity-check.
TLDR: Please add a test 😆
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
When the window regains focus, getTarget(event) returns the Window object which is not a Node, causing contains() to throw. Short-circuit with target === win since the window is never inside the viewport.
|
On https://base-ui.com/react/components/toast, when I switch to a new tab (pointer or cmd+tab) then come back, it resumes correctly for me currently |
oliviertassinari
added
the
component: toast
|
@atomiks ah sorry I just realized, this is an Arc bug, not a base-ui toast bug! Seems like window blur events are not sent reliably |
|
@kyrregjerstad so this PR doesn't fix it in Arc still? |
Codex Review (GPT-5.4)This is a focused bug-fix PR for 1. Bugs / Issues (None)2. Root Cause & Patch Assessment
3. Test Coverage AssessmentThe new regression is a logic-level test rather than a full browser-tab automation, but I think that is a reasonable stopping point here. A true alt-tab or tab-backgrounding test is awkward to automate reliably, and the current coverage still protects the important handler path that was broken. I ran Merge Confidence ScoreOverall merge confidence is 4/5. I’m highly confident in the runtime fix and comfortable with the current regression coverage for a small browser-specific bug fix like this. This looks good to merge. |
|
Looks like Safari has the issue as well, but Chrome and Firefox don't seem to for me. This PR fixes Safari. |
Closes #4439
Summary
handleWindowFocushandler inToastViewporthad a guard (getTarget(event) === win) that caused an early return when the window itself was the focus target — which is exactly the case when alt-tabbing back to the browser.resumeTimers()andsetIsWindowFocused(true)from being called, causing toasts to remain visible indefinitely after switching away and back.event.relatedTargetcheck alone is sufficient to filter intra-page focus moves without blocking window-level refocus.Steps to reproduce
Test plan
pnpm test:jsdom toast --no-watch— 206 passed)