bert.hausmans/crewli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(gui-v2): mount Drawer only on mobile (v-if) + shared Tailwind breakpoint

Browse Source 
CRITICAL: replace `lg:hidden` on PrimeVue Drawer with `v-if="isMobile"` so the
teleported portal/overlay is never created on desktop viewports regardless of
mobileOpen state. Replace useMediaQuery raw string in SidebarHeader with
useBreakpoints(breakpointsTailwind).smaller('lg') shared by both components.
Add desktop/mobile comments; adapt tests to useBreakpoints mock; add
Drawer-absent-on-desktop and aside w-16/w-64 width-class assertions (21 tests).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
bert.hausmans
2026-05-16 20:41:50 +02:00
parent f0f9cb7e36
commit 23e1262f9c
4 changed files with 141 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
apps/app/src/components-v2/layout/AppSidebar.vue
View File

@@ -13,6 +13,12 @@
* <aside> — a small amount of template duplication kept intentional for
* readability (no render-fn or dynamic component indirection needed at this scale).
*
* The Drawer is conditionally MOUNTED (v-if="isMobile") rather than hidden via
* a CSS class. PrimeVue Drawer teleports its overlay to <body> via an internal
* Portal; a `lg:hidden` class on the component node does NOT suppress the
* teleported overlay. By unmounting on desktop we guarantee no modal backdrop /
* focus-trapped dialog can appear on wide viewports regardless of mobileOpen state.
*
* nav groups arrive as a prop — this file must NOT import @/navigation directly
* (components-v2 import boundary; the layout page supplies nav data).
*
@@ -32,6 +38,7 @@
*/
import { computed } from 'vue'
import { breakpointsTailwind, useBreakpoints } from '@vueuse/core'
import Drawer from 'primevue/drawer'
import { useShellUiStore } from '@/stores/useShellUiStore'
import type { V2NavGroup } from '@/types/v2/nav'
@@ -49,6 +56,12 @@ defineProps<{
const shell = useShellUiStore()
// Shared breakpoint signal — must match SidebarHeader and the CSS `lg:` boundary (1024px).
// useBreakpoints(breakpointsTailwind).smaller('lg') is true below 1024px, equivalent to
// max-width: 1023px. Both components import from the same Tailwind constant so the
// desktop/mobile boundary cannot drift.
const isMobile = useBreakpoints(breakpointsTailwind).smaller('lg')
/**
* Writable computed so we can use v-model:visible on PrimeVue Drawer
* without mutating the store ref directly.
@@ -63,7 +76,9 @@ const mobileVisible = computed<boolean>({
<!--
DESKTOP: permanent <aside>, hidden below lg, flex column above lg.
Width transitions between w-64 (expanded) and w-16 (collapsed).
`hidden lg:flex` is effective here because <aside> is not teleported.
-->
<!-- desktop -->
<aside
class="hidden lg:flex flex-col overflow-hidden bg-[var(--p-surface-card)] border-e border-[var(--p-content-border-color)] z-40 transition-[width] duration-200 flex-shrink-0"
:class="shell.sidebarCollapsed ? 'w-16' : 'w-64'"
@@ -77,19 +92,27 @@ const mobileVisible = computed<boolean>({
</aside>
<!--
MOBILE: PrimeVue Drawer rendered below lg. The Drawer's own overlay handles
backdrop / close-on-outside-click. position="left" gives the sidebar-style
panel. pt (passthrough) removes Drawer's default padding so our children
control their own spacing exactly as on desktop.
MOBILE: PrimeVue Drawer, conditionally MOUNTED only when isMobile is true.
v-if (not v-show / CSS) is required because PrimeVue Drawer teleports its
overlay to <body> — a CSS class on the component node does NOT reach the
teleported portal. Unmounting on desktop ensures the overlay and focus-trap
are never created on wide viewports, regardless of shell.mobileOpen state.
The Drawer's own overlay handles backdrop / close-on-outside-click.
position="left" gives the sidebar-style panel. pt (passthrough) removes
Drawer's default padding so our children control their own spacing exactly
as on desktop.
The children are intentionally repeated (not factored into a slot or
render-fn) — the 3-component block is short and the duplication is
preferable to the indirection of a dynamic component or slot wiring.
-->
<!-- mobile -->
<Drawer
v-if="isMobile"
v-model:visible="mobileVisible"
position="left"
class="!w-64 lg:hidden"
class="!w-64"
:pt="{
content: { class: 'flex flex-col p-0 overflow-hidden h-full' },
header: { class: 'hidden' },

7
apps/app/src/components-v2/layout/SidebarHeader.vue
View File

@@ -37,14 +37,15 @@
* (per RFC §7.4 — last resort, with justification comment).
*/
import { useMediaQuery } from '@vueuse/core'
import { breakpointsTailwind, useBreakpoints } from '@vueuse/core'
import Icon from '@/components/Icon.vue'
import { useShellUiStore } from '@/stores/useShellUiStore'
const shell = useShellUiStore()
// lg breakpoint = 1024px; mobile is anything below that (max-width: 1023px)
const isMobile = useMediaQuery('(max-width: 1023px)')
// Use the canonical Tailwind lg breakpoint (1024px) — consistent with AppSidebar's
// v-if="isMobile" guard so both components agree on the desktop/mobile boundary.
const isMobile = useBreakpoints(breakpointsTailwind).smaller('lg')
function handleCollapseClick(): void {
if (isMobile.value) {

108
apps/app/src/components-v2/layout/__tests__/AppSidebar.spec.ts
View File

@@ -7,18 +7,44 @@
* 2. Passes `groups` prop to SidebarNav.
* 3. Mobile Drawer v-model:visible wires to shell.mobileOpen (get path).
* 4. Drawer close (v-model:visible = false) calls shell.setMobileOpen(false).
* 5. Drawer is NOT rendered when isMobile=false (desktop); IS rendered when isMobile=true.
* 6. Desktop <aside> applies correct width class based on sidebarCollapsed.
*
* Stubs: Drawer is stubbed with a simple slot passthrough so we can inspect
* whether its `visible` prop is correctly bound to the store.
*
* @vueuse/core is mocked so we can control isMobile per test context.
*/
import { createPinia, setActivePinia } from 'pinia'
import { mount } from '@vue/test-utils'
import { ref } from 'vue'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useShellUiStore } from '@/stores/useShellUiStore'
import AppSidebar from '@/components-v2/layout/AppSidebar.vue'
import type { V2NavGroup } from '@/types/v2/nav'
// ---------------------------------------------------------------------------
// Mock @vueuse/core so we can control `isMobile` per test.
// AppSidebar uses useBreakpoints(breakpointsTailwind).smaller('lg').
// We return an object whose .smaller() method returns a controllable ref.
// ---------------------------------------------------------------------------
const mockIsMobileRef = ref(false)
vi.mock('@vueuse/core', () => ({
breakpointsTailwind: {},
useBreakpoints: () => ({
smaller: (_bp: string) => mockIsMobileRef,
}),
}))
// ---------------------------------------------------------------------------
// Import component AFTER mock is set up
// ---------------------------------------------------------------------------
// eslint-disable-next-line import/first -- intentional: mock must be declared first
import AppSidebar from '@/components-v2/layout/AppSidebar.vue'
// ---------------------------------------------------------------------------
// A minimal nav group fixture
// ---------------------------------------------------------------------------
@@ -75,7 +101,12 @@ function mountSidebar(groups: V2NavGroup[] = testGroups) {
}
describe('AppSidebar', () => {
beforeEach(() => setActivePinia(createPinia()))
beforeEach(() => {
setActivePinia(createPinia())
// Default to desktop (isMobile=false) so most tests exercise the stable path
mockIsMobileRef.value = false
})
// -------------------------------------------------------------------------
// Child composition
@@ -108,10 +139,67 @@ describe('AppSidebar', () => {
})
// -------------------------------------------------------------------------
// Mobile Drawer wiring
// Drawer v-if: mount/unmount based on isMobile
// -------------------------------------------------------------------------
it('Drawer visible is true when shell.mobileOpen is true', async () => {
it('desktop (isMobile=false): Drawer is NOT rendered', () => {
mockIsMobileRef.value = false
const wrapper = mountSidebar()
// v-if="isMobile" means the Drawer stub must be absent on desktop
expect(wrapper.find('.drawer-stub').exists()).toBe(false)
})
it('mobile (isMobile=true): Drawer IS rendered', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
await wrapper.vm.$nextTick()
expect(wrapper.find('.drawer-stub').exists()).toBe(true)
})
// -------------------------------------------------------------------------
// Desktop <aside> width class based on sidebarCollapsed
// -------------------------------------------------------------------------
it('desktop: aside has w-64 class when sidebar is expanded', async () => {
mockIsMobileRef.value = false
const wrapper = mountSidebar()
const shell = useShellUiStore()
shell.sidebarCollapsed = false
await wrapper.vm.$nextTick()
expect(wrapper.find('aside').classes()).toContain('w-64')
expect(wrapper.find('aside').classes()).not.toContain('w-16')
})
it('desktop: aside has w-16 class when sidebar is collapsed', async () => {
mockIsMobileRef.value = false
const wrapper = mountSidebar()
const shell = useShellUiStore()
shell.sidebarCollapsed = true
await wrapper.vm.$nextTick()
expect(wrapper.find('aside').classes()).toContain('w-16')
expect(wrapper.find('aside').classes()).not.toContain('w-64')
})
// -------------------------------------------------------------------------
// Mobile Drawer wiring (only exercised when isMobile=true)
// -------------------------------------------------------------------------
it('mobile: Drawer visible is true when shell.mobileOpen is true', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
const shell = useShellUiStore()
@@ -125,7 +213,9 @@ describe('AppSidebar', () => {
expect(drawer.attributes('data-visible')).toBe('true')
})
it('Drawer visible is false when shell.mobileOpen is false', async () => {
it('mobile: Drawer visible is false when shell.mobileOpen is false', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
const shell = useShellUiStore()
@@ -138,7 +228,9 @@ describe('AppSidebar', () => {
expect(drawer.attributes('data-visible')).toBe('false')
})
it('emitting update:visible false from Drawer calls shell.setMobileOpen(false)', async () => {
it('mobile: emitting update:visible false from Drawer calls shell.setMobileOpen(false)', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
const shell = useShellUiStore()
@@ -152,7 +244,9 @@ describe('AppSidebar', () => {
expect(setMobileOpenSpy).toHaveBeenCalledWith(false)
})
it('emitting update:visible true from Drawer calls shell.setMobileOpen(true)', async () => {
it('mobile: emitting update:visible true from Drawer calls shell.setMobileOpen(true)', async () => {
mockIsMobileRef.value = true
const wrapper = mountSidebar()
const shell = useShellUiStore()

12
apps/app/src/components-v2/layout/__tests__/SidebarHeader.spec.ts
View File

@@ -2,7 +2,7 @@
* SidebarHeader.spec.ts — unit tests for the non-trivial logic in SidebarHeader.
*
* Strategy: mount with @vue/test-utils + createPinia. Child components (Icon)
* and @vueuse/core's useMediaQuery are stubbed so we test only:
* and @vueuse/core's useBreakpoints are stubbed so we test only:
* 1. Collapse-button desktop path → shell.toggleSidebar() called.
* 2. Collapse-button mobile path → shell.setMobileOpen(false) called.
* 3. Collapsed state hides wordmark + pill; keep collapse button.
@@ -18,14 +18,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useShellUiStore } from '@/stores/useShellUiStore'
// ---------------------------------------------------------------------------
// Mock @vueuse/core so we can control `isMobile` per test
// Mock @vueuse/core so we can control `isMobile` per test.
// The component uses useBreakpoints(breakpointsTailwind).smaller('lg').
// We return an object whose .smaller() method returns a controllable ref.
// ---------------------------------------------------------------------------
// We expose a reactive ref that individual tests can flip.
const mockIsMobileRef = ref(false)
vi.mock('@vueuse/core', () => ({
useMediaQuery: () => mockIsMobileRef,
breakpointsTailwind: {},
useBreakpoints: () => ({
smaller: (_bp: string) => mockIsMobileRef,
}),
}))
// ---------------------------------------------------------------------------