Skip to content

[6.x] Restore focus when the global sidebar collapses or expands - #19586

Open
gcamacho079 wants to merge 4 commits into
6.xfrom
lupe/acc-261-keyboard-focus-is-dropped-when-collapsing-the-global
Open

[6.x] Restore focus when the global sidebar collapses or expands#19586
gcamacho079 wants to merge 4 commits into
6.xfrom
lupe/acc-261-keyboard-focus-is-dropped-when-collapsing-the-global

Conversation

@gcamacho079

@gcamacho079 gcamacho079 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description

Toggling the docked sidebar swaps #sidebar-toggle between header and footer (v-if), destroying the button that was just clicked and dropping focus to <body> (WCAG 2.4.3 Focus Order).

Fix refocuses deliberately, direction-aware:

  • Expand: focus the relocated toggle.
  • Collapse: focus the nav list instead. Refocusing the toggle here would skip screen reader/keyboard users past the whole nav, since the toggle sits below it in the footer.

Scoped to docked mode only — floating (mobile) mode never destroys its toggle, so it already keeps focus correctly and needs no change.

This PR also adds agent instructions to recommend writing tests to prevent regressions.

Testing

CpSidebar.test.ts covers both directions plus floating mode (no regression).

Related issues

ACC-261

@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

ACC-261

@gcamacho079
gcamacho079 requested a lite review from Copilot September 8, 2026 22:25
@github-actions

github-actions Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown

📚 Storybook previews

@craftcms/ui — open Storybook

No changed components detected in this Storybook.

resources/js — open Storybook

No changed components detected in this Storybook.

@gcamacho079 gcamacho079 added the accessibility 👤 features related to accessibility label Sep 8, 2026
Copilot AI reviewed Sep 8, 2026
View reviewed changes

Copilot AI 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.

🟡 Changes recommended

The new floating-mode unit test currently exercises an unrealistic interaction path (clicking the sidebar’s internal toggle while hidden/inert) and should be rewritten or removed to avoid false confidence.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Restores predictable keyboard focus when toggling the docked global sidebar, preventing focus from being lost when the toggle button is destroyed/recreated due to v-if placement changes (WCAG 2.4.3 Focus Order).

Changes:

  • Add a docked-only toggleAndRestoreFocus() handler to intentionally restore focus after expanding/collapsing.
  • Make the sidebar body programmatically focusable and target it on collapse to avoid skipping past navigation.
  • Add/extend repository agent guidance to encourage adding regression tests after behavior changes, plus a new CpSidebar unit test file.
File summaries
File Description
resources/js/common/components/CpSidebar.vue Adds direction-aware focus restoration after docked toggle swaps the button between header/footer.
resources/js/common/components/CpSidebar.test.ts Adds unit tests intended to cover focus behavior for docked expand/collapse and floating mode.
AGENTS.md Adds guidance to propose regression tests after verifying a fix/behavior change.
.ai/guidelines/craft.md Mirrors the regression-test guidance in the Craft guidelines.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread resources/js/common/components/CpSidebar.test.ts Outdated
@gcamacho079
gcamacho079 marked this pull request as ready for review September 9, 2026 15:15
@github-actions
Merge remote-tracking branch 'origin/6.x' into lupe/acc-261-keyboard-…
ac42c9f 
…focus-is-dropped-when-collapsing-the-global

# Conflicts:
#	resources/js/common/components/CpSidebar.vue
#	resources/js/common/composables/useGlobalSidebar.test.ts
@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

This PR had conflicts with 6.x. Claude attempted an automatic resolution and pushed a merge commit. Please review the merge commit carefully before approving/merging.

Claude's response

I merged 6.x into the branch as ac42c9f and pushed it. I couldn't install the JS dependencies here (pnpm isn't available), so none of the updated tests have been run. CI will be their first run.

Conflicts found and how I resolved them:

  • resources/js/common/components/CpSidebar.vue (needs review): 6.x redesigned the sidebar. The header and footer toggle buttons that this branch's toggleAndRestoreFocus fix (fe322fb) worked on are gone. 6.x now has one collapse item in the footer, and its own toggleCollapsed puts focus back on that item after it re-renders. That already fixes focus being dropped when the sidebar collapses, so I took the 6.x version. The branch's behaviour is gone: focus no longer moves to the nav list on collapse or to the relocated toggle on expand. I left a CONFLICT-REVIEW comment asking for confirmation that the 6.x behaviour is acceptable for ACC-261.
  • resources/js/common/composables/useGlobalSidebar.test.ts (added on both sides): 6.x's version already has the same two tests this branch added in 95c6596, so I took 6.x's file.
  • resources/js/common/components/CpSidebar.test.ts (merged without conflicts, but out of date): Its tests clicked the #sidebar-toggle buttons and expected focus on .cp-sidebar__body, and that markup no longer exists. I rewrote the two docked tests to click the footer collapse item and check that focus stays on it. I also added a breakpoint mock so the floating-sidebar focus test still renders its header toggle. The file is marked with a CONFLICT-REVIEW comment and needs a close look, since those tests were never run.

@brianjhanson @claude
Remove merge-review notes from the sidebar focus fix
631d14b 
The collapse item tests pass against 6.x's toggleCollapsed, so the
notes asking for confirmation are no longer needed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@brianjhanson brianjhanson brianjhanson approved these changes

Assignees

No one assigned

Labels

accessibility 👤 features related to accessibility

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gcamacho079 @brianjhanson