ACM-31937 Node pool dropdown has misaligned selection indicator and clear button#6091
ACM-31937 Node pool dropdown has misaligned selection indicator and clear button#6091oksanabaza wants to merge 1 commit intostolostron:mainfrom
Conversation
…lacing with AcmSelectBase Signed-off-by: Oksana Bazylieva <obazylie@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oksanabaza The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThis PR refactors combobox components and updates their test suites to support creatable (user-typed) values and improved input commit handling. Core changes include new ChangesCombobox Creatable & Commit Flow Enhancement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/routes/Home/Overview/OverviewClusterLabelSelector.test.tsx (1)
75-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIndex-based
getAllByTextchip assertions are brittle.
getAllByText('Amazon')[1]hard-codes the assumption that exactly two'Amazon'nodes exist in the DOM at assertion time (e.g., one in the dropdown trigger and one in the chip). IfAcmSelectBase(the new PF6 control) does not surface the selected value in the trigger button, there will only be one node, and[1]will beundefined—expect(undefined).toBeTruthy()throws rather than passing, making the failure message misleading. Conversely, if more nodes exist the index shifts silently.Prefer scoping the assertion to the chip container:
♻️ Suggested fix
- await waitFor(() => expect(getAllByText('cloud')[0]).toBeTruthy()) - await waitFor(() => expect(getAllByText('Amazon')[1]).toBeTruthy()) + // Chip group labels render inside a labelled region; scope assertions there + await waitFor(() => expect(screen.getByRole('group', { name: 'cloud' })).toBeTruthy()) + await waitFor(() => expect(screen.getByText('Amazon', { selector: '.pf-v6-c-chip__text, [class*="chipText"]' })).toBeTruthy())Or, more portably, assert via chip role/label without relying on DOM order:
- await waitFor(() => expect(getAllByText('cloud')[0]).toBeTruthy()) - await waitFor(() => expect(getAllByText('Amazon')[1]).toBeTruthy()) + await waitFor(() => expect(screen.getAllByText('cloud').length).toBeGreaterThanOrEqual(1)) + await waitFor(() => expect(screen.getAllByText('Amazon').length).toBeGreaterThanOrEqual(1))🤖 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/src/routes/Home/Overview/OverviewClusterLabelSelector.test.tsx` around lines 75 - 76, The assertions using index-based getAllByText (e.g., getAllByText('Amazon')[1] and getAllByText('cloud')[0]) are brittle; change them to scope the query to the chip container or assert by role/label instead: locate the chip container rendered by the component (or add/use an existing test id/class) and replace getAllByText(...)[i] with within(chipContainer).getByText('Amazon') (or use getByRole/getByLabelText for the chip/button), so the test no longer depends on DOM order or whether AcmSelectBase surfaces the selected value in the trigger. Ensure you update the test to reference the chip container variable and remove index-based array accesses.
🧹 Nitpick comments (10)
frontend/src/routes/Applications/CreateSubscriptionApplication/SubscriptionApplication.test.tsx (1)
422-426: ⚡ Quick winExtract
commitComboBoxByTestIdtolib/test-util.tsto eliminate duplication across three test files.The helper is defined identically in
CreateCluster.test.tsx,CreateClusterPool.test.tsx, andSubscriptionApplication.test.tsx. Extract once to prevent divergence and improve maintainability.♻️ Suggested extraction
In
frontend/src/lib/test-util.ts, add:+export const commitComboBoxByTestId = async (id: string, value: string): Promise<void> => { + const input = await screen.findByTestId(id) + fireEvent.change(input, { target: { value } }) + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter', charCode: 13 }) +}Then remove the local definition from each test file and import from
'../../../lib/test-util'(adjust path per file).🤖 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/src/routes/Applications/CreateSubscriptionApplication/SubscriptionApplication.test.tsx` around lines 422 - 426, Extract the duplicate commitComboBoxByTestId helper into a shared module: create frontend/src/lib/test-util.ts exporting an async function named commitComboBoxByTestId(id: string, value: string) that performs screen.findByTestId, fireEvent.change and fireEvent.keyDown as in the diff; then remove the local commitComboBoxByTestId definitions from CreateCluster.test.tsx, CreateClusterPool.test.tsx and SubscriptionApplication.test.tsx and import the helper (e.g. import { commitComboBoxByTestId } from '../../../lib/test-util') in each test file so all three tests use the single shared implementation.frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js (3)
115-117: ⚡ Quick win
screen.getAllByRole('combobox')+.find(el => el.type === 'text')is fragile.
MenuToggleitself hasrole="combobox"(seeAcmSelectBase.tsxLine 642), sogetAllByRole('combobox')returns both the toggle and the input. The fallbackinputs[0]may pick the toggle, anduserEvent.typeon the toggle won't filter options. Preferscreen.getByTestId('masterType')(the input now carriesdata-testid={controlId}viainputProps) for an unambiguous target.🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js` around lines 115 - 117, Replace the fragile selector that calls screen.getAllByRole('combobox') and then finds by element.type with a deterministic test-id lookup: stop using inputs.find((el) => el.type === 'text') || inputs[0] and instead use screen.getByTestId('masterType') (the input receives data-testid via inputProps/controlId) as the target for userEvent.type; this avoids hitting the MenuToggle role="combobox" element from AcmSelectBase and ensures userEvent.type is invoked on the actual text input.
189-214: 💤 Low valueAsserting on a mutated
control.activecouples tests to internal mutation.
expect(control.active).toBe('test-path')works only becauseControlPanelComboBox.onChangemutates the prop object directly (Line 46 inControlPanelComboBox.js). That's the implementation reality, but the test's intent — "Enter commits the typed value" — is more robustly expressed viahandleControlChangearguments / call order, not mutation side-effects on the input object. Consider asserting viahandleChangeinvocation with the expected value, which would survive a future refactor that stops mutatingcontrolin place.As per coding guidelines, "Verify test coverage is meaningful, not just for metrics" and "Check that tests actually test the described behavior."
🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js` around lines 189 - 214, The test currently asserts on mutated control.active (coupling to internal mutation); instead assert that ControlPanelComboBox's onChange invoked the provided handleControlChange with the committed value — replace the control.active expectation with assertions on the mock handleChange (e.g. that handleChange was called and that one of its calls includes an object containing id: 'githubPath' and active: 'test-path' or the expected payload shape), referencing ControlPanelComboBox and its onChange/handleControlChange behavior so the spec verifies the outward contract rather than in-place mutation.
125-135: ⚡ Quick win
'clears selection'does not open the dropdown but still finds a clear button — verify it's the right one.
screen.getByRole('button', { name: /clear/i })is a loose regex; it can match anything whose accessible name contains "clear" (e.g. menu items in other tests). Here it works becausepropsPlain.control.active === 'm5.xlarge'causes the clear-input button (aria-label="Clear input value") to render, but the regex isn't anchored. Tighten toname: /clear input value/ito ensure the assertion targets the intended control and won't falsely match a future addition.🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js` around lines 125 - 135, The test "'clears selection'" in ControlPanelComboBox.test.js is using a loose matcher for the clear button; update the call to screen.getByRole in that test to target the specific clear-input control by using the exact accessible name (e.g. change the matcher from /clear/i to /clear input value/i) so it reliably selects the Clear input value button rendered by ControlPanelComboBox (propsPlain, control.active) and avoids accidentally matching unrelated menu items; keep the rest of the test (render, userEvent.click, waitFor expect(handleChange)) the same.frontend/src/components/AcmSelectBase.tsx (1)
296-303: 💤 Low value
commitTypeaheadInputuses staleinputValueas fallback.Callers always pass
event.currentTarget.value, so the?? inputValuebranch is effectively unreachable on the current call sites. Either makevaluerequired to remove the dead fallback, or document when the fallback applies. As-is, a future caller callingcommitTypeaheadInput()with no argument could read a staleinputValue(closure captures it viauseCallbackdeps, but only one render behind in racey scenarios).🤖 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/src/components/AcmSelectBase.tsx` around lines 296 - 303, commitTypeaheadInput currently falls back to a potentially-stale inputValue, which is dead code for current callers; make the value parameter required and remove the fallback and inputValue capture: change commitTypeaheadInput to (value: string) => { if (!onTypeaheadInputCommit) return; onTypeaheadInputCommit(value); closeMenu(); }, remove inputValue from the useCallback dependencies, and update all callers to always pass event.currentTarget.value; alternatively, if you need a no-arg call path keep the fallback but replace inputValue with a mutable ref (e.g., inputValueRef.current) to avoid stale closures.frontend/src/routes/Infrastructure/Clusters/ManagedClusters/CreateCluster/CreateCluster.test.tsx (2)
3508-3511: 💤 Low valueBrittle PatternFly v6 class selector.
Targeting the clear button via
inputElement.closest('.pf-v6-c-menu-toggle')couples the test to PatternFly's internal v6 class name. Any PF major bump silently breaks these tests with a confusingnull.getByRoleerror (the castas HTMLElementwill not catch a nullclosestresult). Prefer scoping by a stable selector — e.g., scopingwithinto the toggle button found viagetByRole('button', { name: /menu toggle/i })or to the container of the input'sdata-testid.Also applies to: 4062-4065
🤖 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/src/routes/Infrastructure/Clusters/ManagedClusters/CreateCluster/CreateCluster.test.tsx` around lines 3508 - 3511, The test is brittle because it uses inputElement.closest('.pf-v6-c-menu-toggle') which couples to PatternFly v6 CSS; replace that lookup with a stable role-based or testid-based scope: locate the menu toggle using getByRole('button', { name: /menu toggle/i }) or the input container via its data-testid, then call within(...) on that element to get the clear button and click it (update the lines that set clearButton and the within(...) call where inputElement.closest('.pf-v6-c-menu-toggle') is used).
89-93: 💤 Low valueHelper bypasses React state by using
fireEvent.change.
fireEvent.changesynchronously setsvaluewithout going throughuserEvent's realistic input pipeline. Combined with the immediatekeyDown(Enter), this works becauseAcmSelectBase'sonInputKeyDownreadsevent.currentTarget.value, but it skips the controlledonChangeflow (soinputValuestate may not update before commit). It works today, but is fragile ifAcmSelectBaselater relies oninputValuestate insidecommitTypeaheadInput. ConsideruserEvent.type(input, '${value}{enter}')for parity with the other KubeVirt tests in this file.🤖 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/src/routes/Infrastructure/Clusters/ManagedClusters/CreateCluster/CreateCluster.test.tsx` around lines 89 - 93, The helper commitComboBoxByTestId bypasses React's controlled input flow by using fireEvent.change plus fireEvent.keyDown; update it to simulate real user input using userEvent.type so the component's onChange/onInput handlers and inputValue state update before committing—replace the fireEvent.change + fireEvent.keyDown sequence in commitComboBoxByTestId with userEvent.type(input, `${value}{enter}`) (keeping the same test-id lookup) to mirror other KubeVirt tests and ensure AcmSelectBase and its commitTypeaheadInput see the updated state.frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.js (3)
56-60: 💤 Low value
control.activecan desync fromlastActivewhen a custom value equals currentlastActive.In the custom branch (Line 46)
control.activeis set totypedValueunconditionally, but the trailing assignment tocontrol.active = mappedValueonly runs whenlastActive !== mappedValue. IfmappedValue === lastActive(no-op path) andtypedValue === mappedValue, the result is fine; but the two writes tocontrol.activein different branches make the final state hard to reason about. Collapsing to a single write at the end (after computingmappedValue) avoids ambiguity.🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.js` around lines 56 - 60, Compute the final mappedValue first (using typedValue for the custom branch) and collapse the writes so you only assign control.active = mappedValue once; then if control.lastActive !== mappedValue update control.lastActive = mappedValue and call handleControlChange(). In other words, remove the unconditional control.active = typedValue in the custom branch, determine mappedValue, set control.active = mappedValue, and only update lastActive + invoke handleControlChange when lastActive actually changes (referencing control.active, control.lastActive, mappedValue, typedValue, and handleControlChange).
87-87: 💤 Low value
key={controlId}-${name}will remountAcmSelectBaseon every i18n / name change.If
nameis a localized string (e.g.,'Instance type'), language switching forces a full remount of the select, dropping focus and inputValue mid-interaction.controlIdalone should be sufficiently unique.Details
- const key = `${controlId}-${name}` + const key = controlIdAlso applies to: 99-119
🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.js` at line 87, The key currently built as `${controlId}-${name}` causes AcmSelectBase to remount whenever the localized `name` changes (e.g., on i18n switch), losing focus and input; change the key construction to use only the stable identifier `controlId` wherever `${controlId}-${name}` is used (including the occurrence around AcmSelectBase and the repeated spots referenced at lines 99-119) so the select is not remounted on name/i18n updates.
65-71: 💤 Low value
uniqueAvailableandactiveDisplaydeps look correct, butavailableMapreference identity matters.
availableMapis read offcontroland used inuseMemodeps. Ifcontrol.availableMapis rebuilt on every parent render (e.g., viafetchAvailable.setAvailableMap),activeDisplaywill recompute even when content is unchanged. Acceptable cost given how small the map is, but worth being aware of for hot paths.🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.js` around lines 65 - 71, The review notes that using control.availableMap (availableMap) as a dependency can cause unnecessary recomputations if its reference changes even when contents are equal; update the memo usage to stabilize identity or compare contents: inside the uniqueAvailable/activeDisplay logic (useMemo for uniqueAvailable and activeDisplay) either derive availableMap from a stable source on control or compute a shallow key-based fingerprint (e.g., JSON.stringify(Object.keys(availableMap).sort()) or similar) and use that fingerprint in the dependency array so activeDisplay only recomputes when the actual map contents change; ensure you update the dependency arrays for activeDisplay (and uniqueAvailable if relevant) to reference the stable value (the fingerprint or stable map) rather than the raw availableMap reference.
🤖 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/src/components/AcmSelectBase.tsx`:
- Around line 370-374: When handling keyboard commits for typeahead in
AcmSelectBase, add the same blur-commit guard used in selectOption and mouse
handlers by setting skipBlurCommitRef.current = true immediately before calling
commitTypeaheadInput in both the Tab branch (current case 'Tab') and the Enter
branch's typeahead else-branch (the branch that checks onTypeaheadInputCommit
and inputValue !== selectedItem); this prevents the blur handler from invoking
commitTypeaheadInput a second time when the input loses focus.
In `@frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.js`:
- Around line 37-63: The custom-value detection in the onChange callback (used
by ControlPanelComboBox) only checks against available (display labels) and
control.userData, so values that match an availableMap value slip through;
update the isCustomValue logic to also treat any string present in
Object.values(control.availableMap || {}) as non-custom (i.e., compute a
set/array that is [...(control.userData||[]), ...available,
...Object.values(control.availableMap||{})] and use that when computing
isCustomValue) so we don't append mapped values to control.userData or create
raw-value options; keep the rest of the flow (setting control.active,
lastActive, calling get(control, 'fetchAvailable.setAvailableMap') and
handleControlChange) unchanged.
In
`@frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js`:
- Around line 238-252: The test "displays full value from short active value"
currently only opens the dropdown and asserts an option exists (from available)
so update it to verify the component's rendered display uses the activeDisplay
conversion: render ControlPanelComboBox with control.active = 'm5.xlarge' and
assert the toggle/input element (the element returned by
screen.getByRole('button', { name: /menu toggle/i }) or the visible input)
contains the full label string produced by activeDisplay (the same string used
in the option: 'm5.xlarge - 4 vCPU, 16 GiB RAM - General Purpose');
alternatively if you prefer not to assert UI text, rename the test to reflect
that it only validates the dropdown contains the option. Ensure references to
activeDisplay and ControlPanelComboBox are used so the change targets the
conversion behavior.
In `@frontend/src/components/TemplateEditor/TemplateEditor.test.js`:
- Around line 85-86: The test uses userEvent.type(...) for combobox fields
githubURL and githubPath which fires per-character events but no trailing Enter
so onTypeaheadInputCommit never runs; replace those userEvent.type calls with
the commitComboBoxByTestId pattern used elsewhere: simulate
fireEvent.change(...) to set the input value for test ids 'githubURL' and
'githubPath' and then simulate fireEvent.keyDown(element, { key: 'Enter' }) to
commit—ensuring onTypeaheadInputCommit is invoked and the form state is actually
updated.
---
Outside diff comments:
In `@frontend/src/routes/Home/Overview/OverviewClusterLabelSelector.test.tsx`:
- Around line 75-76: The assertions using index-based getAllByText (e.g.,
getAllByText('Amazon')[1] and getAllByText('cloud')[0]) are brittle; change them
to scope the query to the chip container or assert by role/label instead: locate
the chip container rendered by the component (or add/use an existing test
id/class) and replace getAllByText(...)[i] with
within(chipContainer).getByText('Amazon') (or use getByRole/getByLabelText for
the chip/button), so the test no longer depends on DOM order or whether
AcmSelectBase surfaces the selected value in the trigger. Ensure you update the
test to reference the chip container variable and remove index-based array
accesses.
---
Nitpick comments:
In `@frontend/src/components/AcmSelectBase.tsx`:
- Around line 296-303: commitTypeaheadInput currently falls back to a
potentially-stale inputValue, which is dead code for current callers; make the
value parameter required and remove the fallback and inputValue capture: change
commitTypeaheadInput to (value: string) => { if (!onTypeaheadInputCommit)
return; onTypeaheadInputCommit(value); closeMenu(); }, remove inputValue from
the useCallback dependencies, and update all callers to always pass
event.currentTarget.value; alternatively, if you need a no-arg call path keep
the fallback but replace inputValue with a mutable ref (e.g.,
inputValueRef.current) to avoid stale closures.
In `@frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.js`:
- Around line 56-60: Compute the final mappedValue first (using typedValue for
the custom branch) and collapse the writes so you only assign control.active =
mappedValue once; then if control.lastActive !== mappedValue update
control.lastActive = mappedValue and call handleControlChange(). In other words,
remove the unconditional control.active = typedValue in the custom branch,
determine mappedValue, set control.active = mappedValue, and only update
lastActive + invoke handleControlChange when lastActive actually changes
(referencing control.active, control.lastActive, mappedValue, typedValue, and
handleControlChange).
- Line 87: The key currently built as `${controlId}-${name}` causes
AcmSelectBase to remount whenever the localized `name` changes (e.g., on i18n
switch), losing focus and input; change the key construction to use only the
stable identifier `controlId` wherever `${controlId}-${name}` is used (including
the occurrence around AcmSelectBase and the repeated spots referenced at lines
99-119) so the select is not remounted on name/i18n updates.
- Around line 65-71: The review notes that using control.availableMap
(availableMap) as a dependency can cause unnecessary recomputations if its
reference changes even when contents are equal; update the memo usage to
stabilize identity or compare contents: inside the uniqueAvailable/activeDisplay
logic (useMemo for uniqueAvailable and activeDisplay) either derive availableMap
from a stable source on control or compute a shallow key-based fingerprint
(e.g., JSON.stringify(Object.keys(availableMap).sort()) or similar) and use that
fingerprint in the dependency array so activeDisplay only recomputes when the
actual map contents change; ensure you update the dependency arrays for
activeDisplay (and uniqueAvailable if relevant) to reference the stable value
(the fingerprint or stable map) rather than the raw availableMap reference.
In
`@frontend/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js`:
- Around line 115-117: Replace the fragile selector that calls
screen.getAllByRole('combobox') and then finds by element.type with a
deterministic test-id lookup: stop using inputs.find((el) => el.type === 'text')
|| inputs[0] and instead use screen.getByTestId('masterType') (the input
receives data-testid via inputProps/controlId) as the target for userEvent.type;
this avoids hitting the MenuToggle role="combobox" element from AcmSelectBase
and ensures userEvent.type is invoked on the actual text input.
- Around line 189-214: The test currently asserts on mutated control.active
(coupling to internal mutation); instead assert that ControlPanelComboBox's
onChange invoked the provided handleControlChange with the committed value —
replace the control.active expectation with assertions on the mock handleChange
(e.g. that handleChange was called and that one of its calls includes an object
containing id: 'githubPath' and active: 'test-path' or the expected payload
shape), referencing ControlPanelComboBox and its onChange/handleControlChange
behavior so the spec verifies the outward contract rather than in-place
mutation.
- Around line 125-135: The test "'clears selection'" in
ControlPanelComboBox.test.js is using a loose matcher for the clear button;
update the call to screen.getByRole in that test to target the specific
clear-input control by using the exact accessible name (e.g. change the matcher
from /clear/i to /clear input value/i) so it reliably selects the Clear input
value button rendered by ControlPanelComboBox (propsPlain, control.active) and
avoids accidentally matching unrelated menu items; keep the rest of the test
(render, userEvent.click, waitFor expect(handleChange)) the same.
In
`@frontend/src/routes/Applications/CreateSubscriptionApplication/SubscriptionApplication.test.tsx`:
- Around line 422-426: Extract the duplicate commitComboBoxByTestId helper into
a shared module: create frontend/src/lib/test-util.ts exporting an async
function named commitComboBoxByTestId(id: string, value: string) that performs
screen.findByTestId, fireEvent.change and fireEvent.keyDown as in the diff; then
remove the local commitComboBoxByTestId definitions from CreateCluster.test.tsx,
CreateClusterPool.test.tsx and SubscriptionApplication.test.tsx and import the
helper (e.g. import { commitComboBoxByTestId } from '../../../lib/test-util') in
each test file so all three tests use the single shared implementation.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/CreateCluster/CreateCluster.test.tsx`:
- Around line 3508-3511: The test is brittle because it uses
inputElement.closest('.pf-v6-c-menu-toggle') which couples to PatternFly v6 CSS;
replace that lookup with a stable role-based or testid-based scope: locate the
menu toggle using getByRole('button', { name: /menu toggle/i }) or the input
container via its data-testid, then call within(...) on that element to get the
clear button and click it (update the lines that set clearButton and the
within(...) call where inputElement.closest('.pf-v6-c-menu-toggle') is used).
- Around line 89-93: The helper commitComboBoxByTestId bypasses React's
controlled input flow by using fireEvent.change plus fireEvent.keyDown; update
it to simulate real user input using userEvent.type so the component's
onChange/onInput handlers and inputValue state update before committing—replace
the fireEvent.change + fireEvent.keyDown sequence in commitComboBoxByTestId with
userEvent.type(input, `${value}{enter}`) (keeping the same test-id lookup) to
mirror other KubeVirt tests and ensure AcmSelectBase and its
commitTypeaheadInput see the updated state.
🪄 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: Enterprise
Run ID: 83de18d8-8465-4726-8ec5-7475251c3d6c
⛔ Files ignored due to path filters (2)
frontend/src/components/TemplateEditor/controls/__snapshots__/ControlPanelMultiSelect.test.js.snapis excluded by!**/*.snap,!**/*.snapfrontend/src/components/TemplateEditor/controls/__snapshots__/ControlPanelSingleSelect.test.js.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (13)
frontend/src/components/AcmSelectBase.tsxfrontend/src/components/TemplateEditor/TemplateEditor.test.jsfrontend/src/components/TemplateEditor/controls/ControlPanelComboBox.jsfrontend/src/components/TemplateEditor/controls/ControlPanelComboBox.test.jsfrontend/src/components/TemplateEditor/controls/ControlPanelMultiSelect.jsfrontend/src/components/TemplateEditor/controls/ControlPanelSingleSelect.tsxfrontend/src/routes/Applications/CreateSubscriptionApplication/SubscriptionApplication.test.tsxfrontend/src/routes/Applications/CreateSubscriptionApplication/controlData/ControlDataGit.jsfrontend/src/routes/Applications/CreateSubscriptionApplication/controlData/ControlDataHelm.jsfrontend/src/routes/Applications/CreateSubscriptionApplication/controlData/ControlDataObjectStore.jsfrontend/src/routes/Home/Overview/OverviewClusterLabelSelector.test.tsxfrontend/src/routes/Infrastructure/Clusters/ClusterPools/CreateClusterPool/CreateClusterPool.test.tsxfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/CreateCluster/CreateCluster.test.tsx
| case 'Tab': | ||
| if (onTypeaheadInputCommit && inputValue !== selectedItem) { | ||
| commitTypeaheadInput(event.currentTarget.value) | ||
| } | ||
| break |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l frontend/src/components/AcmSelectBase.tsxRepository: stolostron/console
Length of output: 108
🏁 Script executed:
# First, let's look at the Tab handler context (around lines 360-380)
sed -n '355,385p' frontend/src/components/AcmSelectBase.tsxRepository: stolostron/console
Length of output: 916
🏁 Script executed:
# Now look at the mouse-driven selection (around line 479)
sed -n '475,490p' frontend/src/components/AcmSelectBase.tsxRepository: stolostron/console
Length of output: 511
🏁 Script executed:
# Find the blur handler to understand skipBlurCommitRef usage
rg -n "onBlur|skipBlurCommitRef" frontend/src/components/AcmSelectBase.tsx -A 2 -B 2Repository: stolostron/console
Length of output: 1298
🏁 Script executed:
# Check the lines 707-715 mentioned in "Also applies to"
sed -n '705,720p' frontend/src/components/AcmSelectBase.tsxRepository: stolostron/console
Length of output: 638
🏁 Script executed:
# Search for all Tab key handlers to check if there are multiple instances
rg -n "case 'Tab'" frontend/src/components/AcmSelectBase.tsx -A 5 -B 2Repository: stolostron/console
Length of output: 300
🏁 Script executed:
# Search for all Enter key handlers to compare patterns
rg -n "case 'Enter'" frontend/src/components/AcmSelectBase.tsx -A 10 -B 2Repository: stolostron/console
Length of output: 571
🏁 Script executed:
# Check the full commitTypeaheadInput function to understand what it does
rg -n "commitTypeaheadInput" frontend/src/components/AcmSelectBase.tsx -B 2 -A 8Repository: stolostron/console
Length of output: 1472
Add skipBlurCommitRef.current = true guard before Tab commit to prevent double-commit on blur.
When Tab is pressed, commitTypeaheadInput runs but does not set skipBlurCommitRef.current = true. Tab causes the input to lose focus, triggering onBlur (lines 707-715); since the ref is not set, the blur handler evaluates its condition again and may call commitTypeaheadInput a second time with the same value due to async state updates.
The same gap exists in the Enter handler's typeahead branch (line 362). The fix mirrors patterns used in selectOption (line 479) and mouse handlers (lines 563, 681, 750).
Suggested fix
case 'Tab':
if (onTypeaheadInputCommit && inputValue !== selectedItem) {
+ skipBlurCommitRef.current = true
commitTypeaheadInput(event.currentTarget.value)
}
breakConsider applying the same guard to the Enter handler's else branch (line 360–362).
🤖 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/src/components/AcmSelectBase.tsx` around lines 370 - 374, When
handling keyboard commits for typeahead in AcmSelectBase, add the same
blur-commit guard used in selectOption and mouse handlers by setting
skipBlurCommitRef.current = true immediately before calling commitTypeaheadInput
in both the Tab branch (current case 'Tab') and the Enter branch's typeahead
else-branch (the branch that checks onTypeaheadInputCommit and inputValue !==
selectedItem); this prevents the blur handler from invoking commitTypeaheadInput
a second time when the input loses focus.
| const onChange = useCallback( | ||
| (value) => { | ||
| if (control.disabled) return | ||
|
|
||
| const typedValue = (value || '').trim() | ||
| const userData = control.userData ?? [] | ||
| const isCustomValue = typedValue.length > 0 && !userData.includes(typedValue) && !available.includes(typedValue) | ||
|
|
||
| if (isCustomValue) { | ||
| control.active = typedValue | ||
| control.userData = uniq([...userData, typedValue]) | ||
| get(control, 'fetchAvailable.setAvailableMap')?.(control) | ||
| } | ||
| } | ||
| const { active, available } = control | ||
| const { currentSelection } = state | ||
| let { isOpen, preselect, searchText } = state | ||
| const { isBlurred, typedText } = state | ||
| const setAvailableMap = get(control, 'fetchAvailable.setAvailableMap') || noop | ||
|
|
||
| ///////////////////////////////////////////////////////////// | ||
| // search mode | ||
| if (searchText && searchText.length && !preselect) { | ||
| // nothing selected, filter list | ||
| if (currentSelection === undefined) { | ||
| if (isBlurred) { | ||
| const { userData = [] } = control | ||
| if (!userData.includes(searchText) && available && !available.includes(searchText)) { | ||
| control.active = searchText | ||
| userData.push(searchText) | ||
| set(control, 'userData', userData) | ||
| const mappedValue = | ||
| typedValue && typeof control.availableMap?.[typedValue] === 'string' | ||
| ? control.availableMap[typedValue] | ||
| : typedValue | ||
|
|
||
| // make sure whatever user types in has an availableMap entry | ||
| setAvailableMap(control) | ||
| } | ||
| handleComboChange(searchText) | ||
| searchText = null | ||
| isOpen = false | ||
| } else { | ||
| isOpen = true | ||
| } | ||
| } else { | ||
| // handle change | ||
| handleComboChange(currentSelection) | ||
| isOpen = false | ||
| searchText = null | ||
| if (control.lastActive !== mappedValue) { | ||
| control.active = mappedValue | ||
| control.lastActive = mappedValue | ||
| handleControlChange() | ||
| } | ||
| } else if (currentSelection !== undefined) { | ||
| // handle change | ||
| handleComboChange(currentSelection) | ||
| searchText = null | ||
| isOpen = false | ||
| preselect = false | ||
| } else if (isBlurred && !preselect) { | ||
| handleComboChange(typedText || active) | ||
| isOpen = false | ||
| } | ||
| return { | ||
| active, | ||
| currentSelection: undefined, | ||
| isOpen, | ||
| isBlurred: false, | ||
| preselect, | ||
| searchText, | ||
| } | ||
| } | ||
|
|
||
| constructor(props) { | ||
| super(props) | ||
| this.state = { | ||
| isOpen: false, | ||
| isBlurred: false, | ||
| searchText: null, | ||
| sortToTop: null, | ||
| } | ||
| } | ||
|
|
||
| setControlRef = (ref) => { | ||
| this.controlRef = ref | ||
| } | ||
|
|
||
| setInputRef = (ref) => { | ||
| this.inputRef = ref | ||
| } | ||
|
|
||
| setMenuRef = (ref) => { | ||
| this.menuRef = ref | ||
| } | ||
|
|
||
| setClearRef = (ref) => { | ||
| this.clearRef = ref | ||
| } | ||
|
|
||
| setToggleRef = (ref) => { | ||
| this.toggleRef = ref | ||
| } | ||
|
|
||
| render() { | ||
| const { isOpen, searchText, sortToTop } = this.state | ||
| const { controlId, i18n, control, controlData } = this.props | ||
| const { | ||
| name, | ||
| userData = [], | ||
| availableMap, | ||
| exception, | ||
| hasReplacements, | ||
| isRefetching, | ||
| disabled, | ||
| simplified, | ||
| describe, | ||
| } = control | ||
| const { isLoading } = control | ||
| let { active, available = [], placeholder = '' } = control | ||
| if (!placeholder) { | ||
| }, | ||
| [available, control, handleControlChange] | ||
| ) |
There was a problem hiding this comment.
Custom-value detection compares against display labels but ignores mapped values.
available typically holds display labels (e.g. 'OpenShift 4.8.0-fc.7-x86_64') while availableMap maps label → real value (e.g. 'quay.io/.../ocp-release:...'). When a caller commits a value that matches an availableMap value (not key) — which is exactly what commitComboBoxByTestId('imageSet', clusterImageSetAws.spec.releaseImage) does in CreateCluster.test.tsx — isCustomValue is true, the release image is appended to control.userData, and an option with that raw URL appears in the dropdown.
Consider also excluding values present in Object.values(control.availableMap || {}) from the custom-value check:
Suggested guard
- const isCustomValue = typedValue.length > 0 && !userData.includes(typedValue) && !available.includes(typedValue)
+ const mappedValues = control.availableMap ? Object.values(control.availableMap) : []
+ const isCustomValue =
+ typedValue.length > 0 &&
+ !userData.includes(typedValue) &&
+ !available.includes(typedValue) &&
+ !mappedValues.includes(typedValue)📝 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.
| const onChange = useCallback( | |
| (value) => { | |
| if (control.disabled) return | |
| const typedValue = (value || '').trim() | |
| const userData = control.userData ?? [] | |
| const isCustomValue = typedValue.length > 0 && !userData.includes(typedValue) && !available.includes(typedValue) | |
| if (isCustomValue) { | |
| control.active = typedValue | |
| control.userData = uniq([...userData, typedValue]) | |
| get(control, 'fetchAvailable.setAvailableMap')?.(control) | |
| } | |
| } | |
| const { active, available } = control | |
| const { currentSelection } = state | |
| let { isOpen, preselect, searchText } = state | |
| const { isBlurred, typedText } = state | |
| const setAvailableMap = get(control, 'fetchAvailable.setAvailableMap') || noop | |
| ///////////////////////////////////////////////////////////// | |
| // search mode | |
| if (searchText && searchText.length && !preselect) { | |
| // nothing selected, filter list | |
| if (currentSelection === undefined) { | |
| if (isBlurred) { | |
| const { userData = [] } = control | |
| if (!userData.includes(searchText) && available && !available.includes(searchText)) { | |
| control.active = searchText | |
| userData.push(searchText) | |
| set(control, 'userData', userData) | |
| const mappedValue = | |
| typedValue && typeof control.availableMap?.[typedValue] === 'string' | |
| ? control.availableMap[typedValue] | |
| : typedValue | |
| // make sure whatever user types in has an availableMap entry | |
| setAvailableMap(control) | |
| } | |
| handleComboChange(searchText) | |
| searchText = null | |
| isOpen = false | |
| } else { | |
| isOpen = true | |
| } | |
| } else { | |
| // handle change | |
| handleComboChange(currentSelection) | |
| isOpen = false | |
| searchText = null | |
| if (control.lastActive !== mappedValue) { | |
| control.active = mappedValue | |
| control.lastActive = mappedValue | |
| handleControlChange() | |
| } | |
| } else if (currentSelection !== undefined) { | |
| // handle change | |
| handleComboChange(currentSelection) | |
| searchText = null | |
| isOpen = false | |
| preselect = false | |
| } else if (isBlurred && !preselect) { | |
| handleComboChange(typedText || active) | |
| isOpen = false | |
| } | |
| return { | |
| active, | |
| currentSelection: undefined, | |
| isOpen, | |
| isBlurred: false, | |
| preselect, | |
| searchText, | |
| } | |
| } | |
| constructor(props) { | |
| super(props) | |
| this.state = { | |
| isOpen: false, | |
| isBlurred: false, | |
| searchText: null, | |
| sortToTop: null, | |
| } | |
| } | |
| setControlRef = (ref) => { | |
| this.controlRef = ref | |
| } | |
| setInputRef = (ref) => { | |
| this.inputRef = ref | |
| } | |
| setMenuRef = (ref) => { | |
| this.menuRef = ref | |
| } | |
| setClearRef = (ref) => { | |
| this.clearRef = ref | |
| } | |
| setToggleRef = (ref) => { | |
| this.toggleRef = ref | |
| } | |
| render() { | |
| const { isOpen, searchText, sortToTop } = this.state | |
| const { controlId, i18n, control, controlData } = this.props | |
| const { | |
| name, | |
| userData = [], | |
| availableMap, | |
| exception, | |
| hasReplacements, | |
| isRefetching, | |
| disabled, | |
| simplified, | |
| describe, | |
| } = control | |
| const { isLoading } = control | |
| let { active, available = [], placeholder = '' } = control | |
| if (!placeholder) { | |
| }, | |
| [available, control, handleControlChange] | |
| ) | |
| const onChange = useCallback( | |
| (value) => { | |
| if (control.disabled) return | |
| const typedValue = (value || '').trim() | |
| const userData = control.userData ?? [] | |
| const mappedValues = control.availableMap ? Object.values(control.availableMap) : [] | |
| const isCustomValue = | |
| typedValue.length > 0 && | |
| !userData.includes(typedValue) && | |
| !available.includes(typedValue) && | |
| !mappedValues.includes(typedValue) | |
| if (isCustomValue) { | |
| control.active = typedValue | |
| control.userData = uniq([...userData, typedValue]) | |
| get(control, 'fetchAvailable.setAvailableMap')?.(control) | |
| } | |
| const mappedValue = | |
| typedValue && typeof control.availableMap?.[typedValue] === 'string' | |
| ? control.availableMap[typedValue] | |
| : typedValue | |
| if (control.lastActive !== mappedValue) { | |
| control.active = mappedValue | |
| control.lastActive = mappedValue | |
| handleControlChange() | |
| } | |
| }, | |
| [available, control, handleControlChange] | |
| ) |
🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.js`
around lines 37 - 63, The custom-value detection in the onChange callback (used
by ControlPanelComboBox) only checks against available (display labels) and
control.userData, so values that match an availableMap value slip through;
update the isCustomValue logic to also treat any string present in
Object.values(control.availableMap || {}) as non-custom (i.e., compute a
set/array that is [...(control.userData||[]), ...available,
...Object.values(control.availableMap||{})] and use that when computing
isCustomValue) so we don't append mapped values to control.userData or create
raw-value options; keep the rest of the flow (setting control.active,
lastActive, calling get(control, 'fetchAvailable.setAvailableMap') and
handleControlChange) unchanged.
| it('displays full value from short active value', async () => { | ||
| const control = { | ||
| ...propsPlain.control, | ||
| active: 'm5.xlarge', | ||
| } | ||
| render(<ControlPanelComboBox {...propsPlain} control={control} />) | ||
|
|
||
| const toggle = screen.getByRole('button', { name: /menu toggle/i }) | ||
| userEvent.click(toggle) | ||
|
|
||
| await waitFor(() => { | ||
| const option = screen.getByText('m5.xlarge - 4 vCPU, 16 GiB RAM - General Purpose') | ||
| expect(option).toBeInTheDocument() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Test name promises more than it asserts.
'displays full value from short active value' only opens the dropdown and confirms the matching option exists. That option is rendered for any active value because options come from available, not from active. This doesn't actually verify the short-to-full conversion done by activeDisplay (Lines 68-71 in ControlPanelComboBox.js). Either assert on the toggle/input's displayed text matching the full label, or rename the test.
As per coding guidelines, "Before claiming what the code under test does or allows, confirm it from the actual source ...; do not infer behavior from test names or comments alone."
🤖 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/src/components/TemplateEditor/controls/ControlPanelComboBox.test.js`
around lines 238 - 252, The test "displays full value from short active value"
currently only opens the dropdown and asserts an option exists (from available)
so update it to verify the component's rendered display uses the activeDisplay
conversion: render ControlPanelComboBox with control.active = 'm5.xlarge' and
assert the toggle/input element (the element returned by
screen.getByRole('button', { name: /menu toggle/i }) or the visible input)
contains the full label string produced by activeDisplay (the same string used
in the option: 'm5.xlarge - 4 vCPU, 16 GiB RAM - General Purpose');
alternatively if you prefer not to assert UI text, rename the test to reflect
that it only validates the dropdown contains the option. Ensure references to
activeDisplay and ControlPanelComboBox are used so the change targets the
conversion behavior.
| userEvent.type(screen.getByTestId('githubURL'), 'https://github.com/fxiang1/app-samples') | ||
| userEvent.type(screen.getByTestId('githubPath'), 'ansible') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the test file
fd -type f "TemplateEditor.test.js" --exec wc -l {} \;Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Get the file location
fd -type f "TemplateEditor.test.js" | head -5Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Read the context around lines 85-86
fd -type f "TemplateEditor.test.js" -exec cat -n {} \; | sed -n '75,100p'Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Check the full test to understand assertions
fd -type f "TemplateEditor.test.js" -exec wc -l {} \;Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Search for SubscriptionApplication.test.tsx and the commitComboBoxByTestId helper
fd -type f "SubscriptionApplication.test.*" | head -1Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Look for commitComboBoxByTestId definition
rg "commitComboBoxByTestId" -A 5 -B 2Repository: stolostron/console
Length of output: 13994
🏁 Script executed:
# Check typeahead/combobox component implementations
fd -type f -name "*ComboBox*" -o -name "*Typeahead*" -o -name "*AcmSelect*" | grep -v node_modules | head -10Repository: stolostron/console
Length of output: 233
🏁 Script executed:
# Search for onTypeaheadInputCommit usage
rg "onTypeaheadInputCommit" -B 3 -A 3Repository: stolostron/console
Length of output: 4379
🏁 Script executed:
# Find TemplateEditor.test.js
find . -name "TemplateEditor.test.js" -type fRepository: stolostron/console
Length of output: 126
🏁 Script executed:
# Read the full TemplateEditor.test.js file to understand context and assertions
find . -name "TemplateEditor.test.js" -type f -exec cat -n {} \;Repository: stolostron/console
Length of output: 19457
Align userEvent.type() with the commitComboBoxByTestId pattern for combobox fields.
userEvent.type() fires per-character key events but does not emit a trailing Enter, so onTypeaheadInputCommit is never triggered. Since this test lacks assertions validating the submitted values, it may pass without actually committing githubURL and githubPath to form state. SubscriptionApplication.test.tsx, CreateCluster.test.tsx, and CreateClusterPool.test.tsx consistently use fireEvent.change() + fireEvent.keyDown(Enter) for these fields.
🔧 Suggested fix
- userEvent.type(screen.getByTestId('githubURL'), 'https://github.com/fxiang1/app-samples')
- userEvent.type(screen.getByTestId('githubPath'), 'ansible')
+ const githubURLInput = screen.getByTestId('githubURL')
+ fireEvent.change(githubURLInput, { target: { value: 'https://github.com/fxiang1/app-samples' } })
+ fireEvent.keyDown(githubURLInput, { key: 'Enter', code: 'Enter' })
+ const githubPathInput = screen.getByTestId('githubPath')
+ fireEvent.change(githubPathInput, { target: { value: 'ansible' } })
+ fireEvent.keyDown(githubPathInput, { key: 'Enter', code: 'Enter' })📝 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.
| userEvent.type(screen.getByTestId('githubURL'), 'https://github.com/fxiang1/app-samples') | |
| userEvent.type(screen.getByTestId('githubPath'), 'ansible') | |
| const githubURLInput = screen.getByTestId('githubURL') | |
| fireEvent.change(githubURLInput, { target: { value: 'https://github.com/fxiang1/app-samples' } }) | |
| fireEvent.keyDown(githubURLInput, { key: 'Enter', code: 'Enter' }) | |
| const githubPathInput = screen.getByTestId('githubPath') | |
| fireEvent.change(githubPathInput, { target: { value: 'ansible' } }) | |
| fireEvent.keyDown(githubPathInput, { key: 'Enter', code: 'Enter' }) |
🤖 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/src/components/TemplateEditor/TemplateEditor.test.js` around lines
85 - 86, The test uses userEvent.type(...) for combobox fields githubURL and
githubPath which fires per-character events but no trailing Enter so
onTypeaheadInputCommit never runs; replace those userEvent.type calls with the
commitComboBoxByTestId pattern used elsewhere: simulate fireEvent.change(...) to
set the input value for test ids 'githubURL' and 'githubPath' and then simulate
fireEvent.keyDown(element, { key: 'Enter' }) to commit—ensuring
onTypeaheadInputCommit is invoked and the form state is actually updated.
1 participant
📝 Summary
Ticket Summary (Title):
ACM-31937 Node pool dropdown has misaligned selection indicator and clear button
Ticket Link:
https://redhat.atlassian.net/browse/ACM-31937
Type of Change:
✅ Checklist
General
ACM-12340 Fix bug with...)If Feature
If Bugfix
🗒️ Notes for Reviewers
Summary by CodeRabbit
Release Notes