diff --git a/docs/demos/Window.demo.menuBar.tsx b/docs/demos/Window.demo.menuBar.tsx index b5e8f8a..3c3df8c 100644 --- a/docs/demos/Window.demo.menuBar.tsx +++ b/docs/demos/Window.demo.menuBar.tsx @@ -1,53 +1,24 @@ import { Window } from '@gfazioli/mantine-window'; -import { Box, Button, Group, Menu, Paper, Stack, Text } from '@mantine/core'; +import { Box, Menu, Menubar, Paper, Stack, Text } from '@mantine/core'; import { MantineDemo } from '@mantinex/demo'; import { useState } from 'react'; const code = `import { useState } from 'react'; import { Window } from '@gfazioli/mantine-window'; -import { Box, Button, Group, Menu, Paper, Stack, Text } from '@mantine/core'; +import { Box, Menu, Menubar, Paper, Stack, Text } from '@mantine/core'; const densities = { compact: 'xs', comfortable: 'sm', spacious: 'lg' } as const; function Demo() { - const [query, setQuery] = useState(''); const [layers, setLayers] = useState(['Layer 1', 'Layer 2']); const [nextLayer, setNextLayer] = useState(3); const [lastAction, setLastAction] = useState(null); const [showGrid, setShowGrid] = useState(true); const [density, setDensity] = useState('comfortable'); - // Every File command really operates on the canvas - const commands = [ - { - label: 'New layer', - action: () => { - setLayers((current) => [...current, \`Layer \${nextLayer}\`]); - setNextLayer((n) => n + 1); - }, - }, - { - label: 'Duplicate layer', - action: () => { - setLayers((current) => - current.length > 0 ? [...current, \`\${current[current.length - 1]} copy\`] : current - ); - }, - }, - { - label: 'Remove layer', - action: () => setLayers((current) => current.slice(0, -1)), - }, - { label: 'Export as PNG', action: () => {} }, - ]; - - const filtered = commands.filter((command) => - command.label.toLowerCase().includes(query.trim().toLowerCase()) - ); - - const runCommand = (command: (typeof commands)[number]) => { - command.action(); - setLastAction(command.label); + const run = (label: string, fn?: () => void) => { + fn?.(); + setLastAction(label); }; return ( @@ -57,48 +28,90 @@ function Demo() { opened defaultX={30} defaultY={30} - defaultWidth={440} - defaultHeight={340} + defaultWidth={460} + defaultHeight={360} persistState={false} withinPortal={false} withScrollArea={false} > - {/* The menu bar */} - - - - - - - setQuery(event.currentTarget.value)} - placeholder="Search commands" - /> - {filtered.length > 0 ? ( - filtered.map((command) => ( - runCommand(command)}> - {command.label} - - )) - ) : ( - - Nothing found - - )} - - + {/* Mantine 9.4 Menubar — a real desktop-style menu bar with keyboard navigation */} + + + File + + + ⌘N + + } + onClick={() => + run('New layer', () => { + setLayers((current) => [...current, \`Layer \${nextLayer}\`]); + setNextLayer((n) => n + 1); + }) + } + > + New layer + + + ⌘D + + } + onClick={() => + run('Duplicate layer', () => + setLayers((current) => + current.length > 0 + ? [...current, \`\${current[current.length - 1]} copy\`] + : current + ) + ) + } + > + Duplicate layer + + run('Remove layer', () => setLayers((c) => c.slice(0, -1)))}> + Remove layer + + + + ⌘E + + } + onClick={() => run('Export as PNG')} + > + Export as PNG + + + - - - - - + + Edit + + run('Clear all layers', () => setLayers([]))}> + Clear all layers + + + run('Reset view', () => { + setShowGrid(true); + setDensity('comfortable'); + }) + } + > + Reset view + + + + + + View + Canvas Show grid @@ -113,11 +126,11 @@ function Demo() { Comfortable Spacious - - - + + + - {/* The canvas: layers, grid and density all react to the menus */} + {/* The canvas reacts to every menu action */} - {/* Status bar: File menu commands land here */} + {/* Status bar: every menu command lands here */} - {lastAction ? \`Last action: \${lastAction}\` : 'Run a command from the File menu'} + {lastAction ? \`Last action: \${lastAction}\` : 'Pick a command from the menu bar'} ); -} -`; +}`; const densities = { compact: 'xs', comfortable: 'sm', spacious: 'lg' } as const; function Demo() { - const [query, setQuery] = useState(''); const [layers, setLayers] = useState(['Layer 1', 'Layer 2']); const [nextLayer, setNextLayer] = useState(3); const [lastAction, setLastAction] = useState(null); const [showGrid, setShowGrid] = useState(true); const [density, setDensity] = useState('comfortable'); - const commands = [ - { - label: 'New layer', - action: () => { - setLayers((current) => [...current, `Layer ${nextLayer}`]); - setNextLayer((n) => n + 1); - }, - }, - { - label: 'Duplicate layer', - action: () => { - setLayers((current) => - current.length > 0 ? [...current, `${current[current.length - 1]} copy`] : current - ); - }, - }, - { - label: 'Remove layer', - action: () => setLayers((current) => current.slice(0, -1)), - }, - { label: 'Export as PNG', action: () => {} }, - ]; - - const filtered = commands.filter((command) => - command.label.toLowerCase().includes(query.trim().toLowerCase()) - ); - - const runCommand = (command: (typeof commands)[number]) => { - command.action(); - setLastAction(command.label); + const run = (label: string, fn?: () => void) => { + fn?.(); + setLastAction(label); }; return ( @@ -206,47 +190,92 @@ function Demo() { opened defaultX={30} defaultY={30} - defaultWidth={440} - defaultHeight={340} + defaultWidth={460} + defaultHeight={360} persistState={false} withinPortal={false} withScrollArea={false} > - - - - - - - setQuery(event.currentTarget.value)} - placeholder="Search commands" - /> - {filtered.length > 0 ? ( - filtered.map((command) => ( - runCommand(command)}> - {command.label} - - )) - ) : ( - - Nothing found - - )} - - + {/* Mantine 9.4 Menubar — a real desktop-style menu bar with keyboard navigation */} + + + File + + + ⌘N + + } + onClick={() => + run('New layer', () => { + setLayers((current) => [...current, `Layer ${nextLayer}`]); + setNextLayer((n) => n + 1); + }) + } + > + New layer + + + ⌘D + + } + onClick={() => + run('Duplicate layer', () => + setLayers((current) => + current.length > 0 + ? [...current, `${current[current.length - 1]} copy`] + : current + ) + ) + } + > + Duplicate layer + + run('Remove layer', () => setLayers((c) => c.slice(0, -1)))} + > + Remove layer + + + + ⌘E + + } + onClick={() => run('Export as PNG')} + > + Export as PNG + + + + + + Edit + + run('Clear all layers', () => setLayers([]))}> + Clear all layers + + + run('Reset view', () => { + setShowGrid(true); + setDensity('comfortable'); + }) + } + > + Reset view + + + - - - - - + + View + Canvas Show grid @@ -261,10 +290,11 @@ function Demo() { Comfortable Spacious - - - + + + + {/* The canvas reacts to every menu action */} + {/* Status bar: every menu command lands here */} - {lastAction ? `Last action: ${lastAction}` : 'Run a command from the File menu'} + {lastAction ? `Last action: ${lastAction}` : 'Pick a command from the menu bar'} diff --git a/docs/docs.mdx b/docs/docs.mdx index a3afe3c..0f7aa08 100644 --- a/docs/docs.mdx +++ b/docs/docs.mdx @@ -177,10 +177,14 @@ dynamically from state. ## Window Menu Bar -The Menu.Search, Menu.CheckboxItem and Menu.RadioGroup components added in Mantine 9.3 -make a compact in-window menu bar straightforward. In this demo every menu entry drives -the workspace for real: the searchable File menu reports the executed command in the -status bar, while the View menu toggles the canvas grid and changes the layout density. +Mantine 9.4 ships a real Menubar component — a horizontal row of menus with full keyboard +navigation (arrow keys move between top-level menus, and once one is open, moving to a sibling +opens it immediately) and the WAI-ARIA menubar semantics. Dropped into a Window it completes the +desktop-application metaphor. In this demo every entry drives the workspace for real: the File +and Edit menus report the executed command in the status bar, while the View menu toggles the +canvas grid and changes the layout density. Because the Window drag handler ignores interactive +targets — and the menu triggers carry the `menuitem` role — the menu bar stays fully usable +inside the draggable surface. diff --git a/package/src/Window.test.tsx b/package/src/Window.test.tsx index 671f15c..158ba80 100644 --- a/package/src/Window.test.tsx +++ b/package/src/Window.test.tsx @@ -861,6 +861,40 @@ describe('Window.Group', () => { expect(container.querySelectorAll('[data-mantine-window]').length).toBe(0); }); + it('groupRef.closeAll fires onClose and respects controlled opened (issue #36)', () => { + const onClose1 = jest.fn(); + const onClose2 = jest.fn(); + + function TestCloseAllControlled() { + const groupRef = createRef(); + return ( + <> + + + + + + + ); + } + + const { container } = renderWithMantine(); + expect(container.querySelectorAll('[data-mantine-window]').length).toBe(2); + + act(() => { + fireEvent.click(screen.getByText('Close All')); + }); + + // closeAll must notify every window's onClose (regression: previously it bypassed it)... + expect(onClose1).toHaveBeenCalledTimes(1); + expect(onClose2).toHaveBeenCalledTimes(1); + // ...and must NOT force visibility off on controlled windows: `opened` is still true, + // so the windows stay mounted until the consumer flips `opened` in response to onClose. + expect(container.querySelectorAll('[data-mantine-window]').length).toBe(2); + }); + // ─── groupRef.applyLayout ─────────────────────────────────────────── it('groupRef.applyLayout does not crash', () => { diff --git a/package/src/WindowGroup.context.ts b/package/src/WindowGroup.context.ts index 7055d99..be8ed65 100644 --- a/package/src/WindowGroup.context.ts +++ b/package/src/WindowGroup.context.ts @@ -40,6 +40,13 @@ export interface WindowCallbacks { setSize: (size: WindowSize) => void; setIsCollapsed: (collapsed: boolean) => void; setIsVisible: (visible: boolean) => void; + /** + * Mirrors the window's own close handler (the title-bar "✕" button): invokes `onClose` + * and, only for uncontrolled windows, hides the window. `Group.closeAll` calls this + * instead of `setIsVisible(false)` so it fires `onClose` and respects the controlled + * `opened` prop (issue #36). + */ + requestClose: () => void; } export interface WindowGroupContextValue { diff --git a/package/src/WindowGroup.tsx b/package/src/WindowGroup.tsx index e639028..cfaa5da 100644 --- a/package/src/WindowGroup.tsx +++ b/package/src/WindowGroup.tsx @@ -334,7 +334,9 @@ export const WindowGroup = factory((_props) => { // ─── Global actions — call Window callbacks directly ────────────── const closeAll = useCallback(() => { - callbacksRef.current.forEach((cb) => cb.setIsVisible(false)); + // Route through each window's own close handler (not setIsVisible directly) so that + // `onClose` fires and controlled windows (with `opened`) stay in sync — see issue #36. + callbacksRef.current.forEach((cb) => cb.requestClose()); }, []); const collapseAll = useCallback(() => { diff --git a/package/src/hooks/use-mantine-window.ts b/package/src/hooks/use-mantine-window.ts index e2e28fe..719710f 100644 --- a/package/src/hooks/use-mantine-window.ts +++ b/package/src/hooks/use-mantine-window.ts @@ -121,6 +121,7 @@ export function useMantineWindow(props: WindowBaseProps) { setSize: state.setSize, setIsCollapsed: (v) => state.setIsCollapsed(v), setIsVisible: (v) => state.setIsVisible(v), + requestClose: () => state.handleClose(), } ); return () => groupCtx.unregisterWindow(windowId); diff --git a/package/src/hooks/use-window-state.ts b/package/src/hooks/use-window-state.ts index 9d0773b..5033f3d 100644 --- a/package/src/hooks/use-window-state.ts +++ b/package/src/hooks/use-window-state.ts @@ -220,6 +220,9 @@ export function useWindowState(options: UseWindowStateOptions) { const onSizeChangeRef = useRef(onSizeChange); onSizeChangeRef.current = onSizeChange; + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; + const positionRef = useRef(position); positionRef.current = position; @@ -376,11 +379,15 @@ export function useWindowState(options: UseWindowStateOptions) { }, []); const handleClose = useCallback(() => { - if (onClose) { - return onClose(); + // Read onClose from a ref so this handler keeps a stable identity (it is registered + // once with a WindowGroup) while always invoking the latest onClose. Controlled windows + // (onClose provided) are only notified — the consumer drives `opened`; uncontrolled + // windows hide themselves. WindowGroup.closeAll routes through here too (issue #36). + if (onCloseRef.current) { + return onCloseRef.current(); } setIsVisible(false); - }, [onClose]); + }, []); // ─── Sync with props ────────────────────────────────────────────────