From 81c770835361e16db998434861b495a09e54f05b Mon Sep 17 00:00:00 2001 From: DodgyBadger <4453217+DodgyBadger@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:16:19 +0000 Subject: [PATCH 1/2] fix(files): honor File Viewer header opt-in on phones --- src/web/public/mobile.css | 20 ++- src/web/public/settings-ui.js | 7 +- test/mobile-header-buttons-policy.test.ts | 22 +-- test/mobile/file-browser.test.ts | 181 ++++++++++++++++++++++ 4 files changed, 211 insertions(+), 19 deletions(-) create mode 100644 test/mobile/file-browser.test.ts diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 9a8c3a384..d4459165c 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -13,6 +13,7 @@ html.mobile-init .header-font-controls, html.mobile-init .header-system-stats, html.mobile-init .header-tokens, +html.mobile-init .btn-file-viewer, html.mobile-init .monitor-panel, html.mobile-init .subagents-panel, html.mobile-init .project-insights-panel, @@ -574,18 +575,29 @@ html.mobile-init .file-browser-panel { height: 18px; } - /* Hide header settings gear, lifecycle log, away digest, session manager, and - file viewer on mobile - settings moved to toolbar; the others are secondary / + /* File Viewer is off by default on phones, but honors an explicit opt-in. + Give its header entry the same minimum touch target as the home button. */ + .btn-icon-header.btn-file-viewer { + width: 44px; + height: 44px; + } + + .btn-icon-header.btn-file-viewer svg { + width: 18px; + height: 18px; + } + + /* Hide header settings gear, lifecycle log, away digest, and session manager + on mobile - settings moved to toolbar; the others are secondary / desktop-oriented controls that don't belong on the cramped phone header (the session manager stays reachable via the Ctrl+K palette's "Browse all sessions" - item; the file viewer button is opt-in but its panel is desktop-sized). + item). (The attachments button is opt-in / default-hidden everywhere via its own --hidden marker, so it needs no mobile-specific rule here.) */ .btn-icon-header.btn-settings, .btn-icon-header.btn-lifecycle-log, .btn-icon-header.btn-away-digest, .btn-icon-header.btn-session-manager, - .btn-icon-header.btn-file-viewer, /* Edit history: a desktop panel; phones never show the button (opt-in, default OFF anyway). */ .btn-icon-header.btn-edit-history, /* Approvals bell: phones answer from the overview's NEEDS YOU rows instead diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index c6ce4510e..bf24a1e5f 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -4748,10 +4748,9 @@ Object.assign(CodemanApp.prototype, { attachmentsBtn.classList.toggle('btn-attachments-history--hidden', !showAttachmentsButton); } - // File Viewer header button — opt-in, default OFF. Marker class (base is - // display:inline-flex !important); clicking it toggles the file browser panel. - // Default ON (desktop): the folder button is part of the standard header now; - // phones still hide it via mobile.css (btn-file-viewer in the phone-hidden set). + // File Viewer header button: default ON on desktop, opt-in on phones. + // Marker class (base is display:inline-flex !important); clicking it toggles + // the file browser panel on either device. const showFileViewerButton = settings.showFileViewerButton ?? defaults.showFileViewerButton ?? true; const fileViewerBtn = document.querySelector('.btn-file-viewer'); if (fileViewerBtn) { diff --git a/test/mobile-header-buttons-policy.test.ts b/test/mobile-header-buttons-policy.test.ts index 90dbe1bcb..f10cfb5f5 100644 --- a/test/mobile-header-buttons-policy.test.ts +++ b/test/mobile-header-buttons-policy.test.ts @@ -29,10 +29,9 @@ const PUBLIC = join(HERE, '../src/web/public'); // Matches the device the browser-based test emulates (iPhone 14 Pro = 393px CSS). const PHONE_WIDTH = 393; -// Header buttons intentionally kept VISIBLE in the phone header. Empty today: the -// mobile header is deliberately minimal and essential controls (settings, case) -// live in the toolbar. Add a class here ONLY with a justifying comment. -const MOBILE_VISIBLE_ALLOWLIST = new Set([]); +// Issue #759: the maintainer approved File Viewer on phones when explicitly enabled. +// Its per-device setting still defaults off; the browser test covers that default. +const MOBILE_VISIBLE_ALLOWLIST = new Set(['btn-file-viewer']); // Buttons we expect to STAY hidden on phones — an explicit lock so a future edit // that removes a hide rule fails loudly (not silently). The attachments button is @@ -40,13 +39,7 @@ const MOBILE_VISIBLE_ALLOWLIST = new Set([]); // it's excluded from the default-visible enumeration rather than mobile-hidden. // The edit history button is opt-in the same way, but it IS here: its panel is // desktop-sized, so it stays off phones even when the setting is on. -const KNOWN_PHONE_HIDDEN = [ - 'btn-settings', - 'btn-lifecycle-log', - 'btn-session-manager', - 'btn-file-viewer', - 'btn-edit-history', -]; +const KNOWN_PHONE_HIDDEN = ['btn-settings', 'btn-lifecycle-log', 'btn-session-manager', 'btn-edit-history']; function attrOf(openTag: string, name: string): string { const m = openTag.match(new RegExp(`${name}="([^"]*)"`)); @@ -150,6 +143,13 @@ describe('Mobile header button policy (static guard)', () => { ).toBe(true); } }); + + it('allows the explicitly enabled File Viewer button on phones (issue #759)', () => { + // The setting removes the --hidden marker; a separate blanket phone hide + // would override that choice. Default-off and live toggling are exercised + // against the real page in test/mobile/file-browser.test.ts. + expect(phoneHidden.has('btn-file-viewer')).toBe(false); + }); }); // The flip side of the policy above: the ONE header control phones do keep has diff --git a/test/mobile/file-browser.test.ts b/test/mobile/file-browser.test.ts new file mode 100644 index 000000000..a44e15fcb --- /dev/null +++ b/test/mobile/file-browser.test.ts @@ -0,0 +1,181 @@ +// Issue #759: opt-in phone access, exercised against the real app and filesystem. +// Run with npm run test:mobile -- test/mobile/file-browser.test.ts (not the CI runner). +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { mkdir, writeFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import type { BrowserContext, Locator, Page } from 'playwright'; +import type { WebServer } from '../../src/web/server.js'; +import { assertSandboxedHome, safeRmHomeTree } from '../mocks/test-helpers.js'; +import { REPRESENTATIVE_DEVICES } from './devices.js'; +import { createDevicePage, closeAllBrowsers } from './helpers/browser.js'; +import { assertTouchTarget } from './helpers/assertions.js'; +import { STORAGE_KEYS } from './helpers/constants.js'; +import { createTestServer, stopTestServer } from './helpers/server.js'; + +describe('File browser on a phone (issue #759)', () => { + let server: WebServer; + let baseUrl: string; + let sessionId: string; + let workspace: string; + let context: BrowserContext; + let page: Page; + + beforeAll(async () => { + assertSandboxedHome('Phone file browser fixture'); + workspace = join(homedir(), 'file-browser-phone'); + await mkdir(join(workspace, 'src'), { recursive: true }); + await writeFile(join(workspace, 'README.md'), 'Phone browser fixture'); + await writeFile(join(workspace, '.hidden.txt'), 'Hidden fixture'); + await writeFile(join(workspace, 'src', 'note.txt'), 'Nested fixture'); + server = await createTestServer(); + baseUrl = `http://localhost:${server.boundPort}`; + const response = await fetch(`${baseUrl}/api/sessions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ name: 'phone-files', mode: 'shell', workingDir: workspace }), + }); + const body = await response.json(); + expect(response.ok, JSON.stringify(body)).toBe(true); + expect(body.success, JSON.stringify(body)).toBe(true); + expect(body.data?.session?.id).toBeTypeOf('string'); + sessionId = body.data.session.id; + }); + + beforeEach(async () => { + ({ context, page } = await createDevicePage( + { + ...REPRESENTATIVE_DEVICES['standard-phone'], + viewport: { width: 390, height: 844 }, + deviceScaleFactor: 1, + }, + baseUrl, + 'chromium' + )); + await page.waitForFunction((id) => (window as any).app?.sessions.has(id), sessionId); + await page.evaluate(async (id) => { + await (window as any).app.selectSession(id); + }, sessionId); + }); + + afterEach(async () => { + await context?.close(); + }); + + afterAll(async () => { + await closeAllBrowsers(); + if (sessionId) await fetch(`${baseUrl}/api/sessions/${sessionId}`, { method: 'DELETE' }); + if (server) await stopTestServer(server); + if (workspace) safeRmHomeTree(workspace); + }); + + async function saveSetting(id: string, checked: boolean) { + await page.locator('.btn-settings-mobile').tap(); + const control = page.locator(`#${id}`); + // The chip's native checkbox is visually hidden; the label is the touch target. + if ((await control.isChecked()) !== checked) { + await page.locator(`label:has(#${id})`).tap(); + } + await page + .locator('#appSettingsModal .set-head-save') + .filter({ hasText: /^Save$/ }) + .tap(); + await expect.poll(() => page.locator('#appSettingsModal').isVisible()).toBe(false); + } + + async function expectReachable(control: Locator) { + expect(await control.isVisible()).toBe(true); + const box = await control.boundingBox(); + expect(box).not.toBeNull(); + const viewport = page.viewportSize()!; + // Allow subpixel rounding at the fixed header's border. + expect(box!.x).toBeGreaterThanOrEqual(-1); + expect(box!.y).toBeGreaterThanOrEqual(-1); + expect(box!.x + box!.width).toBeLessThanOrEqual(viewport.width + 1); + expect(box!.y + box!.height).toBeLessThanOrEqual(viewport.height + 1); + // A Settings-saved toast can briefly cover the panel after opening it. + // Wait for normal feedback to clear, but reject persistent occlusion. + await expect + .poll( + () => + control.evaluate((element) => { + const rect = element.getBoundingClientRect(); + const target = document.elementFromPoint(rect.x + rect.width / 2, rect.y + rect.height / 2); + return { reachable: target !== null && element.contains(target), target: target?.outerHTML.slice(0, 200) }; + }), + { timeout: 5000 } + ) + .toMatchObject({ reachable: true }); + } + + it('keeps the phone header button off by default', async () => { + expect(await page.evaluate(() => (window as any).app.getDefaultSettings().showFileViewerButton)).toBe(false); + expect(await page.locator('.btn-file-viewer').isVisible()).toBe(false); + }); + + it('honors enable/save/reload, opens and closes from the header, and honors disabling', async () => { + await saveSetting('appSettingsShowFileViewerButton', true); + const button = page.locator('.btn-file-viewer'); + await expect.poll(() => button.isVisible()).toBe(true); + expect( + await page.evaluate( + (key) => JSON.parse(localStorage.getItem(key)!).showFileViewerButton, + STORAGE_KEYS.SETTINGS_MOBILE + ) + ).toBe(true); + await page.reload({ waitUntil: 'domcontentloaded' }); + await page.waitForFunction((id) => (window as any).app?.sessions.has(id), sessionId); + + await expect.poll(() => button.isVisible(), { timeout: 3000 }).toBe(true); + await expectReachable(button); + await assertTouchTarget(button); + const panel = page.locator('#fileBrowserPanel'); + await button.tap(); + await expect.poll(() => panel.isVisible()).toBe(true); + await page.locator('#fileBrowserTree [data-path="README.md"]').waitFor({ state: 'visible' }); + await button.tap(); + await expect.poll(() => panel.isVisible()).toBe(false); + await button.tap(); + await expect.poll(() => panel.isVisible()).toBe(true); + await panel.locator('button[title="Close"]').tap(); + await expect.poll(() => panel.isVisible()).toBe(false); + + await saveSetting('appSettingsShowFileViewerButton', false); + expect(await button.isVisible()).toBe(false); + await page.reload({ waitUntil: 'domcontentloaded' }); + await page.waitForFunction((id) => (window as any).app?.sessions.has(id), sessionId); + expect(await button.isVisible()).toBe(false); + }); + + it('keeps the panel and essential controls usable at 390px through the existing Settings entry', async () => { + // Exercise panel usability independently of the missing header entry point. + await saveSetting('appSettingsShowFileBrowser', true); + const panel = page.locator('#fileBrowserPanel'); + await expect.poll(() => panel.isVisible()).toBe(true); + const tree = panel.locator('#fileBrowserTree'); + await tree.locator('[data-path="README.md"]').waitFor({ state: 'visible' }); + for (const control of [ + panel, + panel.locator('#fileBrowserSearch'), + panel.locator('#fileBrowserHiddenBtn'), + panel.locator('button[title="Refresh"]'), + panel.locator('#fileBrowserExpandBtn'), + panel.locator('button[title="Close"]'), + ]) { + await expectReachable(control); + } + + await panel.locator('#fileBrowserExpandBtn').tap(); + await tree.locator('[data-path="src/note.txt"]').waitFor({ state: 'visible' }); + await panel.locator('#fileBrowserHiddenBtn').tap(); + await tree.locator('[data-path=".hidden.txt"]').waitFor({ state: 'visible' }); + expect(await tree.locator('[data-path="README.md"]').isVisible()).toBe(true); + await panel.locator('#fileBrowserSearch').fill('note.txt'); + await expect.poll(() => tree.locator('[data-path="README.md"]').count()).toBe(0); + await tree.locator('[data-path="src/note.txt"]').waitFor({ state: 'visible' }); + await panel.locator('button[title="Refresh"]').tap(); + await tree.locator('[data-path="README.md"]').waitFor({ state: 'visible' }); + await panel.locator('button[title="Close"]').tap(); + await expect.poll(() => panel.isVisible()).toBe(false); + }); +}); From 6c2e257d480356392cb130fdbb4bbf0abd005006 Mon Sep 17 00:00:00 2001 From: DodgyBadger <4453217+DodgyBadger@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:09:54 +0000 Subject: [PATCH 2/2] fix(files): tighten phone File Viewer icon spacing --- src/web/public/mobile.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index d4459165c..db00b9c47 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -580,6 +580,8 @@ html.mobile-init .file-browser-panel { .btn-icon-header.btn-file-viewer { width: 44px; height: 44px; + /* Tighten the visible icon spacing without shrinking the touch target. */ + padding-inline-end: 12px; } .btn-icon-header.btn-file-viewer svg {