diff --git a/packages/webapp/src/CloudApp.tsx b/packages/webapp/src/CloudApp.tsx index 64742a45..cb328114 100644 --- a/packages/webapp/src/CloudApp.tsx +++ b/packages/webapp/src/CloudApp.tsx @@ -35,6 +35,7 @@ import { AccessApprovalDialog } from './AccessApprovalDialog'; import { useAccessProposals } from './use-access-proposals'; import type { ConnectionsFocus, WorkspaceDetailsTab } from './WorkspaceDetailsDialog'; import { ShellNav } from './shell/ShellNav'; +import { routeShowsMobileRail } from './shell/mobile-rail'; import { isSecondaryRoute, SecondaryRoutes } from './shell/SecondaryRoutes'; import { NewTabControl } from './shell/NewTabControl'; import { WorkPanes } from './shell/WorkPanes'; @@ -283,9 +284,10 @@ function CloudAppContent({ client, resolver }: CloudAppProps) { if (!mobileWebApp) setDrawerOpen(false); }, [mobileWebApp]); + const railIsWorkspaceScreen = routeShowsMobileRail(route); useEffect(() => { - setDrawerOpen(false); - }, [route.page, route.workspaceId]); + setDrawerOpen(mobileWebApp && railIsWorkspaceScreen); + }, [mobileWebApp, railIsWorkspaceScreen, route.workspaceId]); useEffect(() => { if (!mobileWebApp) return; @@ -1685,7 +1687,10 @@ function CloudAppContent({ client, resolver }: CloudAppProps) { if (mobileWebApp) setDrawerOpen(false); setDetails({ workspaceId, tab: 'members', focusAddMember: true }); }} - onCreateWorkspace={() => setShowCreateWorkspace(true)} + onCreateWorkspace={() => { + if (mobileWebApp) setDrawerOpen(false); + setShowCreateWorkspace(true); + }} onOpenSettings={() => navigateToSettings('profile')} onSelectSession={selectTtydSession} onCloseSession={closeTtydSession} diff --git a/packages/webapp/src/WorkspaceMembersEditor.tsx b/packages/webapp/src/WorkspaceMembersEditor.tsx index 816b8d25..706650e8 100644 --- a/packages/webapp/src/WorkspaceMembersEditor.tsx +++ b/packages/webapp/src/WorkspaceMembersEditor.tsx @@ -248,47 +248,49 @@ function MemberRow({ }} /> )} - {showMachine && } - {showMachine && ( - - )} - {showVolume && (machine === null ? ( - - + {showMachine && } + {showMachine && ( + onPersistentVolumeChange(event.currentTarget.checked)} + onChange={onMachineTypeChange} /> - Persistent volume - - ) : ( - // The disk exists, so the row reports it instead of offering a choice - // this route cannot make: how full it is, or that there is none. - - ))} - {showMachine && actions.length > 0 && ( - ({ value: action, label: MACHINE_ACTION_LABELS[action] }))} - onChange={(next) => { - // SAFETY: the options are exactly the MachineAction values above. - onMachineAction?.(next as MachineAction); - }} - /> - )} + )} + {showVolume && (machine === null ? ( + + onPersistentVolumeChange(event.currentTarget.checked)} + /> + Persistent volume + + ) : ( + // The disk exists, so the row reports it instead of offering a choice + // this route cannot make: how full it is, or that there is none. + + ))} + {showMachine && actions.length > 0 && ( + ({ value: action, label: MACHINE_ACTION_LABELS[action] }))} + onChange={(next) => { + // SAFETY: the options are exactly the MachineAction values above. + onMachineAction?.(next as MachineAction); + }} + /> + )} + {!readOnly && !pinned && ( - - - {showRail && ( - - )} - - + - > + {showRail && ( + + )} + ); } diff --git a/packages/webapp/src/shell/mobile-rail.ts b/packages/webapp/src/shell/mobile-rail.ts new file mode 100644 index 00000000..fe39f3ab --- /dev/null +++ b/packages/webapp/src/shell/mobile-rail.ts @@ -0,0 +1,7 @@ +import type { AppRoute } from '../sessions-page-state'; + +/** On a phone the rail is the workspace's screen, so picking a session must + * read as a page change. */ +export function routeShowsMobileRail(route: AppRoute): boolean { + return route.page === 'webApp' && route.chat === 'landing'; +} diff --git a/packages/webapp/src/strip-rail.css b/packages/webapp/src/strip-rail.css index d8a07971..21fe3fc0 100644 --- a/packages/webapp/src/strip-rail.css +++ b/packages/webapp/src/strip-rail.css @@ -8,7 +8,6 @@ rail are real children of the shell grid. Below it, it is the drawer. */ .shell-nav { display: contents; } -.shell-nav-scrim, .shell-strip__close { display: none; } /* --------------------------------------------------------------- column 1 */ @@ -543,7 +542,7 @@ top: var(--mobile-viewport-top, 0); left: 0; display: flex; - width: min(88vw, 348px); + width: 100%; height: var(--mobile-viewport-height, 100dvh); background: var(--paper); transform: translateX(-102%); @@ -578,17 +577,6 @@ .shell-new, .shell-s { min-height: 40px; } - - .shell-nav-scrim { - position: fixed; - z-index: 380; - inset: 0; - display: none; - border: 0; - background: color-mix(in oklab, var(--paper) 68%, transparent); - } - - .shell-nav-scrim--open { display: block; } } @media (prefers-reduced-motion: reduce) { diff --git a/packages/webapp/src/workspace-details-dialog.css b/packages/webapp/src/workspace-details-dialog.css index 7daaaf60..eb4a8a3a 100644 --- a/packages/webapp/src/workspace-details-dialog.css +++ b/packages/webapp/src/workspace-details-dialog.css @@ -118,6 +118,7 @@ .workspace-members-empty { margin: 14px 12px; color: var(--faint); font-size: 11px; } .workspace-member-row { display: grid; grid-template-columns: 28px minmax(0, 1fr) auto auto auto auto auto 28px; align-items: center; gap: 10px; min-height: 48px; padding: 0 12px; border-bottom: 0; } +.workspace-member-machine { display: contents; } .workspace-member-row + .workspace-member-row { border-top: 1px solid var(--mod-8); } .workspace-member-name { display: grid; min-width: 0; gap: 2px; } .workspace-member-name strong, @@ -207,9 +208,18 @@ /* Two classes, so this beats the untabbed override further down whatever the source order. */ .workspace-details-dialog.my-machine-dialog { height: auto; max-height: calc(100dvh - 24px); } - /* The row keeps the avatar, the name and the remove control; the selects - * and the chip wrap under them rather than squeeze. */ - .workspace-member-row { grid-template-columns: 28px minmax(0, 1fr) 28px; } + /* The first row keeps the person controls. Machine controls wrap in a band + * below them rather than auto-placing into narrow columns. */ + .workspace-member-row { grid-template-columns: 28px minmax(0, 1fr) auto 28px; } + .workspace-member-machine { + display: flex; + order: 1; + grid-column: 1 / -1; + flex-wrap: wrap; + align-items: center; + gap: 8px; + padding: 0 0 10px 38px; + } } /* The compact machine-type select sits in a member row, so it takes its diff --git a/packages/webapp/test/WorkspaceDetailsDialog.test.tsx b/packages/webapp/test/WorkspaceDetailsDialog.test.tsx index eece959f..ffa0a63e 100644 --- a/packages/webapp/test/WorkspaceDetailsDialog.test.tsx +++ b/packages/webapp/test/WorkspaceDetailsDialog.test.tsx @@ -223,6 +223,12 @@ describe('WorkspaceDetailsDialog', () => { // The machine state chip, and the type select on the member who holds one. expect(view.container.textContent).toContain('running'); expect(view.container.querySelector('[aria-label="Machine type for Ada Owner"]')).not.toBeNull(); + const machineBand = view.container.querySelector('.workspace-member-machine'); + expect(machineBand, 'member machine controls must share one mobile band').not.toBeNull(); + expect(machineBand?.querySelector('.machine-chip')).not.toBeNull(); + expect(machineBand?.querySelector('[aria-label="Machine type for Ada Owner"]')).not.toBeNull(); + expect(machineBand?.querySelector('[role="meter"]')).not.toBeNull(); + expect(machineBand?.querySelector('[aria-label="Machine actions for Ada Owner"]')).not.toBeNull(); // A viewer never holds a machine (§2.2), so no type select and no menu. expect(view.container.querySelector('[aria-label="Machine type for Grace Viewer"]')).toBeNull(); expect(view.container.querySelector('[aria-label="Machine actions for Grace Viewer"]')).toBeNull(); diff --git a/packages/webapp/test/mobile-only-styles.test.ts b/packages/webapp/test/mobile-only-styles.test.ts new file mode 100644 index 00000000..319da305 --- /dev/null +++ b/packages/webapp/test/mobile-only-styles.test.ts @@ -0,0 +1,110 @@ +/** + * These source checks prove the phone fixes do not move desktop layout. Each + * changed declaration must stay inside its max-width query unless its desktop + * value is deliberately boxless. + */ +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const here = dirname(fileURLToPath(import.meta.url)); +const source = (name: string) => readFileSync(join(here, '..', 'src', name), 'utf8'); +const maxWidthPattern = /@media \(max-width: \d+px\) \{([\s\S]*?)\n\}/gu; + +function maxWidthCss(css: string): string { + return [...css.matchAll(maxWidthPattern)].map((block) => block[1] ?? '').join('\n'); +} + +function desktopCss(css: string): string { + return css.replace(maxWidthPattern, ''); +} + +function ruleBodies(css: string, selector: string): string[] { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&'); + const pattern = new RegExp(`(?:^|\\n)\\s*${escaped}\\s*\\{([^{}]*)\\}`, 'gu'); + return [...css.matchAll(pattern)].map((match) => match[1] ?? ''); +} + +function declares(css: string, selector: string, property: string, value: string): boolean { + const declaration = new RegExp( + `(?:^|;)\\s*${property}\\s*:\\s*${value}\\s*(?:;|$)`, + 'u', + ); + return ruleBodies(css, selector).some((body) => declaration.test(body)); +} + +function expectOnlyBelowBreakpoint( + css: string, + file: string, + selector: string, + property: string, + value: string, +): void { + const declaration = `${property}: ${value}`; + expect( + declares(maxWidthCss(css), selector, property, value), + `${file}: ${selector} must declare ${declaration} inside a max-width query`, + ).toBe(true); + expect( + declares(desktopCss(css), selector, property, value), + `${file}: ${selector} must not declare ${declaration} outside a max-width query`, + ).toBe(false); +} + +describe('mobile-only stylesheet changes', () => { + it('keeps the full-width rail and scrim removal mobile-only', () => { + const css = source('strip-rail.css'); + expectOnlyBelowBreakpoint(css, 'strip-rail.css', '.shell-nav', 'width', '100%'); + // The full-width nav covers the viewport, so a scrim would cover nothing. + // It is deleted rather than hidden, and `ShellNav` stops rendering it. + expect( + css.includes('shell-nav-scrim'), + 'strip-rail.css: the scrim must be deleted, not hidden', + ).toBe(false); + }); + + it('keeps the create-workspace scroller and action layout mobile-only', () => { + const css = source('create-workspace-dialog.css'); + expectOnlyBelowBreakpoint( + css, + 'create-workspace-dialog.css', + '.create-workspace-dialog', + 'overflow-y', + 'auto', + ); + expectOnlyBelowBreakpoint( + css, + 'create-workspace-dialog.css', + '.create-workspace-header', + 'position', + 'sticky', + ); + expectOnlyBelowBreakpoint( + css, + 'create-workspace-dialog.css', + '.create-workspace-actions', + 'justify-content', + 'stretch', + ); + expect( + declares(maxWidthCss(css), '.create-workspace-actions--blueprint', 'display', 'flex'), + 'create-workspace-dialog.css: blueprint actions must stay side by side on mobile', + ).toBe(true); + }); + + it('keeps the member machine wrapper boxless on desktop', () => { + const css = source('workspace-details-dialog.css'); + expect( + declares(desktopCss(css), '.workspace-member-machine', 'display', 'contents'), + 'workspace-details-dialog.css: the desktop member machine wrapper must use display: contents', + ).toBe(true); + expectOnlyBelowBreakpoint( + css, + 'workspace-details-dialog.css', + '.workspace-member-machine', + 'display', + 'flex', + ); + }); +}); diff --git a/packages/webapp/test/mobile-rail-route.test.ts b/packages/webapp/test/mobile-rail-route.test.ts new file mode 100644 index 00000000..d3b9f89a --- /dev/null +++ b/packages/webapp/test/mobile-rail-route.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { routeShowsMobileRail } from '../src/shell/mobile-rail.js'; +import type { AppRoute, ChatAddress } from '../src/sessions-page-state.js'; + +const workspaceRoute = (chat: ChatAddress): AppRoute => ({ + workspaceId: 'workspace-one', + page: 'webApp', + chat, +}); + +describe('routeShowsMobileRail', () => { + it('shows the rail only for the workspace landing', () => { + expect(routeShowsMobileRail(workspaceRoute('landing'))).toBe(true); + }); + + it.each([ + ['pane workspace', workspaceRoute(null)], + ['archive', workspaceRoute('archive')], + ['session', workspaceRoute({ sessionId: 'session-one' })], + ['shared session', workspaceRoute({ sessionId: 'session-two', sharedFrom: 'member-two' })], + ['landing terminal', workspaceRoute({ terminalId: 'terminal-one' })], + ['session terminal', workspaceRoute({ sessionId: 'session-one', terminalId: 'terminal-two' })], + ['home', { workspaceId: null, page: 'home' }], + ['settings', { workspaceId: null, page: 'settings', settingsSection: 'profile' }], + ] satisfies ReadonlyArray)('hides the rail for %s', (_name, route) => { + expect(routeShowsMobileRail(route)).toBe(false); + }); +}); diff --git a/packages/webapp/test/shell-mobile-drawer.test.tsx b/packages/webapp/test/shell-mobile-drawer.test.tsx index 0b44b7a6..11d61daa 100644 --- a/packages/webapp/test/shell-mobile-drawer.test.tsx +++ b/packages/webapp/test/shell-mobile-drawer.test.tsx @@ -6,8 +6,8 @@ * the rail's list with a portal host, and the risk that buys is structural: a * host handed over by ref could plausibly be created outside the drawer, or the * drawer could stop opening because the rail no longer renders rows. So this - * asserts the whole path — open, host present INSIDE the drawer, scrim, close — - * at mobile width, in both rail shapes. + * asserts the whole path — open, host present INSIDE the drawer, close — at + * mobile width, in both rail shapes. * * The breakpoint is read out of `strip-rail.css` rather than repeated here: a * test that hard-codes 899 keeps passing after somebody moves the media query. @@ -73,7 +73,7 @@ function nav(overrides: Partial[0]> = {}) { } describe("the mobile navigation drawer", () => { - it("opens, scrims and closes with the vendored zone inside it", async () => { + it("opens and closes with the vendored zone inside it", async () => { const breakpoint = drawerBreakpoint(); // jsdom does not lay out, so the width is asserted rather than measured: // what this test proves is the DOM and the callbacks, and the CSS above is @@ -96,21 +96,22 @@ describe("the mobile navigation drawer", () => { expect(drawer?.contains(seen.host)).toBe(true); expect(seen.host?.className).toContain("session-list--vendor"); - const scrim = view.container.querySelector(".shell-nav-scrim"); - expect(scrim?.className).toContain("shell-nav-scrim--open"); - await act(async () => scrim?.click()); + // The nav fills the screen, so there is no scrim behind it. The strip's own + // close button is what dismisses it. + expect(view.container.querySelector(".shell-nav-scrim")).toBeNull(); + const close = drawer?.querySelector( + 'button[aria-label="Close workspace navigation"]', + ); + await act(async () => close?.click()); expect(onCloseDrawer).toHaveBeenCalledTimes(1); await view.unmount(); }); - it("closes the drawer and drops the scrim with the flag off", async () => { + it("keeps the drawer closed with the flag off", async () => { window.innerWidth = drawerBreakpoint() - 99; const view = await render(nav({ drawerOpen: false })); expect(view.container.querySelector(".shell-nav")?.className).not.toContain("shell-nav--open"); - expect(view.container.querySelector(".shell-nav-scrim")?.className).not.toContain( - "shell-nav-scrim--open", - ); // Flag off: the rail is still the native list, drawer or not. expect(view.container.querySelector(".session-list .shell-s")).not.toBeNull(); expect(view.container.querySelector(".session-list--vendor")).toBeNull(); diff --git a/packages/webapp/test/shell-smoke.test.tsx b/packages/webapp/test/shell-smoke.test.tsx index 9be9fe84..e85dd389 100644 --- a/packages/webapp/test/shell-smoke.test.tsx +++ b/packages/webapp/test/shell-smoke.test.tsx @@ -1314,6 +1314,59 @@ describe("webapp shell smoke", () => { await view.unmount(); }); + it("opens mobile workspace navigation on the workspace landing", async () => { + window.history.replaceState({}, "", "/workspaces/workspace-running/chat"); + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: () => ({ + matches: true, + addEventListener: () => undefined, + removeEventListener: () => undefined, + }), + }); + const view = await render( + , + ); + await settle(); + await settle(); + + expect(navigationExpanded(view.container)).toBe("true"); + await view.unmount(); + }); + + it("closes mobile workspace navigation before opening create workspace", async () => { + window.history.replaceState({}, "", "/workspaces/workspace-running/chat"); + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: () => ({ + matches: true, + addEventListener: () => undefined, + removeEventListener: () => undefined, + }), + }); + const view = await render( + , + ); + await settle(); + await settle(); + expect(navigationExpanded(view.container)).toBe("true"); + + await click(view.container.querySelector( + 'button[aria-label="Create workspace"]', + )); + await settle(); + + expect(navigationExpanded(view.container)).toBe("false"); + expect(view.container.querySelector('form[aria-label="Create workspace"]')).not.toBeNull(); + await view.unmount(); + }); + it("resizes the side pane by dragging its handle, no narrower than the default", async () => { window.history.replaceState({}, "", "/workspaces/workspace-running"); saveTabs("workspace-running", [