chore(Drawer): rely on FocusTrap returnFocus for focus restoration#1591
Conversation
|
Size stats
|
There was a problem hiding this comment.
Pull request overview
Refactors Drawer focus handling to rely on FocusTrap’s returnFocus (now defaulting to true) as the single focus-restoration mechanism, aligning Drawer with other modal components and avoiding duplicate focus() calls on close.
Changes:
- Removed
Drawer’s bespokeuseRestoreFocusunmount logic to avoid double focus restoration. - Removed the resolved TODO in
FocusTraprelated to unifying focus restoration. - Added focus-restoration tests for
SheetandDrawer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/focus-trap.tsx |
Removes resolved TODO now that consumers are converging on returnFocus. |
src/drawer.tsx |
Deletes useRestoreFocus so Drawer relies on FocusTrap for restoring focus. |
src/__tests__/sheet-test.tsx |
Adds a regression test asserting focus returns to the trigger after closing a Sheet. |
src/__tests__/drawer-test.tsx |
Adds a regression test asserting focus returns to the trigger after closing a Drawer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Deploy preview for mistica-web ready!
Deployed with vercel-action |
|
Accessibility report ℹ️ You can run this locally by executing |
25bec9f to
e70fff3
Compare
FocusTrap defaults to returnFocus = true, which restores focus to the previously focused element when the trap unmounts. Drawer also restored focus through its own useRestoreFocus hook, producing duplicate focus() calls on close. Remove Drawer's useRestoreFocus and rely solely on FocusTrap, matching Dialog and Sheet. Add focus-restoration tests for Drawer and Sheet, which previously had none, and drop the now-resolved TODO in FocusTrap. Closes #1589
e70fff3 to
d69d384
Compare
Guide agents to read only the docs relevant to a task rather than all docs upfront, and provide a tree-style map of the `doc` directory with a one-line description of each entry. (B
…sioning Add doc/commits-and-prs.md covering conventional commits, the supported type subset enforced by validate-pr-title, and the semantic-release flow. Reference it from CONTRIBUTING.md and list both in the AGENTS.md doc map.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
a11y report that |
|
should the PR title be |
…n up AGENTS doc map Deletes doc/commits-and-prs.md and moves the PR title, description, and reviewer rules inline into CONTRIBUTING. Simplifies AGENTS.md by removing the documentation map section that pointed to the now-deleted file.
|
@AlexandraGallipoliRodrigues not really: this is not fixing up anything really, we're simply removing a duplicated behavior we have on a component :). but it's not causing any misbehavior |
| --- | ||
|
|
||
| ## GitHub conventions | ||
| ## Conventions |
There was a problem hiding this comment.
because "github" can be misleading for the AI
There was a problem hiding this comment.
I'd leave the file as it was, we aren't really adding any info with these changes.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| - fix | ||
| - feat | ||
| - chore | ||
| - revert | ||
| ### PR Description | ||
| Concise summary of the problem and fix, ending with `Ref: <ISSUE-ID>`; | ||
|
|
| await page.goto(url, {waitUntil: 'networkidle0'}); | ||
| // Wait until the story has actually mounted, otherwise axe may scan the Storybook | ||
| // loading shell instead of the rendered story, producing flaky page-level results. | ||
| await page |
Context
Follow-up from PR #1578, which changed
FocusTrap's default toreturnFocus = true. With that default,Drawerrestored focus twice on close: once throughFocusTrap(react-focus-lock'sreturnFocus) and once through its ownuseRestoreFocushook. This produced redundantfocus()calls.Changes
Drawer'suseRestoreFocushook and rely onFocusTrap'sreturnFocusas the single source of truth, matchingDialogandSheet.DrawerandSheet, which previously had none.Dialogwas already covered.FocusTrap.Audit notes
All four
FocusTrapconsumers were reviewed.Dialog,Sheet, andNavigationBaralready relied onFocusTrap; onlyDrawercarried bespoke logic.NavigationBaris intentionally excluded: itsFocusTrapwraps the bar including the burger trigger, so focus is retained on the trigger automatically andreturnFocusperforms no restoration there, leaving nothing meaningful to test.Closes #1589