Align top-level tab toolbars and responsive behavior - #229
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR standardizes top-level tab headers and command bars across Relay destinations. It adds shared tab-chrome primitives and responsive styles, updates Header Search action rails and labels, and expands renderer, CSS, Electron layout, zoom, and screenshot coverage. ChangesTab Chrome and Header Search Alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HeaderSearch
participant SearchResultRow
participant ContactBridgeAction
HeaderSearch->>SearchResultRow: Render concise primary action and action rail
SearchResultRow->>ContactBridgeAction: Render bridge action for contact results
HeaderSearch->>SearchResultRow: Update keyboard footer hints
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Pull request was closed
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/e2e/redesign-screenshots.spec.ts (1)
172-184: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSkip zero-size buttons in the containment check.
toolbar.querySelectorAll('button')returns hidden buttons too. A button hidden withdisplay: nonereports a rect of0,0,0,0. When the active panel does not start atx ≈ 0,rect.left >= panelBounds.left - 1then fails and the check reports a containment failure that no user can see.This helper now guards six tabs, so filter out zero-size rects.
♻️ Proposed refactor to ignore non-rendered buttons
return Array.from(toolbar.querySelectorAll('button')).every((button) => { const rect = button.getBoundingClientRect(); + if (rect.width === 0 && rect.height === 0) return true; return rect.left >= panelBounds.left - 1 && rect.right <= panelBounds.right + 1; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/redesign-screenshots.spec.ts` around lines 172 - 184, Update the containment predicate inside the activePanel polling callback to ignore buttons whose bounding rectangle has zero width or height before checking panel bounds. Keep rendered buttons subject to the existing left/right containment checks.tests/e2e/css-visual-contracts.spec.ts (1)
1094-1114: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse the real action labels in the toolbar fixture.
Several fixture labels are shorter than the labels the app renders. Examples:
Copy AllversusCopy All On-Call Info,ExportversusExport to CSV, andRefreshversusRefresh cloud statusandRefresh Radar now. The test then measures narrower toolbars than production. A real overflow regression at 680px can pass this contract.Align the fixture labels with the labels asserted in the renderer tests so the overflow checks at lines 1268-1270 and 1373-1376 measure production widths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/css-visual-contracts.spec.ts` around lines 1094 - 1114, The toolbar fixture in the tabs array uses shortened action labels, causing inaccurate width measurements. Replace each fixture label with the exact labels rendered and asserted by the renderer tests, including the On-Call, Problems, and Radar actions such as “Copy All On-Call Info,” “Export to CSV,” “Refresh cloud status,” and “Refresh Radar now,” while preserving the existing tab structure and overflow checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/DESIGN.md`:
- Around line 375-376: Update the action-order description near History and Save
Image so Save Image remains in the Alerts utility group, consistent with its
definitions elsewhere in the design. Remove its placement in the right-aligned
workflow group and document one consistent ordering of the utility and workflow
actions.
- Around line 300-305: Update the Header Search behavior description around
“Search results preserve context” to limit the no-Compose-change guarantee to
contact, server, document, workspace, and tab results. Explicitly document that
group rows use “Add group” as the primary action and change Compose, while
preserving the existing row and footer guidance.
In `@src/renderer/src/components/HeaderSearch.tsx`:
- Around line 674-680: Update handleKeyDown to handle Tab when
dropdownResults[activeIndex] is a contact: prevent the default focus behavior
and invoke handleSecondarySelect. Also restrict hasContactSecondaryAction so the
“Bridge contact” hint is rendered only when the active result is a contact.
In `@src/renderer/src/styles/tab-chrome.css`:
- Around line 79-83: Update the .tab-command-group--workflow rule by adding an
empty line before the margin-left declaration to satisfy Stylelint’s
declaration-empty-line-before requirement.
- Around line 58-147: Update .tab-command-bar and .tab-command-group to use
flex-wrap: nowrap by default, then override both to flex-wrap: wrap inside the
existing max-width: 720px media query so toolbar controls remain on one row
above the breakpoint.
In `@tests/e2e/css-visual-contracts.spec.ts`:
- Around line 1274-1275: Update the geometry assertions near the utilityHeights
and workflowHeights checks to first assert each measured array’s length matches
its corresponding declared action count, including tabs with zero workflow
actions, then retain the existing uniform-height assertions so empty arrays
cannot pass without validating the expected counts.
---
Nitpick comments:
In `@tests/e2e/css-visual-contracts.spec.ts`:
- Around line 1094-1114: The toolbar fixture in the tabs array uses shortened
action labels, causing inaccurate width measurements. Replace each fixture label
with the exact labels rendered and asserted by the renderer tests, including the
On-Call, Problems, and Radar actions such as “Copy All On-Call Info,” “Export to
CSV,” “Refresh cloud status,” and “Refresh Radar now,” while preserving the
existing tab structure and overflow checks.
In `@tests/e2e/redesign-screenshots.spec.ts`:
- Around line 172-184: Update the containment predicate inside the activePanel
polling callback to ignore buttons whose bounding rectangle has zero width or
height before checking panel bounds. Keep rendered buttons subject to the
existing left/right containment checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e76acb06-bc41-4aa1-83d3-ee0d207114cf
📒 Files selected for processing (40)
docs/DESIGN.mddocs/superpowers/plans/2026-08-06-all-tab-toolbar-search-alignment.mddocs/superpowers/specs/2026-08-06-all-tab-toolbar-search-alignment-design.mdsrc/renderer/src/components/HeaderSearch.tsxsrc/renderer/src/components/__tests__/HeaderSearch.test.tsxsrc/renderer/src/components/oncall/oncall.csssrc/renderer/src/components/tab-chrome/TabChrome.test.tsxsrc/renderer/src/components/tab-chrome/TabChrome.tsxsrc/renderer/src/features/knowledge/KnowledgeHome.tsxsrc/renderer/src/features/knowledge/__tests__/KnowledgeHome.test.tsxsrc/renderer/src/features/knowledge/__tests__/KnowledgeTypographyStyles.test.tssrc/renderer/src/features/knowledge/knowledgeWorkspace.csssrc/renderer/src/styles.csssrc/renderer/src/styles/components.csssrc/renderer/src/styles/cssQualityContracts.test.tssrc/renderer/src/styles/modals.csssrc/renderer/src/styles/tab-chrome.csssrc/renderer/src/tabs/AlertsTab.tsxsrc/renderer/src/tabs/AssemblerTab.tsxsrc/renderer/src/tabs/CloudStatusTab.tsxsrc/renderer/src/tabs/DynatraceProblemsTab.tsxsrc/renderer/src/tabs/PersonnelTab.tsxsrc/renderer/src/tabs/RadarTab.tsxsrc/renderer/src/tabs/__tests__/AlertsTab.test.tsxsrc/renderer/src/tabs/__tests__/AssemblerTab.test.tsxsrc/renderer/src/tabs/__tests__/CloudStatusTab.test.tsxsrc/renderer/src/tabs/__tests__/DynatraceProblemsTab.test.tsxsrc/renderer/src/tabs/__tests__/PersonnelTab.test.tsxsrc/renderer/src/tabs/__tests__/PersonnelTabStyles.test.tssrc/renderer/src/tabs/__tests__/RadarTab.test.tsxsrc/renderer/src/tabs/alerts.csssrc/renderer/src/tabs/alerts/AlertActionsMenu.tsxsrc/renderer/src/tabs/alerts/__tests__/AlertActionsMenu.test.tsxsrc/renderer/src/tabs/assembler/__tests__/AssemblerSidebar.test.tsxsrc/renderer/src/tabs/assembler/assembler.csssrc/renderer/src/tabs/cloud-status.csssrc/renderer/src/tabs/dynatrace-problems.csssrc/renderer/src/tabs/radar.csstests/e2e/css-visual-contracts.spec.tstests/e2e/redesign-screenshots.spec.ts
💤 Files with no reviewable changes (3)
- src/renderer/src/tabs/assembler/tests/AssemblerSidebar.test.tsx
- src/renderer/src/tabs/assembler/assembler.css
- src/renderer/src/tabs/alerts.css
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
Pull request was closed
|



Summary
Root cause
The tabs had accumulated independent header and toolbar layouts, and the shared responsive rule forced every command group into two rows below 1320px even when the controls still fit.
Verification
Summary by CodeRabbit
UI Improvements
Search Improvements
Accessibility
Documentation