Skip to content

Fleet UI: Fix undersized icons on default-size buttons - #51015

Merged
RachelElysia merged 5 commits into
mainfrom
34377-icon-size
Aug 12, 2026
Merged

Fleet UI: Fix undersized icons on default-size buttons#51015
RachelElysia merged 5 commits into
mainfrom
34377-icon-size

Conversation

@RachelElysia

@RachelElysia RachelElysia commented Aug 11, 2026

Copy link
Copy Markdown
Member

Issue

Resolves #34377.

Description

Follow-up to #50195 — QA originally tagged only HostHeader.tsx (Host details Refetch button), but sweeping for the same pattern turned up three more sites still on the legacy <Icon> child inside <Button>. All four migrated to icon="..." on the Button, which auto-sizes to 16x16 on default-size buttons and picks up the variant's default text color.

  • HostHeader.tsx Refetch button — the site QA reported. Had color="ui-fleet-black-75" which looked custom but IS the secondary variant's default, so Fleet UI: Button icon padding correction #50195's earlier "keep child pattern for custom colors" carve-out skipped it for a false-positive reason. Now renders 16x16 per spec.
  • Vitals.tsx custom-vital edit and SelfServiceCategoriesPage.tsx edit/delete row actions — icon-only children with no custom color at all; just weren't migrated in the earlier sweep.
  • InfoBanner.tsx close button — had color="core-fleet-black" (darker than the standard ui-fleet-black-75 used everywhere else on subdued) and size="small". Both were divergences from the design system; the new prop pattern reads the correct color and size from the variant. The __close positioning className moves from the Icon to the Button, where it semantically belongs (it's a negative margin to offset button padding). ariaLabel="Close" added since the button is now icon-only.

Also added a terse rule to .claude/rules/fleet-frontend.md covering the icon prop pattern — the child pattern is not allowed anywhere, no exceptions for color or className. This is the second batch of misses since #50195 landed, so tightening the guidance closes the door on further drift.

Screenrecording

Testing

  • Existing InfoBanner, HostHeader, Vitals, and SelfServiceCategoriesPage tests continue to pass.

  • Added/updated automated tests

  • QA'd all new/changed functionality manually

Migrate three call sites from the legacy `<Icon>` child pattern to the
Button `icon` prop, which auto-sizes correctly per the design spec (16x16
on default-size buttons, 12x12 on size="small").

- Host details Refetch button was rendering the refresh icon at 12x12
  (`<Icon size="small">`) on a default-size button — spec says 16x16.
  Custom `color="ui-fleet-black-75"` is redundant since it's the secondary
  variant's default text color.
- Self-service categories edit/delete row actions were icon-only children.
- Custom-vital edit button on Vitals card matched the correct size but was
  duplicating what Button now handles automatically.

Resolves #34377.
The close-X icon was on the legacy child pattern with color="core-fleet-black"
and size="small". Both were divergences from the design system defaults —
size="small" (12px) on a default-size button contradicted the 16x16 spec,
and the darker color was inconsistent with the ui-fleet-black-75 the subdued
variant uses everywhere else. Migrating to icon="close" picks up both the
correct 16x16 sizing and the standard subdued color automatically. The
__close class (negative margin to offset button padding) moves from the
Icon to the Button, where it semantically belongs.
@RachelElysia
RachelElysia requested a review from a team as a code owner August 11, 2026 21:27
Copilot AI lite review requested due to automatic review settings August 11, 2026 21:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR finishes migrating a few remaining Fleet UI Button call sites off the legacy <Button><Icon ... /></Button> child pattern to the newer icon="..." prop so default-size buttons render icons at the design-system size/color.

Changes:

  • Updated icon-only row-action buttons in SelfServiceCategoriesPage to use icon="pencil" / icon="trash".
  • Updated the custom vital edit button in Vitals to use icon="pencil" (icon sizing now derives from the Button size).
  • Updated HostHeader refetch and InfoBanner close buttons to use the Button icon prop (and moved close-button positioning className onto the Button).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
frontend/pages/SoftwarePage/SoftwareLibrary/SelfServiceCategoriesPage/SelfServiceCategoriesPage.tsx Migrates edit/delete row action buttons to the icon prop for correct default sizing/color.
frontend/pages/hosts/details/cards/Vitals/Vitals.tsx Migrates the custom vital edit button to the icon prop so it inherits icon sizing from size="small".
frontend/pages/hosts/details/cards/HostHeader/HostHeader.tsx Migrates the refetch button icon to the icon prop to render 16×16 and inherit secondary variant color.
frontend/components/InfoBanner/InfoBanner.tsx Migrates the close control to an icon-only Button using the icon prop and moves positioning className onto the Button.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +69 to +74
<Button
variant="subdued"
icon="close"
onClick={() => setHideBanner(true)}
className={`${baseClass}__close`}
/>
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Updated four UI areas to use the Button component’s icon prop instead of nested Icon elements. The changes cover the closable information banner, self-service category edit and delete controls, the host header refresh control, and custom host vital edit controls. The host header also removes its unused Icon import.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #34377 by migrating four button icon usages to the design-system prop and restoring consistent icon sizing and colors.
Out of Scope Changes check ✅ Passed All changes are limited to the four button icon migrations and the related InfoBanner accessibility update described in the objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly identifies the main change: fixing undersized icons on default-size buttons.
Description check ✅ Passed The description includes the issue, scope, affected components, design-system requirements, accessibility change, and testing status.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 34377-icon-size

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@frontend/components/InfoBanner/InfoBanner.tsx`:
- Around line 69-74: Add an accessible name to the icon-only Button in
InfoBanner by supplying ariaLabel="Close" or the component’s localized
equivalent, while preserving the existing setHideBanner(true) behavior.
🪄 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: 2b4396c7-5953-4c66-9227-82a9164e9d5a

📥 Commits

Reviewing files that changed from the base of the PR and between dd1080f and 7c63a21.

📒 Files selected for processing (4)
  • frontend/components/InfoBanner/InfoBanner.tsx
  • frontend/pages/SoftwarePage/SoftwareLibrary/SelfServiceCategoriesPage/SelfServiceCategoriesPage.tsx
  • frontend/pages/hosts/details/cards/HostHeader/HostHeader.tsx
  • frontend/pages/hosts/details/cards/Vitals/Vitals.tsx

Comment thread frontend/components/InfoBanner/InfoBanner.tsx
cdcme
cdcme previously approved these changes Aug 11, 2026
@RachelElysia

Copy link
Copy Markdown
Member Author

@cdcme - sorry, added a claude rule to avoid

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.56%. Comparing base (dd1080f) to head (0b7cce3).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #51015   +/-   ##
=======================================
  Coverage   68.56%   68.56%           
=======================================
  Files        3985     3985           
  Lines      256691   256692    +1     
  Branches    13671    13671           
=======================================
+ Hits       175991   175996    +5     
+ Misses      65046    65042    -4     
  Partials    15654    15654           
Flag Coverage Δ
frontend 62.75% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RachelElysia
RachelElysia merged commit 1270939 into main Aug 12, 2026
27 checks passed
@RachelElysia
RachelElysia deleted the 34377-icon-size branch August 12, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI Improvement: Button icon padding correction

3 participants