Skip to content

fix(Select): match search against rendered DOM text #1154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ariser wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix(Select): match search against rendered DOM text #1154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
65e6c52
refactor(Select): you might not need an effect
ariser Jul 27, 2026
4d09d49
refactor(Select): better naming for internal vars
ariser Jul 27, 2026
dfac3ac
fix(Select): prevent keyboard selection of disabled options
ariser Jul 27, 2026
9228141
fix(Select): match search against rendered DOM text
ariser Jul 27, 2026
c5fcc97
chore: add changeset
ariser Jul 28, 2026
82b4cbd
chore: fix changeset name
ariser Jul 28, 2026
3eb328e
refactor(Select): clearer search naming and leaner comments
ariser Jul 28, 2026
b456b45
fix(Select): keep filtered options mounted so search reads their text
ariser Jul 28, 2026
64078fb
refactor(Select): drop now-unused SelectItemObject type
ariser Jul 28, 2026
35b57c2
refactor(Select): route option value-node registration through regist…
ariser Jul 28, 2026
64347b6
docs(Select): trim duplicated item comment to one line
ariser Jul 28, 2026
5ac29e1
fix(Select): force filtered items hidden via an explicit CSS rule
ariser Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/fix-select-search.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@clickhouse/click-ui": patch
---

Fix a few bugs by changing the way Select works with its options: instead of imperative rebuilding of several service entities,
derive them in-flow, “you might not need an effect”.

Bugs fixed:
- search now always works in Selects and matches any text that is rendered in items, including options that arrive while the menu is open
- disabled items can no longer be selected via keyboard navigation

Note: filtered-out items now stay mounted (hidden) instead of unmounting, so their rendered text stays available to search.
56 changes: 28 additions & 28 deletions src/components/CheckboxMultiSelect/CheckboxMultiSelect.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,11 @@ describe('CheckboxCheckboxMultiSelect', () => {
const selectTrigger = getByTestId('select-trigger');
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Content0')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).toBeInTheDocument();
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content3')).toBeInTheDocument();
expect(queryByText('Content4')).toBeInTheDocument();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
});

it('filters items by search text', () => {
Expand All @@ -275,16 +275,16 @@ describe('CheckboxCheckboxMultiSelect', () => {
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).toBeInTheDocument();
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content3')).toBeInTheDocument();
expect(queryByText('Content4')).toBeInTheDocument();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'content2' },
});
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).not.toBeInTheDocument();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
});

Expand All @@ -299,16 +299,16 @@ describe('CheckboxCheckboxMultiSelect', () => {
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).toBeInTheDocument();
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content3')).toBeInTheDocument();
expect(queryByText('Content4')).toBeInTheDocument();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'content2' },
});
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).not.toBeInTheDocument();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
});

Expand All @@ -325,16 +325,16 @@ describe('CheckboxCheckboxMultiSelect', () => {
fireEvent.change(selectInput, {
target: { value: 'content2' },
});
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).not.toBeInTheDocument();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
fireEvent.click(getByTestId('select-search-close'));
expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).toBeInTheDocument();
expect(queryByText('Content1 long text content')).toBeInTheDocument();
expect(queryByText('Content2')).toBeInTheDocument();
expect(queryByText('Content3')).toBeInTheDocument();
expect(queryByText('Content4')).toBeInTheDocument();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
expect(document.activeElement).toBe(selectInput);
});

Expand All @@ -350,8 +350,8 @@ describe('CheckboxCheckboxMultiSelect', () => {
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'nodata' },
});
expect(queryByText('Content2')).not.toBeInTheDocument();
expect(queryByText('Content1 long text content')).not.toBeInTheDocument();
expect(queryByText('Content2')).not.toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
const btn = queryByText(/No Options found/i);
expect(btn).toBeInTheDocument();
Expand Down
60 changes: 30 additions & 30 deletions src/components/MultiSelect/MultiSelect.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ describe('MultiSelect', () => {
expect(selectTrigger).not.toBeNull();
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Content0')).not.toBeNull();
expect(queryByText('Content1 long text content')).not.toBeNull();
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content3')).not.toBeNull();
expect(queryByText('Content4')).not.toBeNull();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
});

it('filter by text', () => {
Expand All @@ -239,16 +239,16 @@ describe('MultiSelect', () => {
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).not.toBeNull();
expect(queryByText('Content1 long text content')).not.toBeNull();
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content3')).not.toBeNull();
expect(queryByText('Content4')).not.toBeNull();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'content2' },
});
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content1 long text content')).toBeNull();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
});

Expand All @@ -262,16 +262,16 @@ describe('MultiSelect', () => {
selectTrigger && fireEvent.click(selectTrigger);

expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).not.toBeNull();
expect(queryByText('Content1 long text content')).not.toBeNull();
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content3')).not.toBeNull();
expect(queryByText('Content4')).not.toBeNull();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'content2' },
});
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content1 long text content')).toBeNull();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
});

Expand All @@ -287,16 +287,16 @@ describe('MultiSelect', () => {
fireEvent.change(selectInput, {
target: { value: 'content2' },
});
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content1 long text content')).toBeNull();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
fireEvent.click(getByTestId('select-search-close'));
expect(queryByText('Group label')).toBeVisible();
expect(queryByText('Content0')).not.toBeNull();
expect(queryByText('Content1 long text content')).not.toBeNull();
expect(queryByText('Content2')).not.toBeNull();
expect(queryByText('Content3')).not.toBeNull();
expect(queryByText('Content4')).not.toBeNull();
expect(queryByText('Content0')).toBeVisible();
expect(queryByText('Content1 long text content')).toBeVisible();
expect(queryByText('Content2')).toBeVisible();
expect(queryByText('Content3')).toBeVisible();
expect(queryByText('Content4')).toBeVisible();
expect(document.activeElement).toBe(selectInput);
});
it('on no options available show no data', () => {
Expand All @@ -310,8 +310,8 @@ describe('MultiSelect', () => {
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'nodata' },
});
expect(queryByText('Content2')).toBeNull();
expect(queryByText('Content1 long text content')).toBeNull();
expect(queryByText('Content2')).not.toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
const btn = queryByText(/No Options found/i);
expect(btn).not.toBeNull();
Expand All @@ -333,8 +333,8 @@ describe('MultiSelect', () => {
fireEvent.change(getByTestId('select-search-input'), {
target: { value: 'nodata' },
});
expect(queryByText('Content2')).toBeNull();
expect(queryByText('Content1 long text content')).toBeNull();
expect(queryByText('Content2')).not.toBeVisible();
expect(queryByText('Content1 long text content')).not.toBeVisible();
expect(queryByText('Group label')).not.toBeVisible();
const btn = queryByText(/No Field found/i);
expect(btn).not.toBeNull();
Expand Down
Loading
Loading