Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/web/public/mobile.css
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -574,18 +575,31 @@ 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;
/* Tighten the visible icon spacing without shrinking the touch target. */
padding-inline-end: 12px;
}

.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
Expand Down
7 changes: 3 additions & 4 deletions src/web/public/settings-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 11 additions & 11 deletions test/mobile-header-buttons-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,17 @@ 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
// NOT here: it's opt-in (default-hidden everywhere via its own --hidden marker), so
// 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}="([^"]*)"`));
Expand Down Expand Up @@ -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
Expand Down
181 changes: 181 additions & 0 deletions test/mobile/file-browser.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading