Add top-tab layout option - #420
Merged
Merged
Conversation
Adds an alternative horizontal tab strip that renders above the browser viewport instead of the 200px-wide left sidebar, reclaiming that width for the live browser view. Persisted via localStorage[hub-tabs-position] (default 'side', so existing users see no change). - Sidebar.tsx grows a mode: 'side' | 'top' prop. In 'top' mode it renders a horizontal nav with TabChip components — favicon + corner status dot + label, no separator stripes between chips, scrollable horizontally when there are too many to fit. - HubApp.tsx hydrates tabsPosition from localStorage, sets data-tabs-position on .hub-body so CSS can flip flex-direction, and dispatches pane:layout-change AFTER React commits the new layout (via double rAF) so AgentPane re-measures bounds against the updated DOM. The selected-tab highlight is suppressed when not in grid view. - SettingsPane.tsx adds a LayoutSection card under Application with a two-card visual picker (mini-mockups of side vs top), wired via a hub:tabs-position-change event the hub listens for. - hub.css covers the new .tabstrip strip + .layout-picker visual cards.
The hub's emulated viewport width was matched to the physical rect's aspect ratio only on first attach. On subsequent attaches the cached emulatedWidth was reused, so when the user changed layout shape (e.g. toggling the new top-tab layout, which reclaims the sidebar's width), fitBoundsToView shrank-and-centered the WebContentsView using the stale emulatedWidth — leaving black gutters on each side of the live browser. - attachToWindow's already-attached branch now recomputes the aspect-matched emulatedWidth and, when it differs from the cached value, re-applies enableDeviceEmulation and refits the zoom factor. This branch only fires on real layout-shape changes (not casual window resizes, which still hit setViewBoundsFitted), so the user's manual Cmd+=/Cmd+- zoom is preserved everywhere except an explicit layout switch. - MAX_EMULATED_VIEWPORT_WIDTH raised from 1600 to 1920. With top tabs reclaiming the sidebar width, common desktop monitors (~1500-1800px hub area) were running into the 1600 cap and producing ~100px letterbox gutters even with the refit fix above. 1920 covers most desktop sizes without crossing the threshold where sites like X/Twitter shift into ultra-wide centered layouts.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/renderer/hub/hub.css">
<violation number="1" location="app/src/renderer/hub/hub.css:4395">
P2: Top-tab chips still render separator stripes, conflicting with the intended “no separator stripes” layout.</violation>
</file>
<file name="app/src/main/sessions/BrowserPool.ts">
<violation number="1" location="app/src/main/sessions/BrowserPool.ts:621">
P1: Guard the aspect-ratio recomputation against zero/invalid bounds to avoid propagating `NaN` into `setBounds` during transient relayout states.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
The chip separator pseudo-element was left over from an earlier Chrome- inspired pass; the intended top-tab look has no inter-chip stripes. Removes both the separator rule and the now-dead hover/active-hide companions.
If a frame fires while the hub is mid-relayout (pane width or height momentarily 0, or non-finite for any reason), the aspect-ratio recompute in attachToWindow's already-attached branch divides by zero / produces NaN and propagates Infinity/NaN into the clamp + setBounds path. Skip the recompute and the trailing setBounds when the incoming bounds shape is invalid; ResizeObserver fires again with a real rect. Also updates the two BrowserPool unit tests whose hard-coded fitted geometry encoded the old MAX_EMULATED_VIEWPORT_WIDTH (1600). With the cap raised to 1920, a 2000x900 rect at zoom 0.5 yields renderedWidth 960 and a 520px letterbox (was 800/600).
Contributor
Author
|
@cubic review |
Latest main added a light/dark/system appearance picker, so the top-tab layout branch needs its custom picker and tabstrip states to use theme-aware tokens instead of hard-coded dark-mode highlights. The settings regression now clicks both the light theme option and top layout option in one render so this merge point stays covered. Constraint: origin/main now owns the appearance picker and renderer theme-mode plumbing Rejected: Leave white alpha fills in the layout mockups | they wash out when data-mode=light flips the shell palette Confidence: high Scope-risk: narrow Directive: New hub UI states should use semantic theme tokens or mode-aware RGB vars, not fixed dark-surface rgba values Tested: npm run test -- tests/unit/hub/SettingsPane.spec.tsx Tested: npm run lint Tested: npm run typecheck
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed
Hub UI (`feat(hub): add top-tab layout option with settings picker`)
Browser viewport refit (`fix(browser): refit emulated viewport on layout shape change`)
Test plan
Summary by cubic
Adds an optional top-tab layout that replaces the left sidebar with a horizontal tab strip to widen the live browser viewport. Also refits the emulated viewport on layout changes to remove gutters and keeps the new UI readable in light mode.
New Features
localStorage['hub-tabs-position']and reflected on.hub-body[data-tabs-position].Sidebarsupportsmode: 'side' | 'top'.HubAppemitspane:layout-changeafter commit; selected highlight is suppressed outside grid view.Bug Fixes
enableDeviceEmulationand zoom as needed, and skip refit on zero/non-finite bounds (preserves manual zoom except on explicit layout switches).MAX_EMULATED_VIEWPORT_WIDTHto 1920 to prevent letterboxing on wider layouts.Written for commit 3eaff99. Summary will update on new commits.