Skip to content

feat(mobile-nav): add Files as a 7th mobile tab, active-only tab labels#42

Merged
miguelrisero merged 15 commits into
mainfrom
chief/bc-mobile-files
Jul 21, 2026
Merged

feat(mobile-nav): add Files as a 7th mobile tab, active-only tab labels#42
miguelrisero merged 15 commits into
mainfrom
chief/bc-mobile-files

Conversation

@miguelrisero

@miguelrisero miguelrisero commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

Files is completely unreachable on mobile today — the Files panel only
exists in the desktop right sidebar. Owner's report: "apparently on the
phone I cannot use files, and the other new stuff we added, wouldnt fit
also on top so we will need to find a new solution for it. but need to be
able to access files."
Owner's binding decision when asked how to make
room in an already-full 6-tab mobile strip: "just the selected tab should
have text, not the rest. and it should be a new tab."

This PR:

  1. Adds Files as a 7th mobile tab, appended last (after Git).
  2. Changes the tab-strip label rule: only the active tab shows its text
    label now (inactive tabs are icon-only at every width — replaces a prior
    min-[480px]:inline breakpoint rule). This is what buys the horizontal
    room for a 7th tab.
  3. Reuses the existing WorkspaceFilesContainer unmodified, mounted the
    same "always mounted, hidden via CSS" way as the other 6 mobile panes.
  4. Excludes files from remote-web's mobile strip (Files is local-only —
    remote hosts have no local filesystem to browse).

Tab placement rationale: desktop's RightSidebar.tsx already groups
Files alongside Git/Notes/Terminal as sibling collapsible sections — real
structural precedent for adjacency to Git. Appending at the end also leaves
all 6 existing tabs' positions completely undisturbed.

A real bug found and fixed during live e2e

