diff --git a/craft/src/editor/EditorShell.tsx b/craft/src/editor/EditorShell.tsx index 848c75d..59b2866 100644 --- a/craft/src/editor/EditorShell.tsx +++ b/craft/src/editor/EditorShell.tsx @@ -9,6 +9,7 @@ import { ContextMenu } from '../panels/context-menu/ContextMenu'; import { useContextMenu } from '../hooks/useContextMenu'; import { useKeyboardShortcuts } from '../hooks/useKeyboardShortcuts'; import { useIsMobile } from '../hooks/useIsMobile'; +import { MobileChromeProvider } from '../state/MobileChromeContext'; import { DeviceMode } from '../types'; const SHOW_GUIDES_STORAGE_KEY = 'craft-show-guides'; @@ -75,28 +76,35 @@ export const EditorShell: React.FC = () => { }, [query, showMenu]); return ( -
- setShowGuides(!showGuides)} - /> -
- {!isMobile && } -
- + // Mobile-A2: shared sheet/modal-open state (see MobileChromeContext) -- + // provided around the whole shell so TopBar's Templates/Head Code modal + // state and MobilePanelBar's sheet state live in one place. Desktop + // doesn't render MobilePanelBar and TopBar's desktop branch behaves + // identically to before (same booleans, just sourced from context). + +
+ setShowGuides(!showGuides)} + /> +
+ {!isMobile && } +
+ +
+ {!isMobile && }
- {!isMobile && } + {isMobile && } +
- {isMobile && } - -
+ ); }; diff --git a/craft/src/hooks/useIsMobile.test.tsx b/craft/src/hooks/useIsMobile.test.tsx new file mode 100644 index 0000000..ad27519 --- /dev/null +++ b/craft/src/hooks/useIsMobile.test.tsx @@ -0,0 +1,153 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { useIsMobile } from './useIsMobile'; + +/** + * Mobile-A2 (review item 5): useIsMobile is the single source of truth every + * mobile-only branch (bottom tab bar, collapsed topbar, bottom sheets) reads + * from, so it needs direct coverage of: (a) it reflects matchMedia's + * `matches`, (b) the `change` listener is registered AND cleaned up on + * unmount (a leaked listener would keep re-rendering an unmounted tree / + * leak the component instance), and (c) the legacy Safari<14 + * addListener/removeListener fallback is used when addEventListener isn't + * available. + * + * DOM-harness pattern mirrors PageContext.pure-updaters.test.tsx / + * useKeyboardShortcuts.test.tsx: bare createRoot + act, no @testing-library + * (not a project dependency). + */ + +let container: HTMLDivElement; +let root: Root; +let originalMatchMedia: typeof window.matchMedia; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { + root.unmount(); + }); + container.remove(); +} + +/** A minimal fake MediaQueryList. `mode` picks which listener API it + * exposes, so tests can force the legacy fallback path. */ +function makeMql(matches: boolean, mode: 'modern' | 'legacy' = 'modern') { + const listeners: Array<() => void> = []; + const mql: any = { + matches, + media: '(max-width: 768px)', + }; + if (mode === 'modern') { + mql.addEventListener = vi.fn((_type: string, fn: () => void) => listeners.push(fn)); + mql.removeEventListener = vi.fn((_type: string, fn: () => void) => { + const i = listeners.indexOf(fn); + if (i !== -1) listeners.splice(i, 1); + }); + } else { + mql.addListener = vi.fn((fn: () => void) => listeners.push(fn)); + mql.removeListener = vi.fn((fn: () => void) => { + const i = listeners.indexOf(fn); + if (i !== -1) listeners.splice(i, 1); + }); + } + return { mql, fire: () => listeners.forEach((fn) => fn()), listenerCount: () => listeners.length }; +} + +let probedValue: boolean | null = null; +const Probe: React.FC = () => { + probedValue = useIsMobile(); + return ; +}; + +beforeEach(() => { + originalMatchMedia = window.matchMedia; + probedValue = null; +}); + +afterEach(() => { + window.matchMedia = originalMatchMedia; +}); + +describe('useIsMobile', () => { + test('returns true when the mobile media query matches', () => { + const { mql } = makeMql(true); + window.matchMedia = vi.fn(() => mql) as any; + + render(); + expect(probedValue).toBe(true); + unmount(); + }); + + test('returns false when the mobile media query does not match', () => { + const { mql } = makeMql(false); + window.matchMedia = vi.fn(() => mql) as any; + + render(); + expect(probedValue).toBe(false); + unmount(); + }); + + test('registers the change listener via addEventListener and updates on change', () => { + const { mql, fire, listenerCount } = makeMql(false); + window.matchMedia = vi.fn(() => mql) as any; + + render(); + expect(probedValue).toBe(false); + expect(mql.addEventListener).toHaveBeenCalledWith('change', expect.any(Function)); + expect(listenerCount()).toBe(1); + + // Flip the query result and fire the registered 'change' listener -- + // the hook must re-read mql.matches, not just toggle blindly. + mql.matches = true; + act(() => { + fire(); + }); + expect(probedValue).toBe(true); + + unmount(); + }); + + test('cleans up the addEventListener listener on unmount', () => { + const { mql, listenerCount } = makeMql(true); + window.matchMedia = vi.fn(() => mql) as any; + + render(); + expect(listenerCount()).toBe(1); + + unmount(); + + expect(mql.removeEventListener).toHaveBeenCalledWith('change', expect.any(Function)); + expect(listenerCount()).toBe(0); + }); + + test('falls back to legacy addListener/removeListener when addEventListener is unavailable', () => { + const { mql, fire, listenerCount } = makeMql(false, 'legacy'); + window.matchMedia = vi.fn(() => mql) as any; + + render(); + expect(probedValue).toBe(false); + expect(mql.addListener).toHaveBeenCalledWith(expect.any(Function)); + expect(listenerCount()).toBe(1); + + mql.matches = true; + act(() => { + fire(); + }); + expect(probedValue).toBe(true); + + unmount(); + + expect(mql.removeListener).toHaveBeenCalledWith(expect.any(Function)); + expect(listenerCount()).toBe(0); + }); +}); diff --git a/craft/src/panels/left/AssetsPanel.tsx b/craft/src/panels/left/AssetsPanel.tsx index 034b2ac..26572c9 100644 --- a/craft/src/panels/left/AssetsPanel.tsx +++ b/craft/src/panels/left/AssetsPanel.tsx @@ -2,12 +2,14 @@ import React, { useEffect, useRef, useState, useCallback } from 'react'; import { useAssets } from '../../hooks/useAssets'; import { clickableProps } from '../../utils/a11y'; import { copyToClipboard } from '../../utils/clipboard'; +import { useIsMobile } from '../../hooks/useIsMobile'; /** How long the "Delete?" confirm state stays armed before auto-resetting. */ const DELETE_CONFIRM_TIMEOUT_MS = 4000; export const AssetsPanel: React.FC = () => { const { assets, loading, error, loadAssets, uploadAsset, deleteAsset } = useAssets(); + const isMobile = useIsMobile(); const fileInputRef = useRef(null); const [isDragOver, setIsDragOver] = useState(false); const [copiedUrl, setCopiedUrl] = useState(null); @@ -139,7 +141,9 @@ export const AssetsPanel: React.FC = () => { {assets.length === 0 && ( )} - {assets.length === 0 ? 'Drag images here or click to upload' : 'Drop files here to upload'} + {assets.length === 0 + ? (isMobile ? 'Tap to upload images' : 'Drag images here or click to upload') + : 'Drop files here to upload'}
{/* Error message */} diff --git a/craft/src/panels/mobile/BottomSheet.tsx b/craft/src/panels/mobile/BottomSheet.tsx index 920920b..aed4f59 100644 --- a/craft/src/panels/mobile/BottomSheet.tsx +++ b/craft/src/panels/mobile/BottomSheet.tsx @@ -44,7 +44,11 @@ export const BottomSheet: React.FC = ({ open, onClose, title, if (e.target === e.currentTarget) onClose(); }} > -
+ {/* Deliberately NOT aria-modal: the tab bar stays reachable/operable + while a sheet is open (tapping another tab swaps sheets directly), + so the rest of the screen must not be marked inert to assistive + tech the way a true modal dialog would be. */} +
diff --git a/craft/src/panels/mobile/MobilePanelBar.test.tsx b/craft/src/panels/mobile/MobilePanelBar.test.tsx new file mode 100644 index 0000000..f533b0b --- /dev/null +++ b/craft/src/panels/mobile/MobilePanelBar.test.tsx @@ -0,0 +1,145 @@ +import { describe, test, expect, vi } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { MobilePanelBar } from './MobilePanelBar'; +import { MobileChromeProvider } from '../../state/MobileChromeContext'; + +/** + * Mobile-A2 (review item 5): the real BlocksPanel/PagesPanel/LayersPanel/ + * AssetsPanel/GuidedStyles all pull in Craft.js's `useEditor` and other + * heavy dependencies not relevant here, so they're stubbed out -- this + * suite is only exercising MobilePanelBar's OWN wiring: tapping a tab opens + * that tab's sheet with only its panel rendered, tapping the active tab + * again closes it, and (mirroring MobileChromeContext.test.tsx) only one + * sheet's content is ever mounted at once. + */ +vi.mock('../left/BlocksPanel', () => ({ BlocksPanel: () =>
})); +vi.mock('../left/PagesPanel', () => ({ PagesPanel: () =>
})); +vi.mock('../left/LayersPanel', () => ({ LayersPanel: () =>
})); +vi.mock('../left/AssetsPanel', () => ({ AssetsPanel: () =>
})); +vi.mock('../right/GuidedStyles', () => ({ GuidedStyles: () =>
})); + +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { + root.unmount(); + }); + container.remove(); +} + +function tabButton(label: string): HTMLButtonElement { + const buttons = Array.from(container.querySelectorAll('.mobile-tab-btn')) as HTMLButtonElement[]; + const btn = buttons.find((b) => b.textContent?.includes(label)); + if (!btn) throw new Error(`No tab button found for "${label}"`); + return btn; +} + +function openPanelIds(): string[] { + return Array.from(container.querySelectorAll('[data-testid^="panel-"]')).map( + (el) => (el as HTMLElement).dataset.testid!, + ); +} + +describe('MobilePanelBar', () => { + test('no sheet is open initially', () => { + render( + + + , + ); + expect(container.querySelector('.mobile-sheet-backdrop')).toBeNull(); + expect(openPanelIds()).toEqual([]); + unmount(); + }); + + test('tapping a tab opens exactly that tab\'s panel', () => { + render( + + + , + ); + + act(() => { + tabButton('Blocks').click(); + }); + expect(openPanelIds()).toEqual(['panel-blocks']); + + unmount(); + }); + + test('switching tabs replaces the open sheet -- only one panel mounted at a time', () => { + render( + + + , + ); + + act(() => { + tabButton('Blocks').click(); + }); + expect(openPanelIds()).toEqual(['panel-blocks']); + + act(() => { + tabButton('Styles').click(); + }); + expect(openPanelIds()).toEqual(['panel-styles']); + + act(() => { + tabButton('Assets').click(); + }); + expect(openPanelIds()).toEqual(['panel-assets']); + + unmount(); + }); + + test('tapping the active tab again closes the sheet', () => { + render( + + + , + ); + + act(() => { + tabButton('Pages').click(); + }); + expect(openPanelIds()).toEqual(['panel-pages']); + expect(container.querySelector('.mobile-sheet-backdrop')).not.toBeNull(); + + act(() => { + tabButton('Pages').click(); + }); + expect(openPanelIds()).toEqual([]); + expect(container.querySelector('.mobile-sheet-backdrop')).toBeNull(); + + unmount(); + }); + + test('active tab button carries aria-pressed=true only for the open sheet', () => { + render( + + + , + ); + + act(() => { + tabButton('Layers').click(); + }); + + expect(tabButton('Layers').getAttribute('aria-pressed')).toBe('true'); + expect(tabButton('Blocks').getAttribute('aria-pressed')).toBe('false'); + + unmount(); + }); +}); diff --git a/craft/src/panels/mobile/MobilePanelBar.tsx b/craft/src/panels/mobile/MobilePanelBar.tsx index cd438fb..06ad59a 100644 --- a/craft/src/panels/mobile/MobilePanelBar.tsx +++ b/craft/src/panels/mobile/MobilePanelBar.tsx @@ -1,14 +1,13 @@ -import React, { useState } from 'react'; +import React from 'react'; import { BottomSheet } from './BottomSheet'; import { BlocksPanel } from '../left/BlocksPanel'; import { PagesPanel } from '../left/PagesPanel'; import { LayersPanel } from '../left/LayersPanel'; import { AssetsPanel } from '../left/AssetsPanel'; import { GuidedStyles } from '../right/GuidedStyles'; +import { useMobileChrome, MobileSheetTab } from '../../state/MobileChromeContext'; -type MobileTab = 'blocks' | 'pages' | 'layers' | 'assets' | 'styles'; - -const TABS: { id: MobileTab; label: string; icon: string }[] = [ +const TABS: { id: MobileSheetTab; label: string; icon: string }[] = [ { id: 'blocks', label: 'Blocks', icon: 'fa-cube' }, { id: 'pages', label: 'Pages', icon: 'fa-file-o' }, { id: 'layers', label: 'Layers', icon: 'fa-sitemap' }, @@ -30,24 +29,30 @@ const TABS: { id: MobileTab; label: string; icon: string }[] = [ * per-type style panel once something is -- this is the natural place to * edit a tapped element's styling (Phase B deepens the tap-to-select flow; * this component only wires up the sheet, per the Phase A brief). + * + * Sheet-open state itself lives in `MobileChromeContext` (Mobile-A2), not + * a private `useState`, so Phase B can open/close a sheet from elsewhere + * (e.g. a canvas selection toolbar). The toggle-to-close-on-repeat-tap + * behavior is local to this click handler -- `openSheet` always opens. */ export const MobilePanelBar: React.FC = () => { - const [activeTab, setActiveTab] = useState(null); + const { activeSheet, openSheet, closeSheet } = useMobileChrome(); - const handleTabClick = (tab: MobileTab) => { - setActiveTab((current) => (current === tab ? null : tab)); + const handleTabClick = (tab: MobileSheetTab) => { + if (activeSheet === tab) closeSheet(); + else openSheet(tab); }; - const activeLabel = TABS.find((t) => t.id === activeTab)?.label ?? ''; + const activeLabel = TABS.find((t) => t.id === activeSheet)?.label ?? ''; return ( <> - setActiveTab(null)} title={activeLabel}> - {activeTab === 'blocks' && } - {activeTab === 'pages' && } - {activeTab === 'layers' && } - {activeTab === 'assets' && } - {activeTab === 'styles' && } + + {activeSheet === 'blocks' && } + {activeSheet === 'pages' && } + {activeSheet === 'layers' && } + {activeSheet === 'assets' && } + {activeSheet === 'styles' && }