Skip to content

🤖 feat: match Review 1.4 layouts and palette (footer bar, title header, creation hero, composer)#3753

Open
ibetitsmike wants to merge 21 commits into
mainfrom
claude/mux-review-1-4-layouts-pzi9na
Open

🤖 feat: match Review 1.4 layouts and palette (footer bar, title header, creation hero, composer)#3753
ibetitsmike wants to merge 21 commits into
mainfrom
claude/mux-review-1-4-layouts-pzi9na

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Matches the Review 1.4 page of the Mux exploration Figma in both layout and palette: a workspace footer info bar, a workspace-title header, a creation hero, a restyled composer row, and the design's flat surface treatment, plus a design-reference RFC with exported Figma screenshots so the target design is reviewable alongside the code.

Background

Review 1.4 is the latest iteration of the Mux UI exploration (design: Tracy Johnson). Much of its sidebar/right-panel language already exists in the app (the Figma is Code Connect-mapped to AgentListItem), so this PR targets the genuinely new layout structures: the workspace footer info bar, the workspace-title header, the creation hero, the composer row, and the flat surface palette. The target frames are checked in under rfc/assets/review-1.4-layouts/ (start-workspace-layout.png, start-v2-creation.png, chat-column.png, footer-info-bar.png) and indexed with Figma deep links in rfc/20260725_review-1.4-layouts.md.

