Fleet UI: Update label target copies#43763
Conversation
![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.
RachelElysia
had a problem deploying
to
Docker Hub
GitHub Actions
Failure
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #43763 +/- ##
========================================
Coverage 66.91% 66.91%
========================================
Files 2600 2600
Lines 208978 208978
Branches 9342 9228 -114
========================================
Hits 139840 139840
Misses 56396 56396
Partials 12742 12742
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:
|
WalkthroughThis change updates the software deployment targeting UI with styling and help text improvements. The dropdown menu for custom target label selection now has constrained height limits to prevent excessive scrolling. Additionally, help text for the label matching conditions—"labels include any," "labels include all," and "labels exclude any"—has been reformatted as JSX with consistent phrasing and emphasized key terms, making the scope behavior clearer to users when configuring software deployments. 🚥 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.
🧹 Nitpick comments (1)
frontend/pages/SoftwarePage/helpers.tsx (1)
191-218: Inconsistent emphasis tag (<strong>vs<b>) within the same feature.The new
helpTextentries use<strong>, whilegenerateHelpTextjust above (lines 144–184) renders the same phrasing with<b>. Functionally equivalent here, but it's worth aligning on one tag across the label-target copy for consistency (and<strong>is generally preferred semantically).♻️ Optional alignment
- <b>have any</b> of these + <strong>have any</strong> of these(apply similarly to the
have all/don't have anyvariants ingenerateHelpText)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/SoftwarePage/helpers.tsx` around lines 191 - 218, The helpText JSX entries in the options array use <strong> while the generateHelpText function renders the same phrases with <b>, causing inconsistent emphasis tags; update either the array entries (values "labelsIncludeAny", "labelsIncludeAll", "labelsExcludeAny") to use <b> or change generateHelpText to use <strong> so both sources use the same tag (preferably <strong> per review), and ensure the variants for "have any", "have all", and "don't have any" are updated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/pages/SoftwarePage/helpers.tsx`:
- Around line 191-218: The helpText JSX entries in the options array use
<strong> while the generateHelpText function renders the same phrases with <b>,
causing inconsistent emphasis tags; update either the array entries (values
"labelsIncludeAny", "labelsIncludeAll", "labelsExcludeAny") to use <b> or change
generateHelpText to use <strong> so both sources use the same tag (preferably
<strong> per review), and ensure the variants for "have any", "have all", and
"don't have any" are updated consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2fc6e8b7-851d-4cda-9c06-ebc1bfc4ac74
📒 Files selected for processing (2)
frontend/components/TargetLabelSelector/_styles.scssfrontend/pages/SoftwarePage/helpers.tsx
There was a problem hiding this comment.
Pull request overview
Updates Fleet UI copy and dropdown styling for label-based custom targeting (Include any / Include all / Exclude any), aligning the targeting descriptions shown in the Software flows.
Changes:
- Expanded custom target option help text into full sentences with emphasized key phrases.
- Adjusted TargetLabelSelector dropdown menu max-heights to avoid scrolling with the longer option descriptions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/pages/SoftwarePage/helpers.tsx | Updates CUSTOM_TARGET_OPTIONS help text copy for Software label targeting. |
| frontend/components/TargetLabelSelector/_styles.scss | Increases react-select menu max-heights within TargetLabelSelector to better fit the longer option help text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| helpText: ( | ||
| <> | ||
| Software will only be available for install on hosts that{" "} | ||
| <strong>have any</strong> of these labels. | ||
| </> | ||
| ), |
There was a problem hiding this comment.
The new option helpText is very software-install specific ("available for install"), but CUSTOM_TARGET_OPTIONS is reused in other flows (e.g. auto-updates targeting) where the same targeting may not be primarily about installs/self-service. Consider making this copy context-neutral (e.g. "Targeted hosts are those that…") or moving this wording to a caller-provided dropdownHelpText so each flow can supply accurate language.
| // Hardcoded to fit all 3 custom target options (Exclude any, Include all, | ||
| // Include any) without scrolling |
There was a problem hiding this comment.
This comment says the dropdown is hardcoded to fit options in the order "Exclude any, Include all, Include any", but the actual options (and the visible dropdown order) are different. Please update the comment to match the current option labels/order to avoid confusion when someone revisits this later.
| // Hardcoded to fit all 3 custom target options (Exclude any, Include all, | |
| // Include any) without scrolling | |
| // Hardcoded to fit all 3 custom target options without scrolling |
| .Select-menu-outer { | ||
| max-height: 260px; | ||
| } | ||
|
|
||
| .Select-menu { | ||
| max-height: 244px; | ||
| } |
There was a problem hiding this comment.
Hardcoding max-height values here is brittle (option height can change with copy tweaks, font-size changes, or localization), and it may reintroduce scrolling/clipping later. Consider removing the fixed heights (e.g. setting max-height to none/unset) or basing the limit on viewport height (so it remains usable if option content grows).
Issue
Closes #39916
Description
Screenshot of change
Testing
Summary by CodeRabbit