[devswha]

Fixes #1424
Fixes #1416
Fixes #1585

These three are grouped because they share DESIGN.md and the danger-button styles. The parts already fixed on main are left alone:

• Sidebar PC controls: make the row-height regression guard the fix (from #1422) #1424 items 1 and 2: fixed by test(sidebar): catch unequal heights in shared PC control rows #1732 and fix(sidebar): size PC controls from --control-h and dim Remove PC like its neighbours #1422.
• Sidebar Remove PC: disabled styling and hover follow-ups from #1394 #1416 items 1, 3 and 4: fixed by fix(sidebar): size PC controls from --control-h and dim Remove PC like its neighbours #1422.

Changes

• Sidebar PC controls: make the row-height regression guard the fix (from #1422) #1424 item 3, hover feedback: a hovered danger button keeps its tint and its --status-blocked border, and draws that border twice as thick (an inset ring in the same color). Before, ConfirmDialog, DevicesPanel and Remove PC showed no hover at all. The change keeps the danger color, as the issue asked, and does not bring back the neutral fill. The design precedent is T3 Code's destructive-outline button (button.tsx), which DESIGN.md already cites in this paragraph: on hover it strengthens its border and tint in the destructive color (border-destructive/32, bg-destructive/4) instead of switching to a neutral fill. This ring does the same with herdr's danger tokens; no code is borrowed.
• Sidebar PC controls: make the row-height regression guard the fix (from #1422) #1424 item 4, DESIGN.md: a one-line PC control is --touch-target tall on a coarse pointer, where Rename keeps its own line.
• Sidebar Remove PC: disabled styling and hover follow-ups from #1394 #1416 item 2, contrast: a pending PC removal keeps its full strength instead of being dimmed to 0.85.
  • Measured as drawn over the panel's --bg-elevated, any dimming that still reads as dimmed fails some theme. Tokyo Night dark is the worst: 3.89:1 at 0.85, 4.18:1 at 0.9, and only 0.97 reaches 4.5:1.
  • Its neighbours still dim to 0.55 while it runs, and the button is disabled with aria-busy.
  • The removal regression now measures the pending removal in every theme block of its mode, not just the two defaults: text at 4.5:1 or above, border at 3:1 or above.
  • DESIGN.md says so in place of "only lightly dimmed".
• Follow-ups from #1576: DESIGN.md wording for the New tab button toggle #1585, DESIGN.md: the New tab button lives in a mouse-driven window wider than 640px, the composer's (max-width: 640px), (pointer: coarse) query. Settings does not offer the toggle on a touch screen or a narrow window. settings.test.ts keeps only the behavioural checks.

Verification

• bun test src/lib/settings.test.ts: passes.
• bun run typecheck: passes.
• bun scripts/sidebar-machine-removal-demo-regression.ts (through dori heavy): PASS for dark and light. The pending removal measured in all 22 theme blocks has text at 4.78:1 at the lowest (Tokyo Night dark, Solarized light) and borders at 4.78:1 or above. Hover draws the inset ring in the button's own border color.
• bun scripts/sidebar-machine-layout-demo-regression.ts: PASS in all 14 scenarios, among them desktop 240px and 520px sidebars, a 320px phone, a 390px phone and an 844×390 landscape, in dark and light, and in Korean, Japanese and Chinese. I looked at the screenshots for desktop 240 dark (armed), phone 320 light (armed) and the dark pending removal: the pending Confirm remove PC stays at full strength and readable while Rename and Disconnect dim beside it, and the phone panel fits a 320px drawer.
• Code review: omo-native-code-reviewer ran the regression and caught that 0.9 still failed Tokyo Night (4.18:1); the pending removal now keeps full strength. Its LOWs are applied: the ring uses --hairline, and the check asserts that hover draws the button's own border color, inset. It found no clash with focus rings or other .btn box-shadows, confirmed the DESIGN.md wording against Composer.tsx and SettingsDialog.tsx, and found that the settings-test trim loses no behaviour check.
• bun run check fast (through dori heavy) on cf9d7a7: status: success, verification verified.

The pending-removal treatment follows T3 Code's outlined destructive button, as DESIGN.md already cites for PC removal: the warning stays fully legible while the action runs, and only the unrelated controls dim. No code is borrowed.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/sidebar-danger-design-followups
• File: .coderabbit.yaml
• Commit: cf9d7a7
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 65ea9315-cf2a-458e-8cfe-e2aac5618258

Commits

Reviewing files that changed from the base of the PR and between 4c46992 and cf9d7a7.

Files selected for processing: 5

• DESIGN.md
• scripts/sidebar-machine-removal-demo-regression.ts
• src/components/Machines.css
• src/lib/settings.test.ts
• src/styles.css

Files with no reviewable changes: 1

• src/lib/settings.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

• Bug Fixes

  • Danger buttons now keep their full opacity while a removal is pending, improving text readability. Other actions disabled during that time remain visibly dimmed.
  • Hovering over an enabled danger button now retains its danger tint and border while adding an inset outline.

• Design Updates

  • Design guidance clarifies touch-target sizing for SSH management buttons and when the composer’s New tab and Settings toggle should appear on different screen types and sizes.

Danger-button hover and pending-removal styling changed, with regression checks for presentation and palette contrast. Design documentation now describes touch-target sizing and New tab button visibility conditions. Settings tests retain sanitizer behavior checks.

Changes: UI presentation and responsive controls

Layer / File(s)	Summary
Danger-button styling and regression checks src/components/Machines.css, src/styles.css, scripts/sidebar-machine-removal-demo-regression.ts, DESIGN.md	Busy danger buttons now remain fully opaque. Hover adds an inset ring while retaining the danger tint and border. The regression script checks hover styling, pending-removal opacity, and danger contrast across matching palettes.
Responsive control specifications and settings checks DESIGN.md, src/lib/settings.test.ts	The design documentation specifies touch-target height for coarse-pointer SSH controls and limits the New tab button and its Settings toggle to mouse-driven windows wider than 640px. The settings test removes declaration assertions and retains sanitizer checks.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• devswha/herdr-web-ui#1585: The documentation updates and retained sanitizer checks match the issue’s New tab toggle and settings-test objectives.

Suggested reviewers: snowykr, yoonwoo-ha

Merge Risk: Minimal · up to cf9d7

The PR is mergeable with no identified functional or operational blocker.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 3 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the main changes: restored hover feedback for danger buttons and preserved readability during pending PC removal.
Linked Issues check		Scope: #1424, #1416 and #1585 are direct, open links. All coding requirements were checked against the whole-PR diff (merge base 3597f45 to head cf9d7a7). 1. Hover feedback [#1424 item 3; #1416 item …
Out of Scope Changes check		Every changed hunk maps to a linked issue or to its direct support: - src/styles.css: adds the inset hover ring to .btn-danger:hover:not(:disabled). This is #1424 item 3 and #1416 item 3. - src/compon…
Description check		The description follows the template's two sections, Change and Validation. The Change section states the problem and resulting behavior for each linked issue (#1424, #1416, #1585), and the Validation…

Full details: Linked Issues check

Explanation

Scope: #1424, #1416 and #1585 are direct, open links. All coding requirements were checked against the whole-PR diff (merge base 3597f45 to head cf9d7a7).

1. Hover feedback [#1424 item 3; #1416 item 3]. Verified in src/styles.css. Inside @media (hover: hover), .btn-danger:hover:not(:disabled) keeps var(--danger-tint) and border-color var(--status-blocked). It adds box-shadow: inset 0 0 0 var(--hairline) var(--status-blocked). The border therefore draws about twice as thick, and the danger color stays. This meets the issue's request for a stronger hover border that keeps the color.

2. Pending PC removal contrast [#1416 item 2; #1416 item 1]. Verified in src/components/Machines.css. The rule .machine-controls .btn-danger[aria-busy="true"] changes from opacity 0.85 to 1. The base value failed 4.5:1 in several themes, including Tokyo Night. The PR also replaces the DESIGN.md claim 'only lightly dimmed' (per the whole-PR summary). The issue allows either limiting 0.85 to a pending delete or correcting the DESIGN.md sentence. The PR takes both routes for the pending case. Per the PR description and the summary of scripts/sidebar-machine-removal-demo-regression.ts, the regression now measures every theme block of the mode and checks that the confirm button stays at opacity 1 while Rename dims. I did not re-run the script.

3. Coarse-pointer height and New tab wording [#1424 item 4; #1585]. The whole-PR summary states that DESIGN.md now gives touch-target height for one-line PC controls on coarse pointers. It also states that the New tab toggle is limited to mouse-driven windows wider than 640px, matching the composer query. I did not read the DESIGN.md hunk directly. This assessment relies on the whole-PR summary.

4. Settings test [#1585]. src/lib/settings.test.ts drops two declaration-restating assertions (DEFAULT_SETTINGS.composerNewTabButton and SETTING_SCOPE). It keeps the behavioural checks: default false, accepts true, rejects the string "true", and is not shared. This matches the issue.

5. Items already on main [#1424 items 1 and 2; #1416 items 1, 3 and 4 (per PR)]. At head, scripts/sidebar-machine-layout-demo-regression.ts groups buttons by vertical overlap, not by top edge alone. It also asserts a shared Rename row at 520px. The PR does not change this file. I accept these as existing implementation at the reviewed head, per the PR's statement that #1732 and #1422 fixed them. The opacity-equals-0.85 assertion named in #1416 item 4 was not re-verified in this review.

Unresolved: test results and screenshots are reported by the author. I did not run them. The DESIGN.md wording is assessed from the whole-PR summary only.

Full details: Out of Scope Changes check

Explanation

Every changed hunk maps to a linked issue or to its direct support:

• src/styles.css: adds the inset hover ring to .btn-danger:hover:not(:disabled). This is #1424 item 3 and #1416 item 3.
• src/components/Machines.css: changes the pending-removal opacity from 0.85 to 1 and updates its comment. This is #1416 items 1 and 2.
• scripts/sidebar-machine-removal-demo-regression.ts: adds hover, ring, pending-removal and per-theme contrast checks. This supports #1416 items 1, 2 and 4 and #1424 item 3.
• src/lib/settings.test.ts: removes two declaration-restating assertions. This is #1585.
• DESIGN.md (per whole-PR summary): updates the wording for #1424 item 4, #1416 item 1 and #1585.

No unrelated feature code, refactoring or dependency change appears in the summary. The layout regression script is not modified by this PR. The PR does not add unrelated behaviour.

Full details: Description check

Explanation

The description follows the template's two sections, Change and Validation. The Change section states the problem and resulting behavior for each linked issue (#1424, #1416, #1585), and the Validation section lists the checks run and their results. The author also explains which items were already fixed on main. The description is thorough, though it is long and it does not link a maintainer-agreed issue for the New tab feature beyond #1585 itself.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Merge pass: the code review passes and the PR is queued; it merges once its browser CI is green. One small request, which needs no push: the PR body gives the issue as the reason for the hover ring. Please add one sentence naming its design precedent, as AGENTS.md asks for a UI fix. DESIGN.md already cites T3 Code's outlined destructive button in that paragraph.

[devswha]

Done, with no push: the hover bullet in the PR body now names its design precedent. That is T3 Code's destructive-outline button, which DESIGN.md already cites in that paragraph: on hover it strengthens its border and tint in the destructive color (border-destructive/32, bg-destructive/4) instead of a neutral fill, and this ring does the same with herdr's danger tokens.

[devswha]

Merge pass review-work report. Overall: PASSED. The change is CSS, DESIGN.md and a regression only. Under the maintainer's token rule it merges on the code review, the merge pass's own check and CI, without the gate reviewer.

Lane	Verdict
Code review (omo-native-code-reviewer: programming, remove-ai-slops, frontend)	PASS (WATCH)
Merge pass's own check	settings and conventions tests pass (85); typecheck passes; git merge-tree is clean against main and against the 13 maintainer PRs that share DESIGN.md, plus #1851, which also shares Machines.css and styles.css

What was checked

• Theme blocks and generated files: none are touched. Only tokens are used: --hairline, --status-blocked and opacity: 1.
• Contrast regression: it blends the button over the panel's --bg-elevated, applies opacity, and walks every theme in both modes, 22 theme blocks in all. Text must reach at least 4.5:1 and the border at least 3:1. This lane ran on CI.
• Focus, hover and disabled states:
  • Focus (the global outline outside the button) and hover (a 1px ring inside it) look different.
  • A disabled button shows no ring.
  • The pending Remove PC keeps full strength through its more specific opacity: 1. A button disabled for another reason still dims.
• Design precedent: DESIGN.md cites T3 Code's outlined destructive button in the hover paragraph.

Follow-ups (LOW)

• The ring appears instantly while the rest of the button eases.
• The hover ring is asserted in the Machines panel only.

All required checks are green and no review thread is open.