Fleet UI: Improve internal links/buttons#43470
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43470 +/- ##
==========================================
- Coverage 66.91% 66.91% -0.01%
==========================================
Files 2596 2596
Lines 208101 208094 -7
Branches 9285 9171 -114
==========================================
- Hits 139241 139236 -5
+ Misses 56202 56200 -2
Partials 12658 12658
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Review Summary by QodoUnify internal link styling with Button variant="link"
WalkthroughsDescription• Unified button link styling by replacing variant="text-link" and variant="text-link-dark" with variant="link" • Removed CustomLink component's customClickHandler prop and replaced instances with `Button variant="link"` • Fixed animated underline positioning to prevent jumpiness on hover by adjusting bottom border margins • Simplified button styling to match CustomLink appearance with consistent animated underline behavior Diagramflowchart LR
A["text-link and text-link-dark variants"] -->|"consolidate to"| B["link variant"]
C["CustomLink with customClickHandler"] -->|"replace with"| D["Button variant=link"]
E["Animated underline styling"] -->|"fix positioning"| F["Remove margin-bottom jumpiness"]
B --> G["Consistent link appearance"]
D --> G
F --> G
File Changes1. frontend/components/buttons/Button/_styles.scss
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
WalkthroughThis PR refactors the Fleet UI's internal link styling by consolidating button link variants. It removes the "text-link" and "text-link-dark" button variants in favor of a single unified "link" variant, simplifies the CustomLink component by removing custom click handler functionality, and migrates multiple components across the frontend to use the new Button(variant="link") pattern. Supporting SCSS styling, focus handling, and Storybook stories are updated accordingly to reflect the new design system consolidation. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/buttons/Button/_styles.scss`:
- Around line 221-225: The .button--link block (&--link selector) violates
stylelint spacing by missing a blank line before the declaration at the start of
the rule; open the .button--link (the &--link selector) rule and insert a single
empty line before the first declaration (e.g., before the `@include` link line) so
the rule follows the project's stylelint spacing convention.
In
`@frontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsx`:
- Around line 143-155: In ConditionalAccessModal (JSX in
ConditionalAccessModal.tsx) the trailing period is rendered as a separate text
node causing an extra space before the period; update the JSX so the period is
directly adjacent to the preceding text (for both the CustomLink branch and the
non-link fragment) by moving the period into the same string/element as the
previous content (e.g., attach "." to the CustomLink text or include it inside
the fragment next to <b>Conditional access</b>) and remove the standalone " ."
node.
🪄 Autofix (Beta)
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
Run ID: d2d9e1d7-ca1c-4092-b121-6f66a5398cf4
📒 Files selected for processing (26)
changes/43342-improved-button-link-stylingfrontend/components/AddHostsModal/AddHostsModal.tsxfrontend/components/AddHostsModal/PlatformWrapper/PlatformWrapper.tsxfrontend/components/CustomLink/CustomLink.tsxfrontend/components/TableContainer/DataTable/ActionButton/_styles.scssfrontend/components/buttons/Button/Button.stories.tsxfrontend/components/buttons/Button/Button.tsxfrontend/components/buttons/Button/_styles.scssfrontend/components/buttons/DropdownButton/DropdownButton.stories.tsxfrontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tsxfrontend/pages/SoftwarePage/components/tables/HashCell/HashCell.tsxfrontend/pages/SoftwarePage/components/tables/InstalledPathCell/InstalledPathCell.tsxfrontend/pages/admin/TeamManagementPage/TeamDetailsWrapper/UsersPage/components/AddUsersModal/AddUsersModal.tsxfrontend/pages/admin/UserManagementPage/components/UserForm/_styles.scssfrontend/pages/admin/components/HostStatusWebhookPreviewModal/HostStatusWebhookPreviewModal.tsxfrontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsxfrontend/pages/hosts/details/DeviceUserPage/components/DeviceUserBanners/DeviceUserBanners.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/SelectQueryModal/SelectQueryModal.tsxfrontend/pages/hosts/details/cards/HostSummary/OSSettingsIndicator/OSSettingsIndicator.tsxfrontend/pages/hosts/details/cards/HostSummary/OSSettingsIndicator/_styles.scssfrontend/pages/hosts/details/cards/Software/InstallStatusCell/InstallStatusCell.tsxfrontend/pages/hosts/details/cards/Vitals/Vitals.tsxfrontend/pages/policies/ManagePoliciesPage/components/CalendarEventsModal/_styles.scssfrontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsxfrontend/styles/var/mixins.scss
💤 Files with no reviewable changes (2)
- frontend/pages/hosts/details/cards/HostSummary/OSSettingsIndicator/_styles.scss
- frontend/components/CustomLink/CustomLink.tsx
| &--link { | ||
| @include link; | ||
| @include animated-bottom-border($core-fleet-black, $ui-fleet-black-25); | ||
| background: transparent; | ||
| border: 0; |
There was a problem hiding this comment.
Fix stylelint spacing violation in .button--link.
At Line 224, stylelint expects an empty line before the declaration.
💡 Suggested fix
&--link {
`@include` link;
`@include` animated-bottom-border($core-fleet-black, $ui-fleet-black-25);
+
background: transparent;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &--link { | |
| @include link; | |
| @include animated-bottom-border($core-fleet-black, $ui-fleet-black-25); | |
| background: transparent; | |
| border: 0; | |
| &--link { | |
| `@include` link; | |
| `@include` animated-bottom-border($core-fleet-black, $ui-fleet-black-25); | |
| background: transparent; | |
| border: 0; |
🧰 Tools
🪛 Stylelint (17.6.0)
[error] 224-224: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/buttons/Button/_styles.scss` around lines 221 - 225, The
.button--link block (&--link selector) violates stylelint spacing by missing a
blank line before the declaration at the start of the rule; open the
.button--link (the &--link selector) rule and insert a single empty line before
the first declaration (e.g., before the `@include` link line) so the rule follows
the project's stylelint spacing convention.
| This can be configured in{" "} | ||
| {isGlobalAdmin ? ( | ||
| <CustomLink | ||
| url={PATHS.ADMIN_INTEGRATIONS_CONDITIONAL_ACCESS} | ||
| text="Settings > Integrations > Conditional access" | ||
| /> | ||
| ) : ( | ||
| <> | ||
| <b>Settings</b> > <b>Integrations</b> >{" "} | ||
| <b>Conditional access</b> | ||
| </> | ||
| )} | ||
| . |
There was a problem hiding this comment.
Remove the extra space before the trailing period.
At Line 155, the period is rendered in a separate text node, which can introduce Conditional access . in UI copy.
💡 Suggested fix
- )}
- .
+ )}.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This can be configured in{" "} | |
| {isGlobalAdmin ? ( | |
| <CustomLink | |
| url={PATHS.ADMIN_INTEGRATIONS_CONDITIONAL_ACCESS} | |
| text="Settings > Integrations > Conditional access" | |
| /> | |
| ) : ( | |
| <> | |
| <b>Settings</b> > <b>Integrations</b> >{" "} | |
| <b>Conditional access</b> | |
| </> | |
| )} | |
| . | |
| This can be configured in{" "} | |
| {isGlobalAdmin ? ( | |
| <CustomLink | |
| url={PATHS.ADMIN_INTEGRATIONS_CONDITIONAL_ACCESS} | |
| text="Settings > Integrations > Conditional access" | |
| /> | |
| ) : ( | |
| <> | |
| <b>Settings</b> > <b>Integrations</b> >{" "} | |
| <b>Conditional access</b> | |
| </> | |
| )}. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsx`
around lines 143 - 155, In ConditionalAccessModal (JSX in
ConditionalAccessModal.tsx) the trailing period is rendered as a separate text
node causing an extra space before the period; update the JSX so the period is
directly adjacent to the preceding text (for both the CustomLink branch and the
non-link fragment) by moving the period into the same string/element as the
previous content (e.g., attach "." to the CustomLink text or include it inside
the fragment next to <b>Conditional access</b>) and remove the standalone " ."
node.
There was a problem hiding this comment.
Pull request overview
This PR standardizes Fleet UI “link-like” interactions by consolidating link-styled buttons into a single Button variant and removing CustomLink’s click-handler role in favor of semantic <button> usage for modal triggers.
Changes:
- Replaces
Buttonvariantstext-link/text-link-darkwith a unifiedlinkvariant and updates related Storybook/options and SCSS selectors. - Removes
customClickHandlerbehavior fromCustomLink, migrating modal-opening “links” toButton variant="link". - Tweaks the shared underline/border animation mixins to adjust underline positioning.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/styles/var/mixins.scss | Adjust underline pseudo-element positioning and simplify animated underline behavior. |
| frontend/pages/policies/ManagePoliciesPage/components/ConditionalAccessModal/ConditionalAccessModal.tsx | Makes the settings path an internal link for global admins. |
| frontend/pages/policies/ManagePoliciesPage/components/CalendarEventsModal/_styles.scss | Updates selector to match new button variant class. |
| frontend/pages/hosts/details/cards/Vitals/Vitals.tsx | Replaces CustomLink-driven modal triggers with Button variant="link". |
| frontend/pages/hosts/details/cards/Software/InstallStatusCell/InstallStatusCell.tsx | Migrates text-link variant to link. |
| frontend/pages/hosts/details/cards/HostSummary/OSSettingsIndicator/_styles.scss | Removes now-unneeded button-specific styling. |
| frontend/pages/hosts/details/cards/HostSummary/OSSettingsIndicator/OSSettingsIndicator.tsx | Replaces CustomLink click handling with Button variant="link". |
| frontend/pages/hosts/details/HostDetailsPage/modals/SelectQueryModal/SelectQueryModal.tsx | Migrates text-link variant to link. |
| frontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsx | Updates rotate action styling to inverse variant. |
| frontend/pages/hosts/details/DeviceUserPage/components/DeviceUserBanners/DeviceUserBanners.tsx | Migrates text-link-dark usage to link. |
| frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx | Replaces a CustomLink modal trigger with Button variant="link". |
| frontend/pages/admin/components/HostStatusWebhookPreviewModal/HostStatusWebhookPreviewModal.tsx | Normalizes apostrophe usage in preview text. |
| frontend/pages/admin/UserManagementPage/components/UserForm/_styles.scss | Updates selector to match new button variant class. |
| frontend/pages/admin/TeamManagementPage/TeamDetailsWrapper/UsersPage/components/AddUsersModal/AddUsersModal.tsx | Migrates text-link to link for “Create a user” CTA. |
| frontend/pages/SoftwarePage/components/tables/InstalledPathCell/InstalledPathCell.tsx | Migrates text-link variant to link. |
| frontend/pages/SoftwarePage/components/tables/HashCell/HashCell.tsx | Migrates text-link variant to link. |
| frontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tsx | Migrates text-link variant to link for list item title styling. |
| frontend/components/buttons/DropdownButton/DropdownButton.stories.tsx | Updates Storybook control options to use link. |
| frontend/components/buttons/Button/_styles.scss | Removes old link variants and defines new button--link styling using shared link mixins. |
| frontend/components/buttons/Button/Button.tsx | Updates variant type union and onWhite logic for spinner styling. |
| frontend/components/buttons/Button/Button.stories.tsx | Updates stories to use new link variant. |
| frontend/components/TableContainer/DataTable/ActionButton/_styles.scss | Updates selector to match new button variant class. |
| frontend/components/CustomLink/CustomLink.tsx | Removes customClickHandler and related key handling; keeps link as navigation-only. |
| frontend/components/AddHostsModal/PlatformWrapper/PlatformWrapper.tsx | Replaces “click here” with clearer text and uses Button variant="link". |
| frontend/components/AddHostsModal/AddHostsModal.tsx | Replaces CustomLink modal trigger with Button variant="link". |
| changes/43342-improved-button-link-styling | Adds release note entry for the UI change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Button | ||
| variant="link" | ||
| onClick={onClick} | ||
| className={`${baseClass}__button`} | ||
| /> | ||
| > | ||
| {displayStatus} | ||
| </Button> |
There was a problem hiding this comment.
className={${baseClass}__button} is still applied to the new Button but the corresponding SCSS block for &__button was removed in this PR. Since this class no longer appears to be referenced anywhere, consider removing the className to avoid leaving behind a dead styling hook (or reintroduce styles if it’s still needed).
There was a problem hiding this comment.
ok bot, will followup
Issue
Closes #43342
Description
Screenrecording
Screen.Recording.2026-04-13.at.2.07.10.PM.mov
Screen.Recording.2026-04-13.at.2.06.19.PM.mov
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Release Notes
Style
Bug Fixes