#490 - Select: multiple mode closes the dropdown after every selection - #491
#490 - Select: multiple mode closes the dropdown after every selection#491jaieds wants to merge 1 commit into
Conversation
Selecting an option in multiple mode no longer closes the dropdown, and clicking an already-selected option deselects it. Escape and outside click (useDismiss) still close the list; single mode still closes on select. Updated MultiSelect and InlineSearchMulti story play tests to assert the keep-open and toggle behavior.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -248,17 +248,25 @@ MultiSelect.play = async ( { canvasElement } ) => { | |||
| const listBox = await screen.findByRole( 'listbox' ); | |||
| expect( listBox ).toHaveTextContent( 'Red' ); | |||
|
|
|||
There was a problem hiding this comment.
What: You should verify if the listbox remains open with selected options before each interaction.
Why: This would ensure that you account for the dropdown's state in all test assertions, which is crucial to confirm the component functions correctly as intended in multiple selection mode.
How: Consider adding assertions to check the state of the listbox after clicking each option, for example using expect(screen.queryByRole('listbox')).not.toBeNull(); before subsequent interactions.
| expect( screen.queryByRole( 'listbox' ) ).not.toBeNull(); | ||
| await userEvent.click( allOptions[ 1 ] ); // Orange | ||
| expect( screen.queryByRole( 'listbox' ) ).not.toBeNull(); | ||
|
|
There was a problem hiding this comment.
What: Ensure you validate state change rigorously for toggling options.
Why: Confirming that the deselecting of an already selected item reflects correctly in the button text is essential for both correctness and maintainability.
How: Continue using your pattern of expect( selectButton ).toHaveTextContent(...) to assert the button text updates as expected after a toggle action.
|
|
||
| // Clicking an already-selected option deselects it without closing | ||
| await userEvent.click( allOptions[ 1 ] ); // Orange (toggle off) | ||
| expect( screen.queryByRole( 'listbox' ) ).not.toBeNull(); |
There was a problem hiding this comment.
What: The events concerning the Escape key should be adequately tested in isolation as well to ensure no other interactions interfere with its function.
Why: This adds robustness to your tests, ensuring that all possible user interactions are accounted for, enhancing confidence in the dropdown functionality.
How: Consider adding a similar test case that explicitly focuses on the Escape key functionality after clicking multiple options.
1 participant
Closes #490
What
multiplemode: dropdown stays open after an option is clicked or toggled via Enter/Space, so several options can be picked without reopening.useDismiss(unchanged); focus returns to the trigger throughFloatingFocusManager.How
handleMultiSelectinselect.tsxno longer callssetIsOpen(false)or refocuses the trigger, and the early-return for already-selected values is replaced with a splice (deselect). Checkmarks/badges update live since option checked state derives fromgetValues().Tests
MultiSelectandInlineSearchMultiplay tests: keep-open assertions after each pick, toggle-deselect without close, Escape closes.Notes
component-improvements/editor-input(PR #2600 - Support smart tags in Custom JSON-LD string values #489) — will retarget automatically when that merges.select-atom.stories.tsxhas pre-existing CRLF line endings in HEAD; left untouched to keep the diff clean.