fix: render TopBar on Linux in sidebar and hidden layouts - #3449
Conversation
Sorry, something went wrong.
Extends the darwin-only non-tabs TopBar branch in Shell to also cover linux, so the sidebar and hidden layouts get the update label and downloads indicator on Linux the same way they already do on darwin and win32. The WindowDragBar and TrafficLightSpacer stay darwin-only since Linux has a native titlebar to drag and no macOS traffic lights to inset for. Also fixes the Shell spec's TopBar mock, which dropped centerSlot entirely (so the hidden layout's ServerSwitcher was never actually exercised on any platform), and adds linux coverage asserting the TopBar mounts with the downloads indicator (and server switcher in hidden layout) with no window controls.
Ubuntu CI occasionally rendered the unseen-completed state one tick
after mount, so a synchronous getByTestId('downloads-unseen-dot')
right after render could miss it. Await the unseen state with
findByTestId before asserting on it or clicking through it in every
test that seeds a completed-unseen download at mount time.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 1ee34ec5-f633-4b82-bc0a-1c3640688fbb 📥 CommitsReviewing files that changed from the base of the PR and between 4816217 and 2a2214d. 📒 Files selected for processing (2)
📄 CodeRabbit inference engine (CLAUDE.md)
Files:
docs/KNOWN_ISSUES.md (1) WalkthroughThe Shell now renders the non-tab TopBar on Linux. Shell tests cover Linux layouts and controls. DownloadsIndicator tests stabilize asynchronous state and time-dependent assertions. Known issue documentation describes platform-dependent transition assertions. ChangesLinux TopBar and download indicator
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: type: bug 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
The "reverts to the plain idle glyph" test seeded endTime as mountTime + 1 against a separately captured Date.now() call, requiring store creation and first render to land within the same millisecond as the capture to pass. Pin Date.now for the test's duration so seenAt and endTime ordering is deterministic instead of racing the wall clock. Also reverts the 3 sibling findByTestId conversions (4816217) back to synchronous getByTestId — their endTime seeding already has a 1000ms margin and never flaked, so the async retry was dead weight.
GitHub-hosted Windows/macOS runners report prefers-reduced-motion: reduce at the OS level while ubuntu-latest/xvfb and local dev Macs report no-preference, so getComputedStyle-based transition assertions diverge per platform. Document the parsed-CSSOM workaround pattern already in use in DownloadsIndicator.spec.tsx.
What changed
In 4.16, update notifications moved from a modal to a titlebar UpdateLabel, and a DownloadsIndicator moved into the same title/tab bars. Shell renders a horizontal TopBar for non-tabs layouts (sidebar/hidden) — but that branch was gated to darwin only, and a separate branch covers win32. Linux fell through both, so on Linux those layouts got no TopBar at all: no update surface, no downloads indicator, and no ServerSwitcher in the hidden layout.
This extends the darwin-only branch to also cover linux (['darwin', 'linux'].includes(process.platform)), mirroring darwin: no WindowControls (native frame already provides them) and no WindowDragBar (that block stays darwin-only — Linux has a native titlebar to drag, macOS does not).
Before touching the shared TopBar/TabBar chrome, I checked for darwin-only assumptions that would misrender on Linux under a native frame:
No component-level code changes were needed beyond the Shell gate itself — everything else already generalizes correctly to Linux by existing precedent.
Also fixed
While adding Linux coverage to Shell/index.spec.tsx, found the test's TopBar mock dropped centerSlot entirely, so the hidden layout's ServerSwitcher was never actually exercised by any existing test (darwin included) despite Shell passing it. Fixed the mock to forward centerSlot and added assertions that exercise it.
What CI proves
Deflake (separate commit)
TopBar/DownloadsIndicator.spec.tsx's "reverts to the plain idle glyph ... once ... marked seen" test flaked on Ubuntu CI (run 31395755173): getByTestId('downloads-unseen-dot') didn't find the dot on a slow runner tick, though the same code passed on the previous run. Switched that assertion and its synchronous siblings (that seed the same unseen state at mount and assert on it) to await screen.findByTestId(...), so they wait for the unseen state to render instead of racing it. No component code changed.
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Documentation