Fleet UI: Fix dataset text alignment - #44659
Conversation
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44659 +/- ##
========================================
Coverage 66.68% 66.68%
========================================
Files 2651 2651
Lines 213493 213494 +1
Branches 9610 9732 +122
========================================
+ Hits 142367 142368 +1
+ Misses 58160 58159 -1
- Partials 12966 12967 +1
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:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to resolve subtle text baseline/descender clipping and misalignment issues across the Fleet UI (notably on vulnerability pages) by standardizing line-height usage and improving baseline alignment for “dataset” rows and tooltip-underlined text.
Changes:
- Introduces a
textOnlyoption onDataSetto align values by text baseline when appropriate, and applies it in vulnerability and host/detail views. - Standardizes many component/page line-heights to use the shared
$line-heighttoken and removes several tooltip underline “position” overrides. - Updates tooltip underline rendering from
border-bottomtotext-decorationand removes the Firefox-only alignment hack.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/styles/var/fonts.scss | Intended to centralize $line-height, but currently introduces a self-referential Sass variable assignment. |
| frontend/styles/global/_global.scss | Uses $line-height for body and paragraph defaults instead of hardcoded values. |
| frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/SoftwareVulnSummary.tsx | Applies textOnly to relevant DataSets and simplifies tooltip wrapper markup. |
| frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/_styles.scss | Removes fixed heights/tooltip tweaks and aligns the flex row on baseline; uses $line-height. |
| frontend/pages/SoftwarePage/components/cards/DetailsNoHosts/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/queries/ManageQueriesPage/_styles.scss | Removes tooltip underline positioning override. |
| frontend/pages/queries/live/LiveQueryPage/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/queries/details/QueryDetailsPage/_styles.scss | Removes tooltip underline positioning override. |
| frontend/pages/policies/live/LivePolicyPage/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/labels/components/LabelForm/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/hosts/ManageHostsPage/components/CustomLabelGroupHeading/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/hosts/details/HostReportsTab/HostReportCard.tsx | Applies textOnly to DataSet values in the report card list. |
| frontend/pages/hosts/details/DeviceUserPage/components/SettingUpYourDevice/_styles.scss | Switches line-height to $line-height. |
| frontend/pages/hosts/details/components/InventoryVersions/InventoryVersions.tsx | Applies textOnly to multiple DataSets for baseline-aligned version metadata. |
| frontend/pages/DashboardPage/cards/ActivityFeed/components/ActivityTypeDropdown/_styles.scss | Cleans whitespace and switches search input line-height to $line-height. |
| frontend/pages/admin/OrgSettingsPage/_styles.scss | Switches banner line-height to $line-height. |
| frontend/components/TooltipWrapper/_styles.scss | Uses inherited line-height; changes underline to text-decoration and removes Firefox-only hack. |
| frontend/components/side_panels/QuerySidePanel/QueryTableNotes/_styles.scss | Switches list item line-height to $line-height. |
| frontend/components/side_panels/QuerySidePanel/QueryTableColumns/ColumnListItem/_styles.scss | Removes redundant tooltip underline styling now handled by TooltipWrapper. |
| frontend/components/Modal/_styles.scss | Switches modal content line-height to $line-height. |
| frontend/components/IconStatusMessage/_styles.scss | Switches line-height to $line-height. |
| frontend/components/forms/fields/InputField/_styles.scss | Switches input line-height to $line-height, but introduces an undefined $line-height7 usage. |
| frontend/components/EmptyState/_styles.scss | Switches line-heights to $line-height. |
| frontend/components/DeviceUserError/_styles.scss | Switches line-height to $line-height. |
| frontend/components/DataSet/DataSet.tsx | Adds textOnly prop and corresponding modifier class. |
| frontend/components/DataSet/_styles.scss | Aligns dataset titles by baseline and aligns text-only values by baseline. |
| frontend/components/buttons/Button/_styles.scss | Uses $line-height for link-style button line-height. |
| changes/43688-fix-text-alignment-issues | Adds changelog entry for the UI alignment fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| font-size: $x-small; | ||
| font-weight: $regular; | ||
| line-height: 1.57; | ||
| line-height: $line-height7; |
WalkthroughAdds a 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/styles/global/_global.scss (1)
162-166:⚠️ Potential issue | 🟠 MajorRemove the negative margin compensation for the tooltip underline, which is now unnecessary after switching to
text-decoration.The
component__tooltip-wrapper__underlinestyling now usestext-decoration: underline dashedwithtext-underline-offset: 3px(infrontend/components/TooltipWrapper/_styles.scss), which does not add layout height. The compensation in the form field label styles (margin-top: -2.5pxandpadding-bottom: 0atfrontend/styles/global/_global.scsslines 163-166) was written to offset height added byborder-bottom. Sincetext-decorationrenders within the text box without affecting layout, this negative margin now pushes form-field labels up 2.5px unnecessarily and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/styles/global/_global.scss` around lines 162 - 166, Update the .component__tooltip-wrapper__underline rule to remove the layout compensation that is no longer needed: delete the margin-top: -2.5px and padding-bottom: 0 declarations so the label is not nudged upward; this change targets the selector .component__tooltip-wrapper__underline in frontend/styles/global/_global.scss and aligns it with the text-decoration-based underline in TooltipWrapper/_styles.scss (which uses text-decoration and text-underline-offset).frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/_styles.scss (1)
20-29:⚠️ Potential issue | 🟠 Major
display: flexis missing — all flex properties in this block are inert without it.The
&__description-listrule is missingdisplay: flex, which is required for theflex-direction,gap,flex-wrap, andalign-itemsproperties to take effect. Without it, the element defaults todisplay: block, rendering flex properties as no-ops.A similar component in the same codebase (
SoftwareDetailsSummary/_styles.scss) has the identical selector withdisplay: flexexplicitly declared, confirming this is the intended behavior.Add the missing property:
🛡️ Fix
&__description-list { + display: flex; flex-direction: row; gap: 24px 40px; flex-wrap: wrap; align-items: baseline;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/_styles.scss` around lines 20 - 29, The selector &__description-list is missing display: flex so its flex properties (flex-direction, gap, flex-wrap, align-items) are inert; update the SCSS by adding display: flex to the &__description-list rule (preserving the existing .tooltip__tooltip-text white-space: pre-wrap) — mirror the same declaration used in the similar SoftwareDetailsSummary/_styles.scss to ensure consistent layout.
🤖 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/forms/fields/InputField/_styles.scss`:
- Line 82: The SCSS uses an undefined variable $line-height7 causing build
failure; update the declaration in InputField styles to use the correct variable
$line-height (or replace with the intended numeric value 1.57 if a unique value
is required) so the selector that currently references $line-height7 compiles
successfully.
In `@frontend/styles/var/fonts.scss`:
- Line 13: The SCSS has a self-referential assignment "$line-height:
$line-height;" which breaks compilation; replace this with a concrete value or a
valid design token (e.g., "$line-height: 1.5;" or "$line-height:
$base-line-height;") so all components (buttons, modals, forms) that rely on
$line-height resolve correctly—update the variable definition where $line-height
is declared and ensure any referenced token like $base-line-height exists and is
exported.
---
Outside diff comments:
In
`@frontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/_styles.scss`:
- Around line 20-29: The selector &__description-list is missing display: flex
so its flex properties (flex-direction, gap, flex-wrap, align-items) are inert;
update the SCSS by adding display: flex to the &__description-list rule
(preserving the existing .tooltip__tooltip-text white-space: pre-wrap) — mirror
the same declaration used in the similar SoftwareDetailsSummary/_styles.scss to
ensure consistent layout.
In `@frontend/styles/global/_global.scss`:
- Around line 162-166: Update the .component__tooltip-wrapper__underline rule to
remove the layout compensation that is no longer needed: delete the margin-top:
-2.5px and padding-bottom: 0 declarations so the label is not nudged upward;
this change targets the selector .component__tooltip-wrapper__underline in
frontend/styles/global/_global.scss and aligns it with the text-decoration-based
underline in TooltipWrapper/_styles.scss (which uses text-decoration and
text-underline-offset).
🪄 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: 34a9dda9-549c-4d9b-9aab-23cc5cd3a09e
📒 Files selected for processing (28)
changes/43688-fix-text-alignment-issuesfrontend/components/DataSet/DataSet.tsxfrontend/components/DataSet/_styles.scssfrontend/components/DeviceUserError/_styles.scssfrontend/components/EmptyState/_styles.scssfrontend/components/IconStatusMessage/_styles.scssfrontend/components/Modal/_styles.scssfrontend/components/TooltipWrapper/_styles.scssfrontend/components/buttons/Button/_styles.scssfrontend/components/forms/fields/InputField/_styles.scssfrontend/components/side_panels/QuerySidePanel/QueryTableColumns/ColumnListItem/_styles.scssfrontend/components/side_panels/QuerySidePanel/QueryTableNotes/_styles.scssfrontend/pages/DashboardPage/cards/ActivityFeed/components/ActivityTypeDropdown/_styles.scssfrontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/SoftwareVulnSummary.tsxfrontend/pages/SoftwarePage/SoftwareVulnerabilityDetailsPage/SoftwareVulnSummary/_styles.scssfrontend/pages/SoftwarePage/components/cards/DetailsNoHosts/_styles.scssfrontend/pages/admin/OrgSettingsPage/_styles.scssfrontend/pages/hosts/ManageHostsPage/components/CustomLabelGroupHeading/_styles.scssfrontend/pages/hosts/details/DeviceUserPage/components/SettingUpYourDevice/_styles.scssfrontend/pages/hosts/details/HostReportsTab/HostReportCard.tsxfrontend/pages/hosts/details/components/InventoryVersions/InventoryVersions.tsxfrontend/pages/labels/components/LabelForm/_styles.scssfrontend/pages/policies/live/LivePolicyPage/_styles.scssfrontend/pages/queries/ManageQueriesPage/_styles.scssfrontend/pages/queries/details/QueryDetailsPage/_styles.scssfrontend/pages/queries/live/LiveQueryPage/_styles.scssfrontend/styles/global/_global.scssfrontend/styles/var/fonts.scss
💤 Files with no reviewable changes (3)
- frontend/pages/queries/ManageQueriesPage/_styles.scss
- frontend/pages/queries/details/QueryDetailsPage/_styles.scss
- frontend/components/side_panels/QuerySidePanel/QueryTableColumns/ColumnListItem/_styles.scss
|
@cdcme I had to revert the tooltip underline approach since your last review (Firefox couldn't render text-decoration: dashed), should be good now! I screenshot/screenrecorded 3 different browsers and it looks good. |
Issue
Closes #43688
Summary by CodeRabbit
Bug Fixes
Style
Screenrecording and screenshots
Chrome screen recording
https://fleetdm.zoom.us/clips/share/qr3cY0NHTGuYqCX-vHWY3w
Safari screenshots
Firefox screenshots
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