Repository navigation
feat(desktop): integrate the title bar into the sidebar top strip - #5910
Conversation
On macOS the main window gets TitleBarStyle::Overlay + hidden_title with the traffic lights pinned at a fixed logical position, so the webview draws to the top of the window. The dashboard's new sidebar top strip reserves the lights' inset, carries a Codex-like collapse toggle that folds the sidebar to a rail (persisted, Cmd/Ctrl+B), and the provider usage strip moves up to share the title-bar row. Both strips move the window on drag and zoom on double-click through plugin:window commands granted per origin; the bundled bootstrap and update pages draw their own matching strip. Windows and Linux keep native decorations. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
React may run an updater without committing, so the localStorage write moves to an effect on the collapsed value (react-doctor's no-side-effect-in-state-updater-function gate). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
리뷰 · 우선순위 54 / 80이 글의 바탕은 맥에서는 창 위쪽의 기본 제목 줄을 치워요. 닫기·최소화·확대 단추는 웹 화면 위에 겹쳐요. 대시보드 왼쪽 칸 맨 위에 그 자리를 비워 두고, 옆에는 칸을 접는 단추를 넣어요. 접으면 그 단추와 신호등만 남아요. 접었는지는 이 브라우저에 기억되고, Cmd 또는 Ctrl과 B로도 바뀌어요. 오른쪽 맨 위에는 사용량 줄이 올라와요. 빈 곳을 끌면 창이 움직이고, 빈 곳을 두 번 누르면 창이 커져요. 시작 화면과 업데이트 화면도 맥에서만 위쪽에 같은 손잡이를 그려요. 윈도우와 리눅스는 원래 제목 줄을 그대로 둬요. 창을 끌고 키우는 권한만 새로 줘요. 대시보드가 열리는 라인 - 라인 - 메인테이너의 판단이 필요한 지점 브라우저로 연 대시보드에서도 Cmd/Ctrl+B가 칸을 접어요. 브라우저의 기본 동작을 막아서, 크롬에서는 북마크 막대 단축키를 가져가요. 이 단축키를 데스크톱 앱에만 둘지 정해 주세요. 접힌 칸의 너비도 신호등 여백과 단추 크기를 손으로 더한 값이에요. 여백을 바꾸면 단추가 잘릴 수 있어요. 너의 추천 바탕은 머지하기 전에 두 HTML에서 신호등 좌표와 CSS 여백이 어긋나면 실패하는 검사를 하나 두세요. 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Two revision blockers at exact head ee2f867d2e:
-
desktop/ui/update.htmlnow readsnavigator.userAgentin the updater path, buttests/clients/desktop-update-surface.test.tssupplies neithernavigatornor the new titlebar node. Exact-head test 3/4 and macOS 1/2 fail four existing timeout/supersession cases withReferenceError: navigator is not defined. Update the harness and add coverage for the macOS grip branch. -
At
<=760px,.sidebar-topis hidden while native traffic lights remain overlaid at(18,16). The replacement.mobile-topbarstarts a 44px menu button at x10 and has no macOS inset/minimum-width contract, so a narrow or zoomed macOS window can overlap native controls. Reserve the traffic-light area on the mobile strip/drawer and verify resize/zoom behavior.
The wide-layout inset, main-only Tauri capability, navigation pinning, and interactive-element drag exclusion are good controls. Please address the two contradicted boundaries and rerun exact-head desktop/macOS checks.
…yout entirely
- One shared top row: sidebar-top strip and main-top strip are both exactly
--titlebar-h (40px), traffic lights vertically centered in it via
traffic_light_position(18,14); toggle, quota badges and the bottom border
share that line.
- Collapsed is not a rail: .app--nav-collapsed drops the sidebar to a 0px
column; the strip is a direct .app child fixed to the top-left (the
sidebar's backdrop-filter would clip a fixed in-sidebar strip to 0px).
- .main pinned to grid column 2 while collapsed so auto-placement cannot
squeeze it into the empty sidebar track.
- desktop update/bootstrap pages: guard navigator before UA-sniffing so the
update surface works on non-mac webviews; harness gained a userAgent param
and coverage for the macOS grip branch.
- useSidebarCollapse({ shortcut }) — Cmd/Ctrl+B stays desktop-only so the
browser dashboard keeps Chrome's bookmark-bar shortcut.
- Mobile (<=760px): mobile-topbar reserves the lights inset so the traffic
lights can't overlap the hamburger.
- Pin test: lib.rs traffic_light_position, CSS row/inset/clear values and the
sidebar grid column width are asserted in one place so they can't drift.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Review items addressed at
Layout is also updated per the latest spec: a single 40px row spans the full window (lights centered via |
tao treats traffic_light_position y like a container inset rather than the buttons' top edge: y=14 put the light centers ~8px above the 40px row's center line. Verified empirically — at y=22 the light cluster, the toggle icon, and the strip text all center at the same pixel row (measured 141.5px from screen top on a 1600x1200 capture, window top 122, row border 161). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Traffic-light alignment fixed at |
|
@Ingwannu Both review blockers are resolved at
I reviewed the capability change: only read-only |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da00f5616b
ℹ️ 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".
| draggable, at the very top of the window, level with the traffic lights — so it | ||
| exists even while the bar inside it does not. The browser dashboard keeps the | ||
| bar as it was: no strip, no reserved row. */} | ||
| {desktopShell ? <MainTopStrip>{quotaSummary}</MainTopStrip> : quotaSummary} |
There was a problem hiding this comment.
When the desktop shell opens Models → Combos with quota data available, this wrapper makes .quota-summary-bar no longer a direct child of .main. The existing rules in gui/src/components/quota-summary-bar/quota-summary-bar.css:268-278 therefore stop matching, leaving .main-inner--combos at height: 100dvh; the new 40px title-bar sibling then makes the page one title-bar taller than the viewport and clips or scrolls the bottom of the workspace. Update those selectors for .main-top or otherwise size the Combos shell to the remaining height.
Useful? React with 👍 / 👎.
|
|
||
| /* The strip no longer sits inside .sidebar's flow, so the sidebar's own top padding | ||
| additionally reserves the strip's row (styles.css keeps its base 18px). */ | ||
| .sidebar { padding-top: calc(var(--titlebar-h, 40px) + 18px); } |
There was a problem hiding this comment.
At widths up to 760px, .sidebar-top is hidden and browsers plus Windows/Linux have no overlaid traffic lights, but this unconditional rule still adds the 40px title-bar height to the drawer's top padding. Since the App-loaded component stylesheet follows the base .sidebar { padding: 18px 14px; } declaration and the mobile rule only changes bottom padding, opening the drawer on those surfaces leaves an unintended 40px blank area above its header. Scope this reservation to wide layouts, while retaining the explicit macOS mobile override for traffic-light clearance.
Useful? React with 👍 / 👎.
…5936) * fix(gui): size desktop Combos under the title strip and keep the narrow header usable Three layout defects from the integrated title bar (#5910), found by a post-merge review: - The Combos sizing rules matched only a quota bar directly under .main; in the desktop shell the bar sits in .main-top, so the 100dvh shell overflowed the page by the 40px strip. The same rules now key on .main-top. - At <=760px the sticky .main-top painted over the sticky mobile header while scrolling. It now scrolls with the page (static, z-index auto), and the mobile header carries the window drag handlers so the window stays draggable after scrolling. - The titlebar metrics floored the points-to-CSS ratio at 1, so at 300% zoom a 360pt window kept an 80px inset and pushed the 44px menu off a 120px viewport. Zoomed in, only the lights share of the clearance shrinks (27px inset at 300%); the row keeps its 40px floor. * fix(test): keep config CLI spawn from wedging Bun batch Two Linux test 4/4 logs stop after the first config-show child returns; the next synchronous child never yields despite its 40s timeout and the 120s batch watchdog fires. Run the CLI child asynchronously with a separate deadline and explicit stdin policy so a blocked child produces a bounded test failure. Assert that the test event loop advances during config get; this was red with spawnSync and green with spawn.
2 participants
Summary
TitleBarStyle::Overlay+hidden_title(true)put the traffic lights inside the webview, andtraffic_light_position(18, 22)vertically centers them in a single shared 40px top row that spans the full window width. Windows/Linux keep native decorations — the sidebar-top layout applies there unchanged.gui/src/components/app-titlebar.tsxaddsSidebarTopStrip(traffic-light inset + a Codex-like panel collapse toggle,aria-expanded/aria-controlswired) andMainTopStrip, which lifts the provider usage strip (QuotaSummaryBar) onto the title-bar row so lights, toggle, badges, and the row's bottom border share one line. The strip is a direct.appchild — the sidebar'sbackdrop-filterwould make it the containing block of an in-sidebarposition:fixedstrip and clip it to 0 width when collapsed.display:none, no rail): the toggle floats over the top-left of the same white row, row content starts right after it, and the main area spans full width. State persists inlocalStorage; Cmd/Ctrl+B toggles it on desktop only (useSidebarCollapse({ shortcut })) so the browser dashboard keeps Chrome's bookmark-bar shortcut. On ≤760px the strip is hidden; the mobile bar clears the lights horizontally and the open drawer starts below them. On macOS, native window scale and page zoom keep that clearance in window points, and the window has a 360-point minimum width.plugin:windowcommands (gui/src/lib/window-chrome.ts), granted bycapabilities/dashboard-titlebar.jsonfor the loopback origin anddefault.jsonfor the app origin; the dashboard capability also grants a read-only window-scale query for zoom clearance — the bundled bootstrap and update pages draw their own 40px grip on macOS since an overlay page without one cannot be dragged or zoomed at all.Verification
4065a7bdbad91ecad7c74b19d9394de1bf268b52(before thedevmerge):bun test tests/clients/desktop-update-surface.test.ts(8 pass);cd gui && bun test tests/app-titlebar.test.tsx tests/mobile-topbar-layout.test.ts(10 pass);bun x tsc --noEmitandbun x tsc --noEmit -p gui/tsconfig.app.json(pass);bun run structure:check(pass); focused Rust capability testcargo test the_titlebar_commands_are_granted_on_each_origin --quiet(1 pass).da00f5616b9c3b028abbda897c0e202545f74f98: 31 passed, 6 path-filtered skips, no pending or failed checks.Expanded (desktop):
Collapsed (desktop):
Browser (unchanged except the toggle):
Checklist
structure/desktop-shell.mdcovers the overlay and the capability.start_dragging,toggle_maximize, and read-onlyscale_factor, bound tomainand the loopback origin; the app-origin capability retains only drag/zoom.Link to Devin session: https://app.devin.ai/sessions/a7a109ba9ce8496d9ea23c26e73056ac
Open in Devin Desktop: https://app.devin.ai/desktop/session/a7a109ba9ce8496d9ea23c26e73056ac?variant=devin
Requested by: @lidge-jun
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.