Navya/fix/modal focus effect split - #587
Open
navyamidha95339-arch wants to merge 2 commits into
Open
Conversation
…(AP-029) - Focus-on-open now runs only on [open] change, so a parent re-render with a new inline onClose no longer re-steals focus - Keydown/focus-trap listener stays on [open, onClose] to use the latest onClose, without touching focus itself - Added regression tests: parent re-render doesn't move focus, focus still restores to the previously-focused element on close - Note: ran full test suite as regression check — 1 pre-existing unrelated failure in ProtectedRoute.test.jsx (hasExpiredToken), not touched by this change
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
1 participant
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.
Modal: split focus-on-open effect from keydown/focus-trap (AP-029)
Summary
Fixes a bug in the shared Modal component where a parent re-render while the modal is open (which creates a new inline onClose function) would re-run the whole effect and re-steal focus back to the first focusable element — even if the user had since focused something else inside the modal.
Checklist
✅ Focus-on-open in its own [open]-only effect
✅ Keydown + focus-trap in a separate effect (still keyed on [open, onClose] to use the latest onClose, but doesn't touch focus)
✅ Focus restored to the previously-focused element on close
✅ Regression-tested existing modals — ran the full test suite; all Modal-dependent tests pass. One pre-existing, unrelated failure in ProtectedRoute.test.jsx (hasExpiredToken is not a function) — not touched by this change.
Evidence
Added tests to Modal.test.jsx:
Parent re-render with a new inline onClose does not move focus away from whatever the user has focused
Focus is still restored to the previously-focused element on close
Note: While rebasing onto latest main, found main already had a very similar effect split from another recent commit — this PR keeps the more descriptive comments/eslint-disable from my original work and adds the regression tests, which weren't present before.