fix(emcn): stop the tab strip scrolling vertically by a pixel - #6113
Conversation
The active tab extends one pixel past the strip so it covers the bottom border and reads as joined to the panel below. That pixel came from `-mb-px` on the tab itself, which meant it overflowed the row containing it — and that row is a scroll container, because `overflow-x: auto` computes a visible `overflow-y` to `auto` as well. The strip was therefore scrollable on the y axis by exactly one pixel, which is enough for a trackpad to nudge the tabs out of view. Moving the overlap onto the row keeps the tabs flush inside it, so the scroll container has nothing to scroll, while the row itself still hangs the pixel over the border. The strip is unchanged visually. Measured in a real renderer with the component's own classes: the row was scrollHeight 30 against clientHeight 29, and is now 30 against 30. A first attempt to reproduce it with hand-written inline styles showed no overflow at all — the real class output was needed to see it. Shared by the browser and terminal panels, so both stop scrolling.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview The active tab still overlaps the strip’s bottom border visually, but the overlap no longer sits inside the Adds a source-level test that enforces a single Reviewed by Cursor Bugbot for commit b0bd2f2. Configure here. |
|
@cursor review |
Greptile SummaryMoves the one-pixel tab overlap from each tab button to the scrolling row and adds a source-level regression guard that ensures negative bottom margins remain outside the row’s descendants. Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/emcn/src/components/tab-strip/tab-strip.tsx | Relocates -mb-px to the horizontal scrolling row, preserving the visual overlap without creating descendant vertical overflow. |
| packages/emcn/src/components/tab-strip/tab-strip.test.ts | Adds a Node-compatible invariant test ensuring the sole negative bottom margin belongs to the scrolling row. |
Reviews (2): Last reviewed commit: "test(emcn): pin the invariant, not the c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 93593c2. Configure here.
Review pointed out the guard read source strings rather than rendered geometry, so it could pass while the overflow returned. Correct, and worth being precise about the compromise: this package's vitest runs in `node` with no browser mode, and the repo's only Playwright harness drives Electron against static HTML fixtures, so nothing here can lay out a React tree against the compiled CSS. Adding that capability is real infrastructure, not a line in this fix. What the guard can do is pin the invariant instead of two particular class strings: no negative bottom margin anywhere inside the scrolling row. That now catches the regression wherever a descendant reintroduces it, rather than only on the tab button it came from — verified against both shapes. The rendered behaviour stays measured rather than asserted, and the numbers are recorded next to the guard.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b0bd2f2. Configure here.
1 participant
The desktop browser's tab strip could be scrolled on the y axis — enough for a trackpad to nudge the tabs half out of view.
Root cause
The active tab extends one pixel past the strip so it covers the bottom border and reads as joined to the panel below. That pixel came from
-mb-pxon the tab button, which meant it overflowed the row containing it — and that row is a scroll container, becauseoverflow-x: autocomputes a visibleoverflow-ytoautoas well. One pixel of scrollable overflow is all it takes.Fix
Move the overlap onto the row instead of the tab. The tabs then sit flush inside the scroll container so there is nothing to scroll, while the row itself still hangs the pixel over the border. No visual change.
Verification
Measured in a real renderer using the component's own class strings:
Worth noting: a first attempt to reproduce this with hand-written inline styles showed no overflow. Only the real compiled classes surfaced it, which is why the numbers above come from the actual class output rather than a simplified repro.
The guard asserts the overlap lives on the scrolling row and not on the tab, and was confirmed to fail when the original layout is restored.
TabStripis shared by the browser and terminal panels, so both stop scrolling.Not included
The reported "can't open a new tab" is a separate issue and is deliberately left out of this PR. Diagnosed for whoever picks it up: at
MAX_BROWSER_TABS(8) the new-tab button isdisabled, and the shared button appliesdisabled:pointer-events-none— so the trigger cannot receive hover and the tooltip that would say "Maximum of 8 tabs" never opens. Separately,ipc.ts:578does.catch(() => {})aroundhandlePanelAction, so theSessionErrorthrown byaddTabInternalat the cap is discarded. Between the two, the button looks simply broken.