Implementation

  • WorkspaceFooterBar (new, bottom of the chat pane): matches the design's item sequence of PR link, git drift, repository, branch, token usage, and last prompt, plus a runtime badge. Repository identity shows the GitHub slug (coder/mux) and falls back to the project hierarchy label until PR detection resolves a remote. The design's Lines dropdown labels the drift counts and toggles drift mode through the same persisted key as the divergence dialog, so both surfaces stay in sync. Last prompt reveals the latest real user prompt via tooltip, backed by a new useWorkspaceLastUserPrompt selector that skips synthetic turns. All items are leaf store subscriptions so per-delta streaming stats never cascade through the transcript.
  • WorkspaceMenuBar: header now reads "Workspace title ⓘ". Project label, workspace name, and path move into the info-icon tooltip; repository controls and the PR badge move to the footer. The header keeps identity + actions only, per the design.
  • ProjectPage: "Let's get building." hero headline above the creation card (design's "start V2" state).
  • Composer row (ChatInput, shared by the chat and creation surfaces): the agent pill leads
    the row, model and thinking level share one bordered pill split by a divider, the context meter
    becomes a labelled pill with its percentage always visible, and send is a filled circle.
  • Palette: sampling the frames pixel by pixel showed every surface is one fill (#09090b)
    separated by hairline borders, and that our dark theme already holds those exact values under
    different names. So the fix is mostly token swaps, recorded as a mapping table in the RFC:
    • The prompt input had its own #1D1D1D fill and border. It is now transparent inside a single
      bordered box that also holds the pill row, so there is no nested box, and that box carries the
      amber editing tint that used to sit on the textarea. Since the textarea also loses its focus
      border, the box shows that agent-colored focus state instead, keyed off textarea focus so the
      pill row's own focus rings do not light up the whole box.
    • Pill borders drop from border-medium (#454545) to the same hairline as the box, and the
      context meter track moves from #1D1D1D to the sampled #3f3f46.
    • Send fills with the design's cyan when a message can be sent and sits on #18181b when it
      cannot. The cyan is a new per-theme --color-composer-send: the design's accent is not
      --color-accent's blue, and --color-border-pending (the same cyan today) means "needs
      attention".
    • The chrome (left sidebar, both header bars, footer bar) painted itself #171717, so it read as
      a lighter band around a correctly-colored transcript. --color-sidebar now resolves to
      --color-surface-primary in the dark theme; the light and Flexoki themes keep their own
      sidebar fill, since a lighter sidebar is deliberate there and the frames only specify dark.
    • The Electron titlebar overlay and the theme-color meta tag cannot read CSS variables, so the
      palette is duplicated in src/common/constants/chromeColors.ts (previously three hardcoded
      copies: main process, ThemeContext, and index.html's pre-JS boot script). It mirrors each
      theme's --color-sidebar, the fill both surfaces actually sit against, and a test compares it
      against the boot copy, the static meta tag, and public/manifest.json (the installed PWA's
      splash and OS chrome, which still held the pre-parity #1e1e1e) so none of them can drift.
  • Narrow-width fixes found by comparing live screenshots against the design: the bar's
    horizontal scrollbar was consuming 11px of its 28px height (now scrollbar-none), and footer
    items were shrinking instead of being clipped, which collapsed the repo slug to zero width while
    its icon stayed (items are now shrink-0, and the project label caps its width instead). The
    composer's new bordered box also costs 16px of inner width, which pushed the pill row's 340px
    clamp past what a 375px viewport leaves, so the clamp (and its coupled container query) drops to
    320px.
  • Story fix: App.commandPalette play tests were failing on this branch before the parity pass.
    The header now renders the workspace title that the sidebar row also shows, so findByText
    matched two elements; the helper waits on all matches instead.
  • rfc/20260725_review-1.4-layouts.md + rfc/assets/review-1.4-layouts/ (25 PNGs, ~960 KB): per-area design reference covering top-level layouts, sidebar states, menus and selectors, chat column + footer, and right panel, each with screenshots, Figma deep links, designer intent notes, a design-to-code mapping table, and tracked not-yet-implemented deviations (Cost tab naming, Explorer tab/Panels button, sidebar Refine, footer configurability, sentence-style creation configurator, header action set, drift glyph style).

Validation

  • Scratch workspaces keep hiding repository controls; that gating coverage moved from WorkspaceMenuBar.test.tsx into the new WorkspaceFooterBar.test.tsx (scratch / repo-backed / multi-project cases).
  • The e2e navigation helper now asserts the project name in the footer bar, where it lives after the move.
  • tests/ui/chat failures are pre-existing on main in the dev container (happy-dom color parsing), verified via stash.
  • Every footer item was verified in a dev-server-sandbox instance against the checked-in Figma
    exports, at both desktop (1512px) and narrow (375px) widths, including a repo-backed workspace
    with a live PR so the slug, drift, and PR badge all render real data.
  • Palette parity was checked numerically, not by eye: each surface's sampled Figma value was
    compared against the rendered getComputedStyle value in the dark theme, and the chrome, page,
    transcript, and composer interior all report rgb(9, 9, 11). Verified at desktop and phone
    widths, in the editing state, and in the light theme. The chrome palette was checked the same way
    in all four themes: --color-sidebar renders rgb(9, 9, 11), rgb(245, 245, 245),
    rgb(242, 240, 229), rgb(28, 27, 26), matching CHROME_COLORS entry for entry.
  • Composer widths were measured, not eyeballed: at a 375px viewport the pill row is 325px wide and
    its right edge lands exactly on the box's content edge (send included) with no page overflow, and
    390px is unchanged.
  • A new FocusedComposer story asserts the composer's rendered border color changes when the
    textarea takes focus, so the box's focus state cannot silently disappear again (and the arbitrary
    has-[] variant is proven to compile). It has no Pixel snapshot; the assertion is on computed
    style, not appearance.
  • WorkspaceFooterBar.test.tsx covers the repo-slug/project-label fallback branches and
    last-prompt gating alongside the existing scratch / repo-backed / multi-project cases.
  • ProjectPage.autofocus.test.tsx fails on the base branch too (hasConfiguredProvider export
    missing under bun module mocking), verified via stash.

Risks

  • Repository controls (branch selector, drift indicator) and the PR badge changed home from header to footer. That is a muscle-memory change, and anything selecting them inside workspace-menu-bar (tests, tutorials) needs the footer. In-repo references were updated; external automation selecting by test id may need the same.
  • ProjectPage/WorkspaceMenuBar visual snapshots will shift, as expected. The palette change
    makes that much wider: the composer row renders on every screen and the chrome change touches
    every bg-sidebar consumer, so essentially all dark-theme snapshots move. The full Storybook
    suite is at its pre-existing baseline (the same 5 Task/CodeExecution smoke tests fail with
    these commits stashed, a known local circular-import issue that CI's built Storybook passes).
  • Three bg-sidebar consumers are floating surfaces rather than chrome (the workspace drag ghost,
    the skill count badge, the SQL samples dropdown). They would have lost contrast against a
    page-colored fill, so they move to bg-dark, the fill the repo's other popovers already use.
  • The Lines control writes the shared gitStatusIndicatorMode key, so toggling it in the footer
    also changes the sidebar and divergence dialog. That is intentional (one drift mode per user),
    but it is a wider blast radius than a footer-local control.
  • The repository item shows the real GitHub slug only once PR detection resolves one, and falls
    back to the project label otherwise. Reading the slug straight from the git remote needs a new
    backend accessor plus a cached store, so it is tracked as a deviation rather than done here.

Initial layout implementation generated with Claude Code; footer,
composer, and palette parity passes done with mux against the checked-in Figma exports.

Generated with mux • Model: anthropic:claude-opus-5 • Thinking: max • Cost: $152.25

claude added 2 commits July 25, 2026 09:42
… hero)

Implements the two structural layout changes from the "Review 1.4" page of
the "Mux exploration" Figma:

- New WorkspaceFooterBar under the chat pane: repository state (branch
  selector, devcontainer chip, git drift indicator), PR link badge, and
  session token usage (+ live t/s while streaming) live in a persistent
  bottom status bar. Repository controls move out of WorkspaceMenuBar so
  the header keeps identity (runtime, project, title) and actions only.
- "Let's get building." creation hero headline on ProjectPage, matching the
  design's "start V2" empty/creation state.

Scratch workspaces keep hiding repository controls; gating coverage moved
from WorkspaceMenuBar.test.tsx to WorkspaceFooterBar.test.tsx.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173Z5TUFg4BqnACfxAKDJ3o
Match the Review 1.4 workspace frame more closely:

- Header now reads "Workspace title ⓘ" — the workspace title is the header's
  identity, with project label, workspace name, and path one hover away on
  the info icon. RuntimeBadge and the project label move out of the header.
- Footer bar gains the runtime badge (leading) and the project hierarchy
  label, matching the design's PR / drift / repo / branch item order.
- e2e navigation check follows the project label into the footer bar.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173Z5TUFg4BqnACfxAKDJ3o

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c711bd136d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx Outdated
Checked-in visual reference for the Review 1.4 page of the "Mux exploration"
Figma: top-level layouts, sidebar/project-pane states, menus & selectors,
chat column + footer info bar, and the right panel — each with exported
screenshots, deep links back to the Figma nodes, designer intent notes, a
design → code mapping table, and the tracked not-yet-implemented deviations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173Z5TUFg4BqnACfxAKDJ3o
@ibetitsmike ibetitsmike changed the title 🤖 feat: implement Review 1.4 layouts (workspace footer bar + creation hero) 🤖 feat: implement Review 1.4 layouts (footer bar, title header, creation hero) + design-reference RFC Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@codex review


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95bf6ca21b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/ProjectPage/ProjectPage.tsx Outdated
Bring the footer bar to item-for-item parity with the design:

- Reorder items to the design sequence: PR, drift, repository, branch,
  tokens, last prompt (drops the flex spacer that right-aligned usage).
- Show the GitHub repo slug ("coder/mux") like the design, falling back to
  the project hierarchy label until PR detection resolves a remote.
- Add the design's "Lines" dropdown labelling the drift counts. It toggles
  the drift mode through the same persisted key as the divergence dialog.
- Label the PR badge "PR #1234" and dim the token unit suffixes.
- Add the "Last prompt" item from the footer note, backed by a new
  useWorkspaceLastUserPrompt selector that skips synthetic turns.

Two layout bugs found while checking narrow widths:

- The horizontal scrollbar consumed 11px of the bar's 28px height. The bar
  now uses scrollbar-none.
- Footer items shrank instead of being clipped, so the repo slug collapsed
  to zero width while its icon stayed. Items are now shrink-0, and the
  project label caps its width rather than collapsing.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Footer parity pass on top of the original layout work: item order now follows the design (PR, drift, repository, branch, tokens, last prompt), the repository item shows the GitHub slug with a project-label fallback, the design's Lines control toggles drift mode through the shared persisted key, and Last prompt is backed by a new useWorkspaceLastUserPrompt selector.

Two narrow-width bugs were found and fixed while diffing live screenshots against the Figma exports: the horizontal scrollbar was eating 11px of the bar's 28px height, and footer items were shrinking instead of being clipped (collapsing the repo slug to zero width while its icon stayed).

Mux performed this parity pass on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e681468707

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx
- Hoist the creation hero out of the provider-dependent branch. It was
  skipped entirely in the no-providers state (exactly the first-run case the
  hero is for) and flashed in then out while provider state loaded.
- Fix the command-palette story play tests, which CI was already failing on
  this branch: the header now renders the workspace title that the sidebar
  row also shows, so findByText matched two elements. Wait on all matches.
- Record the repository-slug limitation in the RFC: the footer shows the real
  slug only once PR detection resolves one, so a GitHub-backed branch with no
  PR still reads as the project label.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

Addressed the three Codex findings on c8e70c29d:

1. Preserve footer height when horizontal scrolling (WorkspaceFooterBar.tsx) — fixed. Confirmed the bug in a live sandbox before fixing: at 375px the bar measured clientHeight 17px against its declared h-7 (28px), so the scrollbar was eating 11px. The bar now carries the repo's scrollbar-none utility and measures a full 27px while still scrolling (scrollWidth 730 vs clientWidth 375). The same pass also found that footer items were shrinking rather than being clipped, which collapsed the repo slug to zero width while its icon stayed, so items are now shrink-0.

2. Keep the creation hero visible during provider setup (ProjectPage.tsx) — fixed. The heading now renders outside the !providersLoading && !hasProviders branch, so it is present during provider setup and no longer shifts when provider state resolves.

3. Derive the repository slug independently of PR detection (WorkspaceFooterBar.tsx) — correct diagnosis, deliberately not fixed here. There is no frontend-reachable git remote accessor today: gh pr view is the only path that currently yields owner/repo. Doing this properly needs a new backend accessor plus a cached store keyed per workspace, which is its own change rather than footer layout work. The behavior is now recorded as a tracked deviation in the RFC so it is not lost. Happy to split it into a follow-up PR if you would prefer it landed with this work.

Also fixed a CI failure that predates this parity pass: App.commandPalette story play tests were failing because this PR's header renders the workspace title that the sidebar row also shows, so findByText matched two elements. The helper now waits on all matches. Verified locally against a running Storybook (3/3 pass).

Mux addressed this review on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

All three prior findings are addressed on c8e70c29d (see the comment above for per-finding detail: footer scrollbar height fixed with scrollbar-none plus shrink-0 items, creation hero hoisted out of the provider branch, repo-slug-without-PR recorded as a tracked RFC deviation since it needs a new backend remote accessor).

Mux requested this re-review on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8e70c29d7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx Outdated
The prompt text was only reachable through a hover tooltip on a plain span,
so touch users (no hover) and keyboard users (no focus target) could not read
it at all. Swap in a Popover with a real button trigger: click, tap, and
Enter/Space all open it now.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

Make Last prompt usable without hover — fixed on 735ffeab9. Correct catch: the prompt text lived only in TooltipContent behind a non-focusable span, so it was unreachable on touch and by keyboard.

Replaced it with a Popover on a real button trigger. Verified in a live sandbox: aria-expanded flips false->true on click, the trigger takes focus (document.activeElement matches, tabIndex >= 0), and Enter opens the popover from the keyboard. The content is also scroll-capped rather than clipped so long prompts stay readable.

Mux addressed this review on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

The Last prompt accessibility finding is fixed on 735ffeab9 (Popover on a real button trigger; click, tap, and Enter all open it, verified in a live sandbox).

Mux requested this re-review on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 735ffeab93

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/stores/WorkspaceStore.ts Outdated
Comment thread src/browser/components/WorkspaceMenuBar/WorkspaceMenuBar.tsx Outdated
- Drop the useCallback wrappers from useWorkspaceLastUserPrompt. The React
  Compiler handles this, and useWorkspaceState right above it already
  subscribes to the same subscribeKey channel with plain inline arrows.
- Make the header's workspace-details trigger a real button. It was a bare
  Info SVG behind a hover tooltip, so the project, workspace name, and path
  it holds were unreachable by touch or keyboard. Same fix as the footer's
  Last prompt item: a Popover on a focusable button.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

Both round-3 findings fixed on a6f28a771:

1. Remove manual memoization from the selector hook (WorkspaceStore.ts) — fixed. You are right, and the local precedent confirms it: useWorkspaceState directly above subscribes to the same subscribeKey channel with plain inline arrows. useWorkspaceLastUserPrompt now matches that shape.

2. Use an activatable trigger for workspace details (WorkspaceMenuBar.tsx) — fixed. Same class of bug as the Last prompt one, and this one mattered more since the tooltip is the only place the project, workspace name, and path appear after they moved out of the header. Now a Popover on a focusable button. Verified in a live sandbox: the trigger takes focus and click opens it showing cmux / sidebar-eff8 / /home/coder/.mux/src/cmux/sidebar-eff8.

Mux addressed this review on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Round-3 findings fixed on a6f28a771: dropped the useCallback wrappers from useWorkspaceLastUserPrompt to match useWorkspaceState, and made the header's workspace-details trigger a focusable button with a Popover.

Mux requested this re-review on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6f28a7710

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx Outdated
Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx Outdated
- Hide the Lines/Commits toggle for multi-project workspaces.
  MultiProjectGitStatusIndicator has no mode concept, so the toggle changed
  nothing on screen while still rewriting the shared preference that
  single-repo workspaces later read.
- Register TOGGLE_DRIFT_MODE (Ctrl/Cmd+Shift+G) so the toggle is reachable
  from the keyboard like every other operation, wired with the same
  window-listener pattern as TOGGLE_NOTIFICATIONS and surfaced in the
  keybinds settings section. The button also advertises it in its tooltip.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

Both round-4 findings fixed on 7b2ffaa78:

1. Hide the ineffective mode toggle for multi-project workspaces — fixed, and the diagnosis was exactly right: MultiProjectGitStatusIndicatorProps is only { workspaceId, tooltipPosition, isWorking }, so there was no way for it to honour the mode. The toggle now renders only on the single-repository path, so it can no longer silently rewrite a preference that another workspace reads. Covered by two new tests (omits the drift mode toggle for multi-project workspaces / shows the drift mode toggle for single-repository workspaces).

2. Register a shortcut for the new drift-mode operation — fixed. Added KEYBINDS.TOGGLE_DRIFT_MODE on Ctrl/Cmd+Shift+G (G for Git; verified free against the existing Ctrl+Shift set), wired with the same window keydown pattern as TOGGLE_NOTIFICATIONS, and added to KEYBIND_LABELS plus the General group so it shows in keybind settings. The button's tooltip advertises the shortcut. Verified live in a sandbox: Ctrl+Shift+G flips the footer from Lines +2.4k -1.1k to Commits ↑6 ↓6 and back.

Mux addressed this review on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Round-4 findings fixed on 7b2ffaa78: the Lines/Commits toggle no longer renders for multi-project workspaces (where the indicator has no mode concept), and TOGGLE_DRIFT_MODE is registered on Ctrl/Cmd+Shift+G with a keybind-settings entry.

Mux requested this re-review on Mike's behalf.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 7b2ffaa788

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The earlier passes left both the new-chat screen and the message composer
looking nothing like the design. This addresses the shared composer row plus
the creation surface.

Composer row, now matching the design's order and shapes:
- Agent mode pill leads the row (who is acting reads before which model).
- Model and thinking level share one bordered pill split by a divider,
  matching the design's "claude opus 4.5 | max".
- The context meter is a bordered pill labelled "Context" with its percentage
  always visible, instead of a bare meter whose percent only showed when narrow.
- Send is a filled circle, so it reads as the row's primary action.

Creation surface:
- Drop the raised-card chrome (shadow-lg, heavy padding) for the design's
  plain bordered surface, so the headline carries the page.
- State runtime and branch as a sentence ("This workspace will use Worktree
  from main") instead of stacked labelled fields. Labels move to sr-only, so
  the controls and their handlers are unchanged.
- Placeholder now names the blank-prompt path, per the design: "Describe your
  goals, or leave blank for a codebase summary."

Deliberately not copied from the design: the "Auto: Plan" mode pill (there is
no auto-selection concept, and AgentModePicker's tests assert its absence) and
the send chevron (PR #2633 removed it on purpose to keep Send single-action,
and tests/ui/chat/sendModeDropdown.test.ts guards that).
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Follow-up on reviewer feedback that the new-chat screen and the message composer did not match the supplied design. Restyled both on 822c1ae52:

Shared composer row now follows the design's order and shapes: agent-mode pill leads, model and thinking level share one bordered pill split by a divider ("Opus 5 | MAX"), the context meter is a bordered "Context … 7%" pill with the percentage always visible (previously the percent only appeared on narrow layouts), and Send is a filled circle.

Creation surface: dropped the raised-card chrome for the design's plain bordered surface, restated runtime/branch as a sentence ("This workspace will use Worktree from main") with labels moved to sr-only, and adopted the design's placeholder ("Describe your goals, or leave blank for a codebase summary."). The controls and their handlers are unchanged, so runtimeConfig/trunkBranch still reach workspace.create.

Two things in the design I deliberately did not copy, both because the repo already made the opposite call on purpose:

  • The Auto: Plan mode pill. There is no auto-selection concept, and AgentModePicker.test.tsx asserts auto affordances are absent, so the pill shows the real mode.
  • The send chevron. PR 🤖 refactor: fold ChatInput help indicators into existing tooltips #2633 removed it to keep Send single-action, and tests/ui/chat/sendModeDropdown.test.ts guards that. I had added one, then reverted it once I found that history.

Validation: make static-check, bun x jest tests/ui/chat (42 pass), and unit tests for the touched components (145 pass; the one ProjectPage.autofocus failure predates this branch). Verified live at 1512px and 390px with no overflow.

Mux made these changes on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 822c1ae52b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/ChatInput/index.tsx Outdated
Moving the mode pill to the start of the composer row left its dropdown
anchored right-0, which pushed the menu off the viewport. Playwright could
still see the rows but never click them, so every E2E spec calling
ui.chat.setMode timed out on linux and macOS.

Verified in a sandbox before and after: the "plan" row now reports
onScreen/clickable with elementFromPoint hitting the row itself.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

Enable the blank creation path before advertising it — fixed on a835d4425, and this was a fair catch on a mistake I made. I copied the design's placeholder text verbatim without checking whether the app supports the path it promises. Confirmed your reading: canSend at index.tsx:1138-1144 requires hasTypedText || hasImages || hasReviews, so blank submit does nothing, and there is no codebase-summary behavior to invoke.

Implementing blank-prompt creation plus a codebase-summary flow is a feature, not part of a restyle, so I removed the promise instead of shipping copy the app cannot honor. The placeholder now reads "Describe your goals to create a workspace..." (scratch: "...to start a scratch chat..."), and the blank-prompt path is recorded in the RFC's tracked deviations.

While there I also recorded the creation-sentence slot mapping as a deviation: the design's "My project will use [select source] with [Select model] with [Select level] thinking." is the zero-projects frame where "source" means picking a repository, whereas the implemented sentence covers the has-project screen and must state workspace type and source branch because workspace.create consumes runtimeConfig and trunkBranch.

Separately, the E2E failures on the previous head were a real regression from moving the mode pill (its dropdown was anchored right-0 and opened off-viewport), fixed in 62cd06d71.

Mux made these changes on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Fixed on a835d4425: the creation placeholder no longer advertises the blank-submit path that canSend blocks, and both that gap and the creation-sentence slot mapping are recorded as tracked RFC deviations.

Mux requested this re-review on Mike's behalf.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: a835d44252

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The earlier passes matched the composer's layout but left its legacy colors in
place, so it did not look like the Review 1.4 frames. Sampling the frames shows
they use the zinc scale our dark theme already carries, so this maps them to
existing tokens rather than new hex values:

- The prompt has no fill or border of its own (design: same color as the page).
  One bordered box now wraps the textarea and the pill row, so there is no
  nested box, and it carries the amber editing tint that was on the textarea.
- Pill borders drop to the composer box's hairline (border-light, #262626 vs
  the sampled #27272a) instead of the heavier border-medium (#454545).
- The context meter track becomes surface-quaternary (#3f3f46), the sampled
  value, instead of bg-dark (#1D1D1D).
- Send fills with the design's cyan once a message can be sent, and sits on
  surface-secondary (#18181b, also sampled) when it cannot. The cyan is a new
  --color-composer-send token per theme: the design's accent is not
  --color-accent's blue, and reusing --color-border-pending would borrow a
  token that means "needs attention".
- The agent pill tints its label and icon and keeps a neutral border, per the
  design, rather than tinting the border.

VimTextArea's isEditing and focusBorderColor props are gone: with no surface of
its own it has nothing to color. The EditingMessage story asserted the tint on
the textarea, so it now asserts it on the composer surface it moved to.

The RFC records the sampled-value to token mapping, plus four places the frames
were not followed: their label greys fail WCAG AA (~2.5:1), thinking level and
runtime colors encode state we would lose, and the agent pill uses the agent's
own color instead of the frame's cyan.
…er grey

The frames use one fill for every surface (#09090b) and let hairline borders do
the separating. Our chrome (left sidebar, both header bars, footer bar) painted
itself #171717 through --color-sidebar, so it read as a lighter grey band around
a correctly-colored transcript and composer.

--color-sidebar now resolves to --color-surface-primary in the dark theme, which
covers every bg-sidebar consumer at once. The light and Flexoki themes keep their
own sidebar fill: a lighter sidebar is a deliberate choice there, and the frames
only specify dark.

Three bg-sidebar consumers are floating surfaces rather than chrome (the drag
ghost, the skill count badge, the SQL samples dropdown) and would have lost their
contrast against the page, so they move to bg-dark, matching the fill the repo's
other popovers already use.

Also fixed while here:
- ThemeContext pinned an inline body background of --color-background (#1f1f1f),
  overriding the stylesheet's surface-primary. Nothing visible sat on it, but it
  would show through overscroll and before mount.
- The context pill's hover was bg-sidebar-hover, which is not a defined token, so
  the pill had no hover feedback at all. It now uses bg-hover.

Review follow-ups on the previous commit: --color-composer-send carries the
sampled #a4e8f2 rather than an hsl() that rounded to #a6e8f2; ChatInput's
focusBorderColor is renamed agentColor since nothing draws a focus border with it
anymore; and the story checks the editing class with classList.contains.
@ibetitsmike ibetitsmike changed the title 🤖 feat: implement Review 1.4 layouts (footer bar, title header, creation hero) + design-reference RFC 🤖 feat: match Review 1.4 layouts and palette (footer bar, title header, creation hero, composer) Jul 25, 2026
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Two commits since your last pass, both addressing feedback that the implementation matched the design's layout but not its colors.

  • 5ed158b5d composer palette: the prompt input had its own #1D1D1D fill and border where the design has none. It is now transparent inside a single bordered box that also holds the pill row, and that box carries the editing tint that used to sit on the textarea (VimTextArea's isEditing/focusBorderColor props are gone as a result). Pill borders, meter track, and the send button move to the tokens holding the sampled values. --color-composer-send is new in all four themes.
  • 9745fc1e5 chrome: --color-sidebar resolves to --color-surface-primary in the dark theme only, so the sidebar, header bars, and footer stop painting a lighter grey band. Three bg-sidebar consumers are floating surfaces (drag ghost, skill badge, SQL dropdown) and move to bg-dark so they keep contrast. Also fixes an inline body background that overrode the stylesheet, and a hover:bg-sidebar-hover that was never a defined token so the context pill had no hover feedback.

Worth your attention: whether the dark-only --color-sidebar change misses any surface that needed the old contrast, and whether the send button's pinned hover classes are the right way to defeat the ghost variant's neutral hover.

Mux is working on this PR on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9745fc1e52

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/styles/globals.css
…theme-color

Codex caught that the Windows/Linux titlebar overlay still hardcoded #171717, so
the native window-controls strip stayed the old lighter grey above a now
page-colored header. The main process cannot read CSS variables, so the value has
to be duplicated somewhere; this puts it in one place both layers import.

The theme-color meta tag had the same problem for a different reason: its dark
value was #1e1e1e, matching neither the old chrome nor the new. Both now come
from CHROME_COLORS, so mobile browser chrome matches the page too.

The overlay still uses the dark value on every theme, as before: the window is
created before the renderer resolves a theme, and making the overlay follow theme
changes needs setTitleBarOverlay plumbing that is out of scope here.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Fixed in 93995c1b7. You were right that the overlay would sit as a lighter rectangle above the header.

Since the main process cannot read CSS variables, the value has to live in TS somewhere, so it now lives in exactly one place (src/common/constants/chromeColors.ts) that both the titlebar overlay and the theme-color meta tag import. That surfaced the same bug in the meta tag, whose dark value was #1e1e1e, matching neither the old chrome nor the new; mobile browser chrome now matches the page as well.

The overlay still uses the dark value regardless of theme, as it did before. Making it follow theme changes needs setTitleBarOverlay plumbing from the renderer, which I left out of scope here.

Mux is working on this PR on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93995c1b7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/contexts/ThemeContext.tsx
Codex found a third copy of the palette: index.html sets theme-color
synchronously before any bundle loads, and its dark/light values were still
#1e1e1e / #f5f6f8. So a cold load painted the old color and the React path then
replaced it, flipping the browser chrome during hydration.

The boot script is inline and cannot import, so the values stay duplicated. A
colocated test now parses that block and compares it against CHROME_COLORS, which
turns silent drift into a failing test. Verified it fails by reintroducing the old
dark value.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Fixed in 44e022677. Good catch on the hydration flip: I had synced two of the three copies and missed the one that runs first.

index.html's boot script is inline and cannot import, so its values stay duplicated by necessity. Rather than leave that to comments, a colocated test now parses that block and compares it against CHROME_COLORS, so drift fails a test instead of silently flashing. I verified the guard fails by reintroducing the old #1e1e1e, then restored it.

Mux is working on this PR on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44e022677f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread index.html
Comment thread src/browser/components/ChatPane/WorkspaceFooterBar.tsx
… per workspace

Two Codex findings.

The theme-color flash was not fully fixed: the static meta tag was still #1e1e1e,
which is what a cold load shows before the boot script runs and what survives when
that script's catch path fires (blocked storage sets data-theme dark but never
touches the meta). Correcting the static value fixes both paths at once, so the
catch branch needs no extra code, and the boot script's own last-resort literal
now reads from its palette. The guard test covers the static value too; verified
it fails by reintroducing #1e1e1e.

WorkspaceShell keeps ChatPane mounted across workspace switches, so the footer's
horizontal scroll offset carried over: on a narrow viewport, switching from a
scrolled footer to another overflowing one opened with the runtime, PR, and drift
items off-screen. The bar is now keyed by workspace id so it remounts at scroll 0.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Both fixed in b6f2c1fab.

Boot fallback: you're right that I only fixed the assigned value, not the starting one. The static <meta name="theme-color"> was still #1e1e1e, which covers both paths you identified: the pre-script cold load and the catch branch, which sets data-theme="dark" but never touches the meta. Correcting the static value fixes the catch path without adding code to it, and the boot script's last-resort literal now reads from its own palette. The guard test asserts the static value too, verified failing by reintroducing #1e1e1e.

Footer scroll: confirmed, ChatPane stays mounted so scrollLeft carried across switches. The bar is now keyed by workspaceId, so it remounts at scroll 0 rather than needing an effect to reset it.

Mux is working on this PR on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6f2c1fabe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/styles/globals.css
Comment thread src/browser/features/ChatInput/index.tsx
…inside 375px

The native chrome palette (titlebar overlay, theme-color meta) mirrored the page
fill, which only matches the chrome in the dark theme. It now mirrors each
theme's --color-sidebar, so light and Flexoki keep their own chrome contrast.

The composer's new bordered surface added 16px of inner padding, which pushed
the pill row's 340px minimum past what a 375px viewport leaves. The row now
clamps at 320px, with the matching container-query threshold.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Both P2 findings are addressed in 412adfa (replies on each thread):

  • The native chrome palette now mirrors each theme's --color-sidebar instead of --color-surface-primary, so light and Flexoki keep their deliberate chrome contrast while dark stays #09090b. Verified against rendered values in all four themes.
  • The composer pill row's clamp drops from 340px to 320px (with its coupled container query) so it fits the 325px a 375px viewport leaves after the new box's border and padding. Re-measured at 375px and 390px.

The installed app paints its splash and OS chrome from public/manifest.json,
which still held the old #1e1e1e. The chrome palette test now covers it, so
the manifest cannot drift from the boot script and meta tag.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Latest head is d63f817. Since the previous request it also brings public/manifest.json onto the new palette: the installed PWA painted its splash and OS chrome from the pre-parity #1e1e1e, and the chrome-palette test now covers the manifest alongside the boot script and the static meta tag.

Posted by Mux on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Head moved to 64bbf34 (comment-only: the chrome palette doc now lists the PWA manifest as a consumer). Please review that head.

Posted by Mux on Mike's behalf.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64bbf3421c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/components/VimTextArea/VimTextArea.tsx
Moving the border off the textarea also removed its focus border, and the
textarea keeps focus:outline-none, so keyboard focus had no visible state. The
box now takes the agent-colored border the textarea used to show, keyed off
textarea focus so tabbing across the pills does not light up the whole box.

The story asserts the rendered border color changes, which also covers the
arbitrary has-[] variant compiling.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

The focus-state P2 is fixed in 7151343 (reply on the thread). Head is 7151343.

The composer box now shows the agent-colored border the textarea used to show on focus, keyed off has-[textarea:focus] so the pill row's own focus rings do not light up the whole box, and a new FocusedComposer story asserts the rendered border color actually changes on focus.

Posted by Mux on Mike's behalf.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 71513434c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The comment claimed the pill row's controls carry their own focus rings, which
is false: their computed outline-style is none even at :focus-visible. That gap
predates this branch. State the real reason for keying off textarea focus, which
is that the border replaces the textarea's own focus border and should keep
meaning "the prompt has focus".
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Head is 0462aeb. Comment-only change since your approval of 7151343: my justification comment on the composer focus border claimed the pill row's controls carry their own focus rings, which is false (their computed outline-style is none even at :focus-visible). That gap predates this branch and this PR removes no focus styling from those controls, so the comment now states the accurate rationale instead of the incorrect one. No behavior change.

Posted by Mux on Mike's behalf.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 0462aeb352

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

2 participants