fix: render TopBar on Linux in sidebar and hidden layouts - #3449
Conversation
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: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{md,mdx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (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: 🚥 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. Comment |
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.
1 participant
What changed
In 4.16, update notifications moved from a modal to a titlebar
UpdateLabel, and aDownloadsIndicatormoved into the same title/tab bars.Shellrenders a horizontalTopBarfor non-tabs layouts (sidebar/hidden) — but that branch was gated todarwinonly, and a separate branch coverswin32. Linux fell through both, so on Linux those layouts got noTopBarat all: no update surface, no downloads indicator, and noServerSwitcherin thehiddenlayout.This extends the darwin-only branch to also cover
linux(['darwin', 'linux'].includes(process.platform)), mirroring darwin: noWindowControls(native frame already provides them) and noWindowDragBar(that block staysdarwin-only — Linux has a native titlebar to drag, macOS does not).Before touching the shared
TopBar/TabBarchrome, I checked for darwin-only assumptions that would misrender on Linux under a native frame:TrafficLightSpacerinTabBar/styles.tsx) is already gated withisDarwin && !isVerticalat the call site inTabBar/index.tsx.TopBar/index.tsxnever imports/uses it at all, so there's nothing to gate there.-webkit-app-region: dragon the sharedStripis unconditional today, andTabBaralready renders that sameStripon Linux in thetabslayout (its tabs branch is gated!== 'win32', not=== 'darwin') — so Linux already ships with an unconditional drag region on the tab strip today, without incident.TopBarfollows the same precedent; no new gating needed.win32-onlyprocess.platformreads inUpdateLabel/DownloadsIndicator(dropdown placement) fall through to the darwin/default case for Linux, which is the correct anchor since Linux, like darwin, has no leading meatball button pushing the pill.No component-level code changes were needed beyond the
Shellgate 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'sTopBarmock droppedcenterSlotentirely, so thehiddenlayout'sServerSwitcherwas never actually exercised by any existing test (darwin included) despiteShellpassing it. Fixed the mock to forwardcenterSlotand added assertions that exercise it.What CI proves
Shell/index.spec.tsx: newlinux chromecases assert theTopBarmounts with the downloads indicator (and, inhiddenlayout, theServerSwitcher) with noWindowControls, on a real Electron-runtime component tree (@kayahr/jest-electron-runner).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) toawait screen.findByTestId(...), so they wait for the unseen state to render instead of racing it. No component code changed.Test plan
yarn test --runTestsByPath src/ui/components/Shell/index.spec.tsx src/ui/components/TopBar/DownloadsIndicator.spec.tsx— 68 passed, 0 failedyarn lint— exit 0npx tsc --noEmit— exit 0Summary by CodeRabbit
New Features
Bug Fixes
Documentation