From e7a4b36fb21289f03905c77fcd8309ac3e59b9b3 Mon Sep 17 00:00:00 2001 From: "propr-dev[bot]" <1316198+propr-dev[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:17:11 +0000 Subject: [PATCH 1/3] On phones, a touch-only user can now open global search: the ProPR menu has a Search button in its header, next to Close. Tapping it closes the menu and opens the existing search sheet with its input focused. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit How it works - The button goes through the same `requestSearch` path that Ctrl/Cmd+K and the native Search command already use (`GlobalHeader.tsx`). It doesn't fake a keypress, need native IPC, add an endpoint or change permissions. - The ProPR menu closes before Search opens, so only one modal layer is ever up. Focus moves to the More tab first, so closing Search (with Close, Escape or the backdrop) puts focus back on More. Opening and closing repeatedly behaves the same each time. - The ProPR menu has no unsent work to lose. The Create sheet, which holds the quick to-do draft, is untouched. - The logo, title and all destinations stay as they were. Toolbar search, Ctrl/Cmd+K, the native Search command on narrow or zoomed windows, and the Tasks/Plans/Goals list searches are unchanged. Why it's in the header: my first version put a search row at the top of the menu body. With 150% text at 320px that pushed the Workspace tiles below the fold, and the existing branding test failed. A 44px icon button beside Close keeps everything else where it was. Code changes - `MobileBottomSheet.tsx` gained an optional `headerAction` slot. - The button lives in a new file, `MobileMenuSearchButton.tsx`. - `MobileBottomNavigation.tsx` was already at the lint limit (399 of 400 code lines, complexity 20 of 20). So I moved the existing profile row unchanged into `MobileProfileRow.tsx`, and made `onSearch` required (GlobalHeader is its only caller). Tests - New browser test (`e2e/mobile-menu-search.pw.ts`): a touch-only phone at 320×640 and 390×844, no desktop adapter. It covers: - opening Search by tap, with the input focused and only one modal layer. - Close and the search input not being covered by anything. - long titles (about 170 and 500 characters) shown in full with metadata below, no horizontal overflow and no raw model ID. - closing and reopening, with focus returning to More. - tapping a result to open the right task, then Ctrl+K still working. - New unit tests: the menu hands off to Search with focus on More, and in a plain browser the header opens the search sheet from the menu without a desktop adapter. - Results: 31/31 Playwright tests pass across the new spec and the existing branding, native-zoom, row-layout, palette and list-search specs. All 1560 component unit tests pass, `tsc --noEmit` is clean, and `eslint .` reports no problems. I looked at the 320 and 390 screenshots, including the enlarged-text menu: nothing is clipped or overflows. A representative set is saved for preview. --- propr-ui/e2e/mobile-menu-search.pw.ts | 172 ++++++++++++++++++ .../GlobalHeader.nativeSearch.test.tsx | 22 ++- propr-ui/src/components/GlobalHeader.tsx | 11 +- .../MobileBottomNavigation.test.tsx | 27 ++- .../src/components/MobileBottomNavigation.tsx | 46 ++--- propr-ui/src/components/MobileBottomSheet.tsx | 7 +- .../src/components/MobileMenuSearchButton.tsx | 20 ++ propr-ui/src/components/MobileProfileRow.tsx | 34 ++++ 8 files changed, 299 insertions(+), 40 deletions(-) create mode 100644 propr-ui/e2e/mobile-menu-search.pw.ts create mode 100644 propr-ui/src/components/MobileMenuSearchButton.tsx create mode 100644 propr-ui/src/components/MobileProfileRow.tsx diff --git a/propr-ui/e2e/mobile-menu-search.pw.ts b/propr-ui/e2e/mobile-menu-search.pw.ts new file mode 100644 index 0000000000..2e58e89b62 --- /dev/null +++ b/propr-ui/e2e/mobile-menu-search.pw.ts @@ -0,0 +1,172 @@ +import { expect, test, type Browser, type Locator, type Page } from '@playwright/test'; +import { captureTarget, fixture, minutesAgo } from './dashboard-sections.fixture'; + +// Below `md` the toolbar (and its search field) is hidden, so the ProPR menu's +// Search row is the only way a touch-only operator reaches global search. +// These run in a plain browser: no desktop adapter, no keyboard to open it. + +const repository = 'example-organisation/very-long-repository-name-for-search-layout-validation-workspace'; +// Titles at the lengths real saved plans reach (~170 and ~500 characters). +const mediumTitle = 'Make the repository settings page keep unsaved branch protection edits when the operator ' + + 'switches tabs, and show which fields still differ from the saved configuration'; +const longTitle = 'Consolidate the release readiness checklist so that every packaging target (signed Linux ' + + 'RPM and DEB, notarised macOS DMG, Windows MSI) reports the same artefact provenance, verifies the ' + + 'update feed signature against packages/desktop/src/update/verification/release-feed-signature-policy.ts, ' + + 'records the reviewer who approved the build, links the matching changelog entry, and refuses to publish ' + + 'when any target is missing a reproducible build attestation or the smoke suite on clean hosts fails'; + +const plans = [mediumTitle, longTitle, 'Short plan about search layout'].map((name, index) => ({ + draft_id: `plan-${index}`, repository, name, initial_prompt: `Synthetic plan ${index + 1} for the layout check.`, + status: 'review', created_at: minutesAgo((index + 1) * 60), updated_at: minutesAgo((index + 1) * 60), + issue_summary: { total: 3, pending: 1, processing: 1, merged: 1, closed: 0 }, +})); +const tasks = [{ + id: 'task-1', title: longTitle, status: 'completed', prNumber: 2480, score: 9, repository, + issueNumber: 2400, model: 'gpt-6-astra', subtitle: 'Synthetic task.', createdAt: minutesAgo(3 * 24 * 60), +}]; + +const viewports = [ + { name: '320', width: 320, height: 640 }, + { name: '390', width: 390, height: 844 }, +]; + +async function openPhone(browser: Browser, viewport: { width: number; height: number }) { + const context = await browser.newContext({ viewport, hasTouch: true, isMobile: true, deviceScaleFactor: 2 }); + const page = await context.newPage(); + await fixture(page, viewport, [], []); + await page.route('**/api/planner/drafts?**', route => route.fulfill({ json: { drafts: plans, total: plans.length, page: 1, limit: 5, hasMore: false } })); + await page.route('**/api/tasks?**', route => route.fulfill({ json: { tasks, total: tasks.length } })); + await page.goto('/'); + await expect(page.getByRole('main')).toBeVisible(); + return { context, page }; +} + +// Wait out the sheet's rise and the scrim's fade so taps land on settled boxes. +async function settle(page: Page) { + await page.evaluate(async () => { + const running = () => document.getAnimations().filter(animation => + animation.playState !== 'finished' && animation.playState !== 'idle' && animation.effect?.getComputedTiming().endTime !== Infinity); + for (let pending = running(); pending.length; pending = running()) await Promise.allSettled(pending.map(animation => animation.finished)); + await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); + }); +} + +const noDocumentOverflow = (page: Page) => page.evaluate(() => + document.documentElement.scrollWidth <= document.documentElement.clientWidth); + +/** The element really under the control's centre is the control: nothing covers it. */ +const isHitTarget = (control: Locator) => control.evaluate(element => { + const { left, top, width, height } = element.getBoundingClientRect(); + const hit = document.elementFromPoint(left + width / 2, top + height / 2); + return Boolean(hit && element.contains(hit)); +}); + +async function openSearchFromMenu(page: Page) { + const moreButton = page.getByRole('navigation', { name: 'Primary navigation' }).getByRole('button', { name: 'More' }); + await moreButton.tap(); + const menu = page.getByRole('dialog', { name: 'ProPR', exact: true }); + await expect(menu).toBeVisible(); + await settle(page); + await menu.getByRole('button', { name: 'Search', exact: true }).tap(); + await expect(menu).toHaveCount(0); + const sheet = page.getByRole('dialog', { name: 'Search' }); + await expect(sheet).toBeVisible(); + return { moreButton, sheet, input: sheet.getByRole('combobox', { name: 'Search' }) }; +} + +for (const viewport of viewports) { + test(`touch opens global search from the ProPR menu at ${viewport.name}px`, async ({ browser }) => { + const { context, page } = await openPhone(browser, viewport); + expect(await page.evaluate(() => 'proprDesktop' in window)).toBe(false); + await expect(page.getByRole('banner', { name: 'Application toolbar' })).toBeHidden(); + + // The menu keeps its name, mark and destinations, with Search in its header. + const moreButton = page.getByRole('navigation', { name: 'Primary navigation' }).getByRole('button', { name: 'More' }); + await moreButton.tap(); + const menu = page.getByRole('dialog', { name: 'ProPR', exact: true }); + await settle(page); + await expect(menu.getByRole('heading', { level: 2, name: 'ProPR', exact: true }).locator('img')).toHaveAttribute('alt', ''); + for (const name of ['Goals', 'Plans', 'Automations', 'Repositories', 'Settings']) { + await expect(menu.getByRole('link', { name, exact: true })).toHaveCount(1); + } + const searchRow = menu.getByRole('button', { name: 'Search', exact: true }); + await expect(searchRow).toBeInViewport({ ratio: 1 }); + const searchBox = (await searchRow.boundingBox())!; + expect(searchBox.height).toBeGreaterThanOrEqual(44); + expect(searchBox.width).toBeGreaterThanOrEqual(44); + expect(await isHitTarget(searchRow)).toBe(true); + // Title, then Search, then Close, never overlapping. + const titleBox = (await menu.getByRole('heading', { level: 2 }).boundingBox())!; + const closeBox = (await menu.getByRole('button', { name: 'Close ProPR menu' }).boundingBox())!; + expect(titleBox.x + titleBox.width).toBeLessThanOrEqual(searchBox.x + 0.5); + expect(searchBox.x + searchBox.width).toBeLessThanOrEqual(closeBox.x + 0.5); + // The workspace destinations stay above the fold. + await expect(menu.getByRole('link', { name: 'Repositories', exact: true })).toBeInViewport({ ratio: 1 }); + expect(await noDocumentOverflow(page)).toBe(true); + await captureTarget(page, `mobile-menu-search-button-${viewport.name}`); + await menu.getByRole('button', { name: 'Close ProPR menu' }).tap(); + await expect(menu).toHaveCount(0); + + // Search opens alone: the menu is gone, only one modal remains, the field has focus. + const { sheet, input } = await openSearchFromMenu(page); + await expect(input).toBeFocused(); + await expect(page.locator('[aria-modal="true"]')).toHaveCount(1); + expect(await page.evaluate(() => document.body.style.overflow)).toBe(''); + const close = sheet.getByRole('button', { name: 'Close search' }); + await expect(close).toBeInViewport({ ratio: 1 }); + expect(await isHitTarget(close)).toBe(true); + expect(await isHitTarget(input)).toBe(true); + await captureTarget(page, `mobile-menu-search-open-${viewport.name}`); + + // Real results: long titles in full with metadata beneath, nothing spilling sideways. + await input.fill('search'); + const palette = page.getByTestId('global-search-palette'); + const options = palette.getByRole('option'); + await expect(options).toHaveCount(4); + for (const index of [0, 1, 3]) { + const row = options.nth(index); + const title = row.getByText(index === 0 ? mediumTitle : longTitle, { exact: true }); + expect(await title.evaluate(el => el.scrollWidth <= el.clientWidth && el.scrollHeight <= el.clientHeight)).toBe(true); + const titleBox = (await title.boundingBox())!; + const metaBox = (await row.getByTestId('global-search-result-meta').boundingBox())!; + expect(metaBox.y).toBeGreaterThanOrEqual(titleBox.y + titleBox.height - 1); + } + const paletteBox = (await palette.boundingBox())!; + expect(paletteBox.x).toBeGreaterThanOrEqual(0); + expect(paletteBox.x + paletteBox.width).toBeLessThanOrEqual(viewport.width); + await expect(palette).not.toContainText('gpt-6-astra'); + expect(await noDocumentOverflow(page)).toBe(true); + await expect(close).toBeInViewport({ ratio: 1 }); + expect(await isHitTarget(close)).toBe(true); + await captureTarget(page, `mobile-menu-search-results-${viewport.name}`); + + // Close returns focus to More; reopening works the same way every time. + await close.tap(); + await expect(sheet).toHaveCount(0); + await expect(moreButton).toBeFocused(); + await expect(page.locator('[aria-modal="true"]')).toHaveCount(0); + for (let attempt = 0; attempt < 2; attempt += 1) { + const reopened = await openSearchFromMenu(page); + await expect(reopened.input).toBeFocused(); + await expect(reopened.input).toHaveValue(''); + await page.keyboard.press('Escape'); + await expect(reopened.sheet).toHaveCount(0); + await expect(moreButton).toBeFocused(); + } + + // A tap on the long task result opens that task and closes everything. + const final = await openSearchFromMenu(page); + await final.input.fill('search'); + await expect(options).toHaveCount(4); + await options.nth(3).scrollIntoViewIfNeeded(); + await options.nth(3).tap(); + await expect(page).toHaveURL(/\/tasks\/task-1$/); + await expect(page.locator('[aria-modal="true"]')).toHaveCount(0); + expect(await noDocumentOverflow(page)).toBe(true); + + // Ctrl+K still takes the same route on the new page. + await page.keyboard.press('Control+k'); + await expect(page.getByRole('dialog', { name: 'Search' }).getByRole('combobox', { name: 'Search' })).toBeFocused(); + await context.close(); + }); +} diff --git a/propr-ui/src/components/GlobalHeader.nativeSearch.test.tsx b/propr-ui/src/components/GlobalHeader.nativeSearch.test.tsx index f8b50834aa..59d1e9e15f 100644 --- a/propr-ui/src/components/GlobalHeader.nativeSearch.test.tsx +++ b/propr-ui/src/components/GlobalHeader.nativeSearch.test.tsx @@ -11,7 +11,14 @@ vi.mock('../hooks/useHeaderStats', () => ({ useHeaderStats: () => ({ systemHealth: { isHealthy: true, agents: [] } }), })); vi.mock('./GlobalHeaderComponents', () => ({ SystemHealth: () => null })); -vi.mock('./MobileBottomNavigation', () => ({ default: () => })); +vi.mock('./MobileBottomNavigation', () => ({ + default: ({ onSearch }: { onSearch?: () => void }) => ( + <> + + + + ), +})); vi.mock('./QuickAddTodo', () => ({ default: () => null })); // A local stand-in for the search hook: real typing state, one task result, no network. vi.mock('../hooks/useGlobalSearch', () => ({ @@ -115,6 +122,19 @@ describe('GlobalHeader native Search command', () => { expect(screen.getByLabelText('Location')).toHaveTextContent('/tasks'); }); + it('opens the same sheet from the mobile menu\'s Search row, without a desktop adapter', () => { + setWide(false); + renderHeader(false); + const menuSearch = screen.getByRole('button', { name: 'Menu Search' }); + menuSearch.focus(); + fireEvent.click(menuSearch); + const sheet = screen.getByRole('dialog', { name: 'Search' }); + expect(within(sheet).getByRole('combobox', { name: 'Search' })).toHaveFocus(); + fireEvent.click(within(sheet).getByRole('button', { name: 'Close search' })); + expect(screen.queryByRole('dialog', { name: 'Search' })).not.toBeInTheDocument(); + expect(menuSearch).toHaveFocus(); + }); + it('routes Ctrl/Cmd+K the same way as the native command at narrow widths', () => { setWide(false); renderHeader(false); diff --git a/propr-ui/src/components/GlobalHeader.tsx b/propr-ui/src/components/GlobalHeader.tsx index c159353ef7..2e0bf79321 100644 --- a/propr-ui/src/components/GlobalHeader.tsx +++ b/propr-ui/src/components/GlobalHeader.tsx @@ -72,11 +72,11 @@ function useHeaderKeyboardShortcuts( } /** - * Routes a Search request (Ctrl/Cmd+K, or the native menu's Search command) to - * a field the operator can actually use. From `md` that is the toolbar's own - * input; below it (a narrow window, or enlarged text) the toolbar is - * `display: none`, so focusing its input would silently do nothing, and the - * request opens the search sheet instead. `mobileSearch` is 0 while the sheet + * Routes a Search request (Ctrl/Cmd+K, the native menu's Search command, or the + * ProPR mobile menu's Search row) to a field the operator can actually use. + * From `md` that is the toolbar's own input; below it (a narrow window, or + * enlarged text) the toolbar is `display: none`, so focusing its input would + * silently do nothing, and the request opens the search sheet instead. `mobileSearch` is 0 while the sheet * is closed and increments per request, so a repeat refocuses the field. */ function useSearchRequests(searchInputRef: React.RefObject) { @@ -349,6 +349,7 @@ const GlobalHeader: React.FC = ({ user, onLogout, onMenuToggl unreadCount={inboxUnreadCount} systemHealth={systemHealth} quickAddRequest={mobileQuickAddRequest} + onSearch={requestSearch} /> {mobileSearch > 0 && } diff --git a/propr-ui/src/components/MobileBottomNavigation.test.tsx b/propr-ui/src/components/MobileBottomNavigation.test.tsx index fa2560ae59..5763035080 100644 --- a/propr-ui/src/components/MobileBottomNavigation.test.tsx +++ b/propr-ui/src/components/MobileBottomNavigation.test.tsx @@ -60,7 +60,8 @@ function renderNavigation( initialEntry = '/tasks/task-1', onLogout = vi.fn(), currentUser: CurrentUser | null = user, - isDemoMode = false + isDemoMode = false, + onSearch = vi.fn(), ) { render( @@ -70,6 +71,7 @@ function renderNavigation( isDemoMode={isDemoMode} unreadCount={7} systemHealth={systemHealth} + onSearch={onSearch} /> @@ -236,6 +238,29 @@ describe('MobileBottomNavigation', () => { expect(moreButton).toHaveFocus(); }); + it('opens global search from the ProPR sheet, closing the sheet first and parking focus on More', () => { + let state: { sheetOpen: boolean; focused: Element | null } | null = null; + const onSearch = vi.fn(() => { + state = { sheetOpen: Boolean(screen.queryByRole('dialog', { name: 'ProPR' })), focused: document.activeElement }; + }); + renderNavigation('/', vi.fn(), user, false, onSearch); + const moreButton = screen.getByRole('button', { name: 'More' }); + fireEvent.click(moreButton); + const sheet = screen.getByRole('dialog', { name: 'ProPR' }); + // Search leads the sheet, ahead of identity and every destination. + const search = within(sheet).getByRole('button', { name: 'Search' }); + expect(search).toHaveAttribute('aria-haspopup', 'dialog'); + expect(search.compareDocumentPosition(within(sheet).getByRole('button', { name: 'Sign out' })) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + + fireEvent.click(search); + + expect(onSearch).toHaveBeenCalledTimes(1); + // The search request records the focused element as its return target. + expect(state!.focused).toBe(moreButton); + expect(screen.queryByRole('dialog', { name: 'ProPR' })).not.toBeInTheDocument(); + expect(screen.getByTestId('location')).toHaveTextContent(/^\/$/); + }); + it('closes on route navigation and marks More active for its destinations', () => { renderNavigation('/inbox'); const moreButton = screen.getByRole('button', { name: 'More' }); diff --git a/propr-ui/src/components/MobileBottomNavigation.tsx b/propr-ui/src/components/MobileBottomNavigation.tsx index 632aa129e9..0942009222 100644 --- a/propr-ui/src/components/MobileBottomNavigation.tsx +++ b/propr-ui/src/components/MobileBottomNavigation.tsx @@ -11,7 +11,6 @@ import { Inbox, LayoutDashboard, ListTodo, - LogOut, MoreHorizontal, Plug, Plus, @@ -27,10 +26,11 @@ import type { CurrentUser } from '../api/proprTypes'; import { userHasPermission } from '../contexts/AuthContext'; import { publicAssetUrl } from '../config/runtimeMode'; import AgentTankSidebar from './AgentTankSidebar'; -import UserAvatar from './UserAvatar'; import QuickAddTodo from './QuickAddTodo'; import { creationActions } from './creationActions'; import BottomSheet from './MobileBottomSheet'; +import MobileMenuSearchButton from './MobileMenuSearchButton'; +import MobileProfileRow from './MobileProfileRow'; interface MobileBottomNavigationProps { user: CurrentUser | null; @@ -40,6 +40,8 @@ interface MobileBottomNavigationProps { systemHealth: HeaderStats['systemHealth']; /** Increments to open the Create sheet with quick add expanded (Desktop New To-do). */ quickAddRequest?: number; + /** Opens global search through the header's shared request path (Ctrl/Cmd+K, native Search). */ + onSearch: () => void; } type SheetId = 'create' | 'more' | 'usage'; @@ -194,34 +196,6 @@ const MoreLink: React.FC<{ item: MoreItem; onNavigate: () => void }> = ({ item, ); -// Identity and Sign out share a line while the name has room to be read; -// with narrow screens or enlarged text Sign out wraps beneath it instead of -// squeezing the name down to a couple of letters. -const ProfileRow: React.FC<{ user: CurrentUser; onLogout: () => void }> = ({ user, onLogout }) => ( -
-
- -
-

{user.displayName || user.username}

-

@{user.username}

-
-
- -
-); - const HealthDetails: React.FC<{ systemHealth: HeaderStats['systemHealth'] }> = ({ systemHealth }) => { const rows = [ ['Daemon', systemHealth.daemon], @@ -264,6 +238,7 @@ const MobileBottomNavigation: React.FC = ({ unreadCount, systemHealth, quickAddRequest, + onSearch, }) => { const location = useLocation(); const [openSheet, setOpenSheet] = useState(null); @@ -282,6 +257,14 @@ const MobileBottomNavigation: React.FC = ({ const backToMore = useCallback(() => setOpenSheet('more'), []); // Following a link closes the sheet without pulling focus back to the bar. const closeForNavigation = () => setOpenSheet(null); + // The sheet closes before search opens, so the two modals never compete for + // focus. Focus parks on More first: the search request remembers it as the + // place to return to, since the button pressed here is about to unmount. + const openSearch = () => { + setOpenSheet(null); + moreButtonRef.current?.focus(); + onSearch(); + }; const toggleSheet = (sheet: SheetId) => setOpenSheet(current => (current === sheet ? null : sheet)); useEffect(() => { @@ -344,9 +327,10 @@ const MobileBottomNavigation: React.FC = ({ title="ProPR" titleIcon={} closeLabel="Close ProPR menu" + headerAction={} onClose={closeMore} > - {user && } + {user && }