Repository navigation
fix(alert): only restore focus that this Alert stored - #3255
Merged
interactivellama merged 2 commits intoOct 26, 2026
Merged
Conversation
Alert's unmount effect called DOMElementFocus.returnFocusToStoredElement() unconditionally. The stored element is a module level singleton shared with Popover (utilities/dialog) and Toast. A non dismissible Alert never calls storeActiveElement(), but on unmount it still focused and cleared whatever another component had stored. For example, if an inline Alert unmounts while a Popover is open, focus jumps to the Popover's trigger and the Popover loses the element it meant to restore. Track whether this Alert stored focus and only restore in that case. Dismissible Alerts keep returning focus to the previously focused element after the close button took it.
A non dismissible Alert leaves focus stored by another component alone, and a dismissible Alert still returns focus to the previously focused element on unmount.
interactivellama
approved these changes
Oct 26, 2026
Contributor
|
Great find! @zigzagdev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #
Additional description
Symptom
When an Alert without
dismissibleunmounts while a Popover is open, focus jumps from inside the Popover back to the Popover's trigger.A user typing in an input inside the Popover suddenly stops typing into it, while the Popover stays open.
Root cause
Alert's unmount effect called
DOMElementFocus.returnFocusToStoredElement()unconditionally.The stored element in
DOMElementFocusis a single module level value shared with Popover (utilities/dialog) and Toast. A non dismissible Alert never callsstoreActiveElement(), so on unmount it focused and cleared the element another component had stored.This is not a regression from #3195.
Change
Alert now remembers in a ref whether it stored focus, which only happens when the close button takes focus on mount. Focus is restored on unmount only in that case.
Dismissible Alerts behave exactly as before.
before
2026-10-25.13.44.40.mov
after
2026-10-25.13.53.07.mov
Tests
alert.test.jsx.CONTRIBUTOR checklist (do not remove)
Please complete for every pull request.
Green gate — all of these must pass locally (CI runs the same):
npm run lintpasses with 0 errors (npm run lint:fixto auto-fix). Warnings are the tracked gradual-migration backlog; do not add new errors.npm run typecheckis clean (tsc --noEmit).npm test -- --runpasses — this runs both Vitest projects (jsdomunit+ real-browserbrowser) and the story HTML snapshots. If you changed rendered markup on purpose, update snapshots withnpm test -- --run -uand review the diff.npm run buildsucceeds (type-check + Vite library build +.d.ts+ entry shims).Behavior & docs:
components/button/__tests__). New props and bug fixes should get a regression test. Tests that genuinely need a real browser (focus traversal, layout/measurement, canvas) go in a*.browser.test.jsxfile (thebrowserproject); everything else is jsdom.npm run storybook→ http://localhost:6007/. Confirm the story actually renders and behaves (deprecated Storyshots/Karma are gone; there is no separate browser-tests server).onRenderContent/onRenderVisualPicker,iconprops,events), not assumptions — a common source of "component looks broken" reports has been stories authored against the wrong API. Cross-check againstcomponents/<name>/__examples__..d.ts(they're generated by the build); JS consumers rely on them via editor type-acquisition.REVIEWER checklist (do not remove)
CIworkflow: lint, typecheck, test, build). Chromatic visual review, if configured, has been checked.components/__tests__/__snapshots__/story-snapshots.test.jsx.snap).@salesforce/design-system-react/components/<name>and nested paths resolve for ESM, CJS, and types). Removing/renaming a public component or export is a breaking change →feat!:/BREAKING CHANGE:and a major version bump.