Skip to content

fix(ui): keep the menu bar inside the app's gutter - #693

Merged
benvinegar merged 1 commit into
mainfrom
claude/navbar-bg-char-spacing-61af0c
Aug 9, 2026
Merged

fix(ui): keep the menu bar inside the app's gutter#693
benvinegar merged 1 commit into
mainfrom
claude/navbar-bg-char-spacing-61af0c

Conversation

@benvinegar

@benvinegar benvinegar commented Aug 8, 2026

Copy link
Copy Markdown
Member

The top menu bar painted its chrome background into the two outer gutter columns, so it didn't line up with the panes below — the app's one-column margin ran down the whole UI except the top row.

MenuBar was one full-width box with paddingLeft/paddingRight: 1 and backgroundColor: theme.panelAlt, so the padding cells were chrome. This moves the band into an inner flexGrow: 1 box and lets the outer row paint theme.background.

Before After
before after

Real PTY captures at 90 columns, rendered cell-for-cell from the colors the terminal received. The zooms magnify the edge columns: before, the band runs to the very edge on the menu row; after, the dark gutter is continuous down both sides.

Column positions are unchanged — labels still start at column 1, so MenuSpec.left, MenuDropdown anchoring, and menuBarTitleWidth all still hold.

Adds a PTY regression test asserting the menu bar's edge cells match the body's, plus a rowCellBackgrounds harness helper — text snapshots can't see this bug, only the cell background differs. Confirmed the test fails without the fix.

typecheck, lint, test/pty/chrome.test.ts, and test:tty-smoke are clean. ⚠️ The full bun test has 5 failures in extensions-integration and file-views-integration; they fail identically on the unmodified baseline, so they're pre-existing and unrelated.

🤖 Generated with Claude Code

The menu bar was a single full-width box whose horizontal padding was
painted with the chrome background, so the two outer columns read as
chrome while every row below leaves them as the app background. The
result was a top bar that visibly failed to line up with the panes
under it.

Move the chrome band into an inner box and let the outer row paint the
app background, so the bar leaves the same one-column margin the body
panes leave. Column positions are unchanged: menu labels still start at
column 1, so MenuSpec.left and dropdown anchoring stay correct.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hunk-web Ready Ready Preview Aug 8, 2026 7:15pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR moves the menu bar’s chrome background into an inner flex row so the outer terminal columns retain the application background.

  • Preserves a one-column gutter around the menu bar.
  • Adds PTY coverage that checks rendered cell backgrounds at both edges.
  • Adds a reusable helper for reading row background colors and records the patch in a changeset.

Confidence Score: 4/5

The PR appears safe to merge functionally, with only a non-blocking repository-formatting issue to correct.

The menu-bar layout change has targeted PTY coverage and no concrete behavioral regression was established; the accepted concern is limited to changed TypeScript and TSX formatting.

Files Needing Attention: src/ui/components/chrome/MenuBar.tsx, test/pty/chrome.test.ts, test/pty/harness.ts

Important Files Changed

Filename Overview
src/ui/components/chrome/MenuBar.tsx Restructures the menu bar to preserve outer gutters; behavior is covered, but changed formatting violates the mounted repository style requirement.
test/pty/chrome.test.ts Adds focused PTY regression coverage for gutter and chrome-band backgrounds, with the same formatting violation.
test/pty/harness.ts Adds a fail-fast helper that expands terminal spans into background entries, with the same formatting violation.
.changeset/lucky-moons-smash.md Accurately documents the user-visible menu-bar gutter correction.
Prompt To Fix All With AI
### Issue 1
src/ui/components/chrome/MenuBar.tsx:24-25
**Keep changed code formatting consistent**

The changed TypeScript and TSX blocks use two-space indentation and double-quoted strings instead of the repository-required four-space indentation and single quotes, creating inconsistent formatting across the menu implementation and its PTY coverage.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(ui): keep the menu bar inside the ap..." | Re-trigger Greptile

Comment on lines +24 to +25
// The outer row paints the app background so the bar keeps the same
// one-column gutter the body panes have; only the inner band is chrome.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Keep changed code formatting consistent

The changed TypeScript and TSX blocks use two-space indentation and double-quoted strings instead of the repository-required four-space indentation and single quotes, creating inconsistent formatting across the menu implementation and its PTY coverage.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/components/chrome/MenuBar.tsx
Line: 24-25

Comment:
**Keep changed code formatting consistent**

The changed TypeScript and TSX blocks use two-space indentation and double-quoted strings instead of the repository-required four-space indentation and single quotes, creating inconsistent formatting across the menu implementation and its PTY coverage.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar
benvinegar merged commit 505d9d3 into main Aug 9, 2026
13 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