The first pass implemented the label rule as
cn('hidden', isActive && 'inline'). cn (packages/ui/src/lib/cn.ts) is
intentionally bare clsx — no tailwind-merge, pending a Tailwind v4
migration (mirrors an existing, deliberate, documented decision in
web-core's own cn). Combining conflicting classes through it doesn't
dedupe — the call above produced the literal class string "hidden inline", and the cascade resolved in .hidden's favor. No tab, active
or not, ever showed a label.
This directly broke the owner's core
requirement and was invisible in code review — it reads as correct at a
glance. Caught live via getComputedStyle in a real browser at 390px,
fixed with a plain mutually-exclusive ternary
(isActive ? 'inline' : 'hidden'), re-verified live (exactly 1 of 7 labels
visible at any time, always the active one). Added a warning comment to
cn.ts so this class of bug doesn't recur at the next call site.

Live e2e measurements (390×844, iPhone 14, CDP touch+mobile emulation) — numbers, not adjectives

(a) Does the strip fit at 390px? Yes, in every state tested, no scrolling ever needed:

Active tab Strip scrollWidth = clientWidth Fits?
Chat 247px yes
Preview (longest label, worst case) 265px yes
Files (last position) 248px yes

Viewport is 390px — even the worst case leaves ~125px of headroom for the
right-side icon cluster. scrollLeft stayed 0 throughout; scroll-into-view
code was evaluated and not added, because it's provably not needed —
tested both extremes (longest label, and the new tab in last position) and
neither ever overflows.

(b) Label-driven layout shift — flagging prominently, not burying it.
Because only the active tab carries a label now, switching tabs changes the
strip's total content width, and tabs positioned after the tab whose label
appeared/disappeared reflow. Measured example: the Git tab's on-screen
position shifts ~46.6px depending on whether Preview or Files is the
active tab. This happens instantly — the tab button only has
transition-colors, no width/layout transition, so there's a discrete
snap, not an animation. This is a mathematically necessary consequence
of the approved design (reserving max-label-width on every tab to eliminate
the shift would defeat the point of "only the selected tab has text"), not
a bug — not fixing it here since animating it wasn't asked for and would be
scope creep on a frozen contract. Flagging the exact number so whoever
reviews this on a real phone knows precisely what trade-off was bought.

(c) Mounted-hidden invariant — verified at the DOM level, not just
visually: a precise query against the live page found exactly 7 sibling
pane <div>s inside WorkspacesLayout.tsx's mobile branch at all times,
exactly one lacking the hidden class at any given moment, Files correctly
last (index 6).

Live e2e against production, read-only (parts requiring real workspace data)

No backend is buildable in this sandbox (a private git-dependency fetch
needs SSH credentials that aren't provisioned here — pre-existing,
unrelated to this diff, Cargo.lock/Cargo.toml untouched by any commit
in this PR). To test the Files panel against real data without a local
backend, I pointed my dev server's API proxy at the production backend
(127.0.0.1:4111) and picked the most defensibly idle workspace available
(updated_at several hours stale vs. everything else on the box being
<30min old, since this box runs several concurrently-live sessions).

Methodology note for the next lane that touches production read-only —
this is the important part:
intending read-only isn't the same as
being read-only. Before any app code ran, I added two safety nets: a
hard block on any non-GET request to /api/*, and a block on the
terminal/CLI WebSocket specifically. Both fired for real, not
hypothetically.
The app auto-sent PUT /api/workspaces/{id}/seen merely
on landing on the workspace page (a "mark as seen" side effect), and
separately auto-attempted a live terminal WebSocket connect in CLI mode —
which, per prior work on this box, would have attached a client to (and
resized/repainted) a real session if allowed through. A plain read-only-GET
intention, proxied naively, would have written to production. Enforce it
at the fetch layer, not by careful clicking.

Full disclosure of the resulting footprint — every request made, nothing
hidden: 32 requests total, 100% GET (one 401 on an unauthenticated
/api/auth/token check, still a GET, zero writes reached production).
Endpoints touched: workspace detail, git/status (polled), sessions,
repos, loop-automation config, and files/list at the workspace root
and at path=bg-vertexai-adapter.

Result: the Files tab rendered a real directory listing that matched a
pre-check curl byte-for-byte (CLAUDE.md, 53 B, plus 2 folders).
Navigated into a subfolder (real nested listing, 9 folders + files with
real sizes). Switched to the Diff tab and back to Files — still inside
the subfolder
, not reset to root, proving the mounted-hidden pattern
preserves real component state, not just DOM structure. Confirmed the
download link is wired to a real GET endpoint without completing an actual
download (unnecessary footprint beyond what proves the capability). Upload
was never touched (the one write-capable endpoint in this API).

Remote-web "no Files tab" check: could not live-render past remote-web's
real auth gate (no credentials, didn't attempt to bypass it — out of
scope). This one check is code-level verified, not a live browser
render
: ran the literal predicate from the committed source
(packages/remote-web/src/app/layout/RemoteNavbarContainer.tsx) against
the literal MOBILE_TABS data — result is exactly [chat, changes, logs, git], files absent. Flagging this distinction explicitly rather than
letting a mock-backed/code-level check read as if it were live — the label
bug above is exactly the class of defect that reading code can miss; this
particular check (a static array .filter(), zero CSS/runtime dependency)
is a different risk class where code-level verification is proportionate.

Gauntlet review (council + /simplify + /code-review xhigh + lane Codex sweep) — one more real bug found and fixed

Council (Codex ux-advocate + Kimi maintainability-architect, cross-checked
by a Claude ux-advocate seat) plus 4 parallel /simplify agents plus 5
parallel /code-review agents ran against the diff. Verdict: CAUTION, with
one real, well-evidenced bug — fixed before this PR left draft.

The bug — cross-engine consensus (Codex and Kimi independently found the
identical issue via different reasoning paths):
packages/local-web is
not purely local — it has its own /hosts/$hostId/workspaces_/$workspaceId
route (the "relay" feature: browsing a remote host's workspaces from the
local app). NavbarContainer.tsx (local-web's navbar) never filtered
MOBILE_TABS, so on that specific route the Files tab button rendered and
was tappable, while WorkspacesLayout.tsx's pane content correctly gated
on !hostId — tapping it showed a permanently empty pane. I'd verified the
remote-web case (a separate app/route) but missed that local-web has this
same host-scoped state internally. Independently confirmed desktop's
RightSidebar.tsx handles the equivalent case correctly — its section list
is {sections.filter((s) => s.visible).map(...)}, so the whole Files
section (header included) is excluded from rendering, not merely
collapsed; mobile's gap was real and asymmetric with desktop.

Fix: NavbarContainer.tsx now calls useHostId() and filters files
out of the tabs it passes to <Navbar> whenever hostId is truthy,
mirroring RemoteNavbarContainer.tsx's existing pattern exactly. Live
re-verified after the fix, both states:
on a host-scoped route
(/hosts/:hostId/workspaces/:id), the tab strip is exactly [Wksps, Chat, Diff, Logs, Preview, Git] (files absent); on a plain local route, it's
still the full 7 tabs including files. Not a mocked assertion — measured
via a DOM query against a live-rendered page in both states.

Also fixed, all found by the gauntlet, all cheap and verified:

  • A mechanically-confirmed compliance violation: 3 of the lockstep
    cross-referencing comments (in Navbar.tsx and useUiPreferencesStore.ts)
    exceeded the repo's 80-column style guideline — invisible to
    Prettier/ESLint (neither wraps comment prose) — now rewrapped.
  • aria-current="page" added to the active mobile tab button (selection
    was previously conveyed only visually).
  • The workspaces tab's aria-label is now "Wksps (Workspaces)" instead
    of bare "Wksps" — still satisfies WCAG 2.5.3 (contains the visible
    text verbatim) but is comprehensible when read aloud, unlike "Wksps" on
    its own. Scoped to just that one tab; the other 6 labels are already
    ordinary words.
  • A lockstep comment on WorkspacesLayout.tsx's 7-pane mobile registry,
    flagging that it's a third, compiler-invisible copy of the tab list — a
    future 8th tab added to the two unions without a matching pane here
    would compile clean but silently produce a dead button.

Considered and explicitly not changed, with reasoning (not silent
skips):

  • Two reviewers suggested changing the workspaces tab's aria-label to
    the full word "Workspaces" alone — rejected: the visible text is
    "Wksps", and WCAG 2.5.3 requires the accessible name to contain the
    visible text, so an aria-label of "Workspaces" alone would create a
    new compliance problem, not fix one. Went with the parenthetical form
    instead (above).
  • The 480-767px width band loses full labels it used to have. Under
    the old min-[480px]:inline rule, phones/small tablets in that range
    showed labels for all 6 tabs regardless of which was active — a
    strictly better experience at that width than what ships now (icon-only
    except the active tab, at every width). This is a genuine, disclosed
    trade-off of the owner's own design decision, not a bug: the contract
    explicitly specifies "icon-only at ALL widths," which was the intended
    mechanism for buying room for a 7th tab at the narrowest widths. Flagging
    it here, alongside the ~46.6px reflow number above, as a second concrete
    number for whoever reviews this trade-off — if it's not what was
    intended at wider widths, a width-conditional variant (icon-only-except-
    active only below ~480px, full labels above) is a small, isolated
    follow-up commit, not a redesign.
  • Touch-target sizing (icon + px-1.5 py-1 padding, ~29.5×25.5 CSS px) is
    below the 44×44 HIG/Material comfort zone, though it clears WCAG 2.2's
    24×24 AA floor. Pre-existing (unmodified by any commit in this branch)
    and applies uniformly to all 7 tabs — out of scope for this feature PR.
  • MobileTabId (packages/ui) and MobileTab (packages/web-core) could
    in principle be unified into a single declaration — useUiPreferencesStore.ts
    already imports a type from @vibe/ui elsewhere (RepoAction from
    @vibe/ui/components/RepoCard), proving the dependency direction allows
    it. Both council personas independently called the current
    duplication-plus-lockstep-comments form "acceptable for this release."
    Noting as a real, precedented follow-up opportunity rather than expanding
    this PR's scope further.
  • WorkspaceFilesPanel's per-file download affordance is hover-only (no
    touch equivalent) — flagged by a reviewer as informational, explicitly
    out of scope (upload/download UI is unmodified in this PR).

Design decisions

  • Two independently-declared unions: MobileTabId (packages/ui) and
    MobileTab (packages/web-core's store) are two separate 7-member
    string-literal unions kept in lockstep by convention plus
    cross-referencing comments, not a shared type — packages/ui cannot
    depend on packages/web-core (dependency flows the other way).
  • Cast removal over runtime guards: 4 as MobileTab/as MobileTabId
    casts were removed in favor of plain (uncast) assignments across
    useUiPreferencesStore.ts (×2, both governing how RightMainPanelMode
    syncs into mobileActiveTab on mobile — confirmed both share the exact
    same RightMainPanelMode type, not two divergent ones), plus
    NavbarContainer.tsx and RemoteNavbarContainer.tsx (the web-core/ui
    package-boundary casts). All four compile cleanly as plain assignments
    today because the relevant types are currently proper subsets/equal sets
    — which means a future mismatch (someone extends one union without the
    other) now fails tsc instead of an as cast silently producing an
    invalid tab id at runtime.
  • !hostId gating: the new Files pane in WorkspacesLayout.tsx gates
    on selectedWorkspace?.id && !hostId, mirroring RightSidebar.tsx's
    pre-existing local-only rule rather than inventing a new condition.
  • Accessibility: aria-label={tab.label} added to every mobile tab
    button, since the visual label is now conditionally hidden for inactive
    tabs at every width (previously only hidden below 480px).
  • Icon: FolderSimpleIcon (aliased FolderIcon), already used
    elsewhere in this repo (CreateRepoDialog.tsx) — consistent, precedented
    choice.

Checks

  • pnpm run ui:check / web-core:check / local-web:check /
    remote-web:check — all pass.
  • pnpm run local-web:lint / ui:lint (max-warnings 0) — pass.
    remote-web has no lint script of its own (pre-existing, unrelated to
    this diff).
  • i18n unused-key checker — pass (no i18n keys touched; tab labels are
    plain hardcoded strings, matching the existing 6 tabs).
  • pnpm run format — clean, no diff.
  • vitest via npx for packages/web-core (frontend tests don't run in CI
    on this repo — tsconfig excludes *.test.ts, flagging this caveat per
    repo convention): 208/208 pass across 18 files.
  • pnpm run backend:checknot runnable in this sandbox: a private
    git-dependency fetch needs SSH credentials not provisioned here
    (pre-existing environment limitation, unrelated to this diff — zero
    crates/ files touched, Cargo.lock/Cargo.toml untouched). CI's own
    paths-filter already skips the backend job for frontend-only diffs, so
    this is non-blocking.
  • CodeRabbit CLI: confirmed unusable non-interactively on this fork —
    recorded as a deviation from the standard gauntlet, not attempted.

Out of scope (per contract)

Desktop right-sidebar layout, the other 6 tabs' behavior/order, any
upload/download feature changes inside WorkspaceFilesPanel.tsx itself
(reused completely unmodified), anything in crates/.

Files changed

  • packages/ui/src/components/Navbar.tsxfiles tab, active-only label rule, aria-label
  • packages/ui/src/lib/cn.ts — warning comment (no behavior change)
  • packages/web-core/src/shared/stores/useUiPreferencesStore.tsfiles in MobileTab, cast removal ×2
  • packages/web-core/src/pages/workspaces/WorkspacesLayout.tsx — Files pane, mounted-hidden
  • packages/web-core/src/shared/components/ui-new/containers/NavbarContainer.tsx — cast removal
  • packages/remote-web/src/app/layout/RemoteNavbarContainer.tsx — exclude files, cast removal

Draft PR — boss/owner flips to ready.

Append Files after Git to preserve the existing six tab positions and mirror the desktop right-sidebar grouping. Show text only for the selected tab while retaining accessible names for icon-only tabs.
Keep the web-core tab union aligned with the UI tab ids. Assign RightMainPanelMode directly so TypeScript rejects future modes that are not also valid mobile tabs.
Mount the local workspace files container as the seventh mobile pane after Git. Keep it mounted across tab switches and mirror the existing local-only host guard.
Keep the shared Files tab out of remote navigation because workspace filesystem browsing remains local-only.
…applied)

cn is a plain clsx wrapper, so cn("hidden", isActive && "inline") emitted both conflicting Tailwind utilities together. In this build, .hidden won the stylesheet cascade and kept every tab icon-only regardless of active state.
The top-level pnpm run format chain skipped packages/ui, which let Navbar.tsx's formatting issue survive every local format run on this branch. CI caught it because packages/ui's own format:check runs independently.
@miguelrisero
miguelrisero marked this pull request as ready for review July 21, 2026 22:36
@miguelrisero

Copy link
Copy Markdown
Owner Author

Reviewer handoff + merge record (chief run bc-lanes-2026-07-19)

What it does: adds Files as a 7th mobile tab so the file browser is reachable on phones (it was desktop-sidebar-only before). To make room, the tab strip now shows a text label only on the active tab — inactive tabs are icon-only at every width. Files is excluded on remote-host views (the panel is local-only) and on local-web's own host-scoped routes.

Measured, live at 390px: strip width 247–265px across all states — never scrolls. Label-driven reflow when switching tabs is ~47px (disclosed, not a defect — it's the necessary consequence of the owner-chosen 'only active tab has a label' rule).

Three real bugs found by live testing, not code review:

  1. The label rule used cn('hidden', isActive && 'inline') — but this repo's cn is bare clsx with no conflict dedup, so every tab computed to display:none and no label ever showed. Caught via getComputedStyle in a real browser. Fixed with a mutually-exclusive ternary; documented the footgun in cn.ts.
  2. Cross-engine council consensus caught a dead Files tab on local-web's remote-host routes (rendered but opened an empty pane). Fixed to mirror the desktop sidebar's exclusion, re-verified live in both states.
  3. CI surfaced that packages/ui was never wired into the top-level pnpm run format — a repo-wide gap that predates this PR. Fixed at the source.

Methodology note for the next lane touching production: read-only intent is not read-only behavior. Testing the file browser against the live instance, the app auto-fired a PUT .../seen and auto-attempted a terminal WebSocket connect (which would have touched a live tmux session) — both caught by fetch-layer + WS blocks. Enforce read-only at the fetch layer, not by intention.

Boss verification passed (all CI green, files in-lane, identity/trailers clean) — merging under the standing merge-when-ready authorization.

@miguelrisero
miguelrisero merged commit 8f153c7 into main Jul 21, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant