fix: suppress PrimeVue default drawer close button to fix focus a11y

Review follow-up (crewli-reviewer). With header:'!hidden' the default close
button was still mounted (display:none); PrimeVue Drawer.focus() falls back to
focusing it (no [autofocus] in our slots), a no-op that strands keyboard focus
behind the overlay. Set :show-close-icon="false" so the button is never
created — focus() no-ops cleanly and v-focustrap focuses the visible brand-row
X. Also: flip the previously tautological showCloseIcon test to assert the
decision (toBe(false)); align the mobile close aria-label to English 'Close
menu' (matching 'Collapse sidebar') for locale consistency.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-03 04:12:27 +02:00
parent ecbcdea814
commit eca624ee9d
5 changed files with 37 additions and 22 deletions

View File

@@ -115,15 +115,25 @@ const mobileVisible = computed<boolean>({
render-fn) the 3-component block is short and the duplication is
preferable to the indirection of a dynamic component or slot wiring.
Close control (MOBILE-SHELL-PARITY defect 2): PrimeVue's default header
close-X is force-hidden via `header: '!hidden'`. Plain `hidden` lost to
PrimeVue's base `.p-drawer-header` display in stylesheet order, so the
default X leaked through and overlapped the brand row (the reported
defect). The important variant (`!hidden`, matching this file's existing
`!w-64`) wins. The mobile close affordance instead lives in
SidebarHeader's brand row, which renders an X ("Sluit menu") on mobile
a single, non-overlapping control in the same top-right slot the desktop
sidebar uses for its collapse chevron. No duplicate, no overlap.
Close control (MOBILE-SHELL-PARITY defect 2): the mobile close affordance
lives in SidebarHeader's brand row, which renders an X ("Close menu") on
mobile — a single, non-overlapping control in the same top-right slot the
desktop sidebar uses for its collapse chevron.
PrimeVue's OWN default header close-X is suppressed two ways, by design:
- `:show-close-icon="false"` stops PrimeVue mounting the default close
button at all. This is required for a11y, not just visuals: PrimeVue's
Drawer.focus() (no [autofocus] in our slots) falls back to focusing
`this.closeButton`; if that button is merely display:none (the earlier
`header: 'hidden'` approach), focus() is a no-op and keyboard focus is
stranded behind the overlay. With showCloseIcon=false the button is
never created, focus() no-ops cleanly, and v-focustrap focuses the
first focusable element — the visible brand-row X.
- `header: '!hidden'` then collapses the now-empty header band so it adds
no space above the content. The important variant beats PrimeVue's base
`.p-drawer-header` display (plain `hidden` lost to it in stylesheet
order the original leak that let the default X overlap the brand row).
No duplicate control, no overlap, no stranded focus.
Bottom anchoring (MOBILE-SHELL-PARITY defect 3 WorkspaceSwitcher):
the content pt is a full-height flex COLUMN (`flex flex-col h-full
@@ -148,6 +158,7 @@ const mobileVisible = computed<boolean>({
v-model:visible="mobileVisible"
position="left"
class="!w-64"
:show-close-icon="false"
:pt="{
content: { class: 'flex flex-col p-0 overflow-hidden h-full min-h-0' },
header: { class: '!hidden' },

View File

@@ -133,7 +133,7 @@ function handleCollapseClick(): void {
Desktop (>=lg): collapse chevron (◀) → toggleSidebar().
Mobile (<lg): the drawer has no icon-rail collapse mode, so this same
control becomes an explicit CLOSE (✕ "Sluit menu") → setMobileOpen(false)
control becomes an explicit CLOSE (✕ "Close menu") → setMobileOpen(false)
(handleCollapseClick already branches on isMobile). This is the single
mobile close affordance (MOBILE-SHELL-PARITY defect 2); PrimeVue's
default drawer X is force-hidden in AppSidebar so the two never overlap.
@@ -141,7 +141,7 @@ function handleCollapseClick(): void {
<button
v-if="!effectiveCollapsed"
class="ms-auto w-7 h-7 border-0 bg-transparent text-[var(--p-text-muted-color)] rounded-[var(--p-border-radius)] inline-flex items-center justify-center transition-[background,color] duration-150 hover:bg-[var(--p-content-hover-background)] hover:text-[var(--p-text-color)] cursor-pointer"
:aria-label="isMobile ? 'Sluit menu' : 'Collapse sidebar'"
:aria-label="isMobile ? 'Close menu' : 'Collapse sidebar'"
type="button"
@click="handleCollapseClick"
>

View File

@@ -177,16 +177,17 @@ describe('AppSidebar', () => {
expect(pt.content.class).toContain('min-h-0')
})
it('mobile: drawer does NOT suppress the close affordance via showCloseIcon=false', async () => {
it('mobile: drawer suppresses the PrimeVue default close button (showCloseIcon=false) to avoid a stranded focus target', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
await wrapper.vm.$nextTick()
// The mobile close control lives in SidebarHeader's brand row; the Drawer
// is left at its default showCloseIcon (never forced to false).
expect(wrapper.findComponent(DrawerStub).props('showCloseIcon')).not.toBe(false)
// showCloseIcon=false stops PrimeVue mounting the default close button, so
// Drawer.focus() never falls back to a display:none button and leaves focus
// stranded. The explicit close control is SidebarHeader's brand-row X.
expect(wrapper.findComponent(DrawerStub).props('showCloseIcon')).toBe(false)
})
it('mobile: WorkspaceSwitcher renders inside the drawer', async () => {

View File

@@ -200,13 +200,13 @@ describe('SidebarHeader', () => {
// header (defect 1)
// -------------------------------------------------------------------------
it('mobile: brand-row control is an explicit close (aria-label "Sluit menu", X icon)', () => {
it('mobile: brand-row control is an explicit close (aria-label "Close menu", X icon)', () => {
mockIsMobileRef.value = true
const wrapper = mountHeader()
const btn = wrapper.find('button[aria-label]')
expect(btn.attributes('aria-label')).toBe('Sluit menu')
expect(btn.attributes('aria-label')).toBe('Close menu')
expect(btn.find('.icon-stub').attributes('data-icon')).toBe('tabler-x')
})
@@ -237,7 +237,7 @@ describe('SidebarHeader', () => {
const buttons = wrapper.findAll('button[aria-label]')
expect(buttons).toHaveLength(1)
expect(buttons[0].attributes('aria-label')).toBe('Sluit menu')
expect(buttons[0].attributes('aria-label')).toBe('Close menu')
})
// P6-styling-fix — the brand row is ALWAYS left-aligned with constant