Fleet UI: Dropdown button options don't stay highlighted - #46740
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #46740 +/- ##
==========================================
- Coverage 66.93% 66.93% -0.01%
==========================================
Files 2831 2833 +2
Lines 224806 224803 -3
Branches 11664 11656 -8
==========================================
- Hits 150473 150470 -3
- Misses 60682 60683 +1
+ Partials 13651 13650 -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
Fixes the “Actions” (button-variant) DropdownWrapper behavior where a previously-clicked menu item remains highlighted when reopening the dropdown (reported on Software title details), while preserving keyboard-visible focus styling for accessibility.
Changes:
- Prevent
react-selectfrom persisting a selected option forvariant="button"by forcing the controlledvaluetonull. - Remove
state.isFocused-driven trigger highlighting for the button variant to avoid “stuck” highlighted trigger states after close. - Add a button-variant CSS focus-visible treatment and a regression test for the previously-selected highlight.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tsx | Clears button-variant selection state and avoids focus-based trigger highlighting that can persist after close. |
| frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tests.tsx | Adds regression coverage for ensuring no option is marked selected after reopen (needs a small robustness tweak). |
| frontend/components/forms/fields/DropdownWrapper/_styles.scss | Adds keyboard focus-visible background styling for the button variant control. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR fixes a bug in the DropdownWrapper component's button variant (used by the Actions dropdown) where previously-clicked menu items remained highlighted after the dropdown was reopened. The fix involves three coordinated changes: removing focus-state styling from the button variant, returning null as the selected value to prevent react-select from re-selecting the last item, and adding a button-specific CSS class with corresponding SCSS styling. A regression test validates that the 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.
🧹 Nitpick comments (1)
frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tests.tsx (1)
152-159: ⚡ Quick winAssert options are actually rendered to avoid a vacuous pass.
If the reopened menu renders no
.react-select__optionelements (e.g. menu didn't reopen),forEachruns zero assertions and the test passes without verifying anything. Add a length check so a regression in reopening behavior is also caught.💚 Proposed guard
- document - .querySelectorAll(".react-select__option") - .forEach((option) => - expect(option.className).not.toMatch(/--is-selected/) - ); + const reopenedOptions = document.querySelectorAll(".react-select__option"); + expect(reopenedOptions.length).toBe(sampleOptions.length); + reopenedOptions.forEach((option) => + expect(option.className).not.toMatch(/--is-selected/) + );🤖 Prompt for 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. In `@frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tests.tsx` around lines 152 - 159, The test currently checks that reopened menu options do not have the `--is-selected` class by iterating over document.querySelectorAll(".react-select__option") but it can vacuously pass when no options are rendered; update the test in DropdownWrapper.tests.tsx (the block that queries ".react-select__option") to first assert that the NodeList length is greater than zero (e.g., expect(options.length).toBeGreaterThan(0)) before running the per-option class assertions so a regression where the menu fails to reopen will fail the test.
🤖 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.
Nitpick comments:
In `@frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tests.tsx`:
- Around line 152-159: The test currently checks that reopened menu options do
not have the `--is-selected` class by iterating over
document.querySelectorAll(".react-select__option") but it can vacuously pass
when no options are rendered; update the test in DropdownWrapper.tests.tsx (the
block that queries ".react-select__option") to first assert that the NodeList
length is greater than zero (e.g., expect(options.length).toBeGreaterThan(0))
before running the per-option class assertions so a regression where the menu
fails to reopen will fail the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 558f3ebd-f0c1-4ec3-86ef-c8afd974358f
📒 Files selected for processing (3)
frontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tests.tsxfrontend/components/forms/fields/DropdownWrapper/DropdownWrapper.tsxfrontend/components/forms/fields/DropdownWrapper/_styles.scss
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Issue
Closes #45853
Description
Screenrecording of fix
Screen.Recording.2026-06-03.at.12.16.40.PM.mov
Related issue: Resolves #
Testing
Summary by CodeRabbit