fix(files): honor File Viewer header opt-in on phones

This commit is contained in:
DodgyBadger committed 2026-11-11 20:16:19 +00:00
1 parent 8bedbbe793
commit 81c7708353
4 files changed
+211 -19

No files matched your search

+16 -4
View File
@@ -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
+3 -4
View File
@@ -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) {
+11 -11
View File
@@ -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<string>([]);
// 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<string>(['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<string>([]);
// 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
+181
View File
@@ -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);
});
});