Skip to content

Commit 1eefdf5

Browse files
thekevinbrownthekevinbrown
authored
Merge pull request #2066 from exogee-technology/fix/combobox-improvements-and-tests
Fix: Always include selected options at the top of the list
2 parents b92eb83 + 49140f7 commit 1eefdf5

File tree

5 files changed

+77
-56
lines changed

5 files changed

+77
-56
lines changed

‎.github/workflows/postgres-end-to-end-tests.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ jobs:
7777
pnpm test-ui-postgres &&
7878
killall node
7979
80+
- uses: actions/upload-artifact@v4
81+
if: always()
82+
with:
83+
name: playwright-report
84+
path: src/packages/end-to-end/playwright-report/
85+
retention-days: 30
86+
8087
env:
8188
CI: true
8289
DATABASE_HOST: localhost

‎src/examples/sqlite/databases/trace.sqlite‎

456 KB
Binary file not shown.

‎src/packages/admin-ui-components/src/combo-box/component.tsx‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,12 @@ export const ComboBox = ({
6868
dataFetcher,
6969
searchDebounceMs = 300,
7070
}: SelectProps) => {
71-
const valueArray = arrayify(value);
71+
const valueArray = arrayify(value) as SelectOption[];
7272
const inputRef = useAutoFocus<HTMLInputElement>(autoFocus);
7373
const selectBoxRef = useRef<HTMLDivElement>(null);
7474

7575
// Lazy loading state
76-
const [dynamicOptions, setDynamicOptions] = useState<SelectOption[]>([]);
76+
const [dynamicOptions, setDynamicOptions] = useState<SelectOption[]>(valueArray);
7777
const [isLoadingMore, setIsLoadingMore] = useState(false);
7878
const [searchTerm, setSearchTerm] = useState('');
7979
const [searchTimeout, setSearchTimeout] = useState<NodeJS.Timeout | null>(null);
@@ -86,17 +86,20 @@ export const ComboBox = ({
8686
// Use ref to track if we're already loading data to prevent duplicate fetches
8787
const fetchedPagesRef = useRef(new Set<number>());
8888

89+
// Store the selected ids in a set for easy lookup - this is our source of truth for selection
90+
const selectedIds = useMemo(() => new Set(valueArray.map((item) => item.value)), [valueArray]);
91+
92+
const sortOptionsBySelectedFirst = useCallback((opt1: SelectOption, opt2: SelectOption) => {
93+
return (selectedIds.has(opt2.value) ? 1 : 0) - (selectedIds.has(opt1.value) ? 1 : 0)
94+
}, [selectedIds]);
95+
8996
// Calculate items once - use dynamic options if dataFetcher is provided, otherwise use static options
9097
const options = useMemo(() => {
9198
return dataFetcher ? dynamicOptions : staticOptions || [];
9299
}, [dataFetcher, dynamicOptions, staticOptions]);
93100

94-
// Store the selected ids in a set for easy lookup - this is our source of truth for selection
95-
const selectedIds = useMemo(() => new Set(valueArray.map((item) => item.value)), [valueArray]);
96-
97-
const sortOptionsBySelectedFirst = (opt1: SelectOption, opt2: SelectOption) => {
98-
return (selectedIds.has(opt2.value) ? 1 : 0) - (selectedIds.has(opt1.value) ? 1 : 0)
99-
}
101+
// Duplicates can occur for a variety of reasons – e.g., when using this for a string field filter
102+
const optionIds = useMemo(() => new Set(options.map((item) => item.value)), [options]);
100103

101104
// Handle individual item deselection
102105
const handleItemDeselect = useCallback(
@@ -121,14 +124,15 @@ export const ComboBox = ({
121124
} = useCombobox({
122125
items: options,
123126
id: fieldId,
127+
selectedItem: null,
124128
itemToString: (item) => item?.label ?? '',
125129
isItemDisabled: () => disabled,
126130
onInputValueChange: ({ inputValue }) => {
127131
onInputChange?.(inputValue);
128132

129133
if (dataFetcher && inputValue !== undefined) {
130134
fetchedPagesRef.current.clear();
131-
setDynamicOptions([]);
135+
setDynamicOptions(valueArray);
132136
setCurrentPage(1);
133137
setHasReachedEnd(false);
134138
setSearchTerm(inputValue);
@@ -187,14 +191,14 @@ export const ComboBox = ({
187191
if (lastSearchTermRef.current === search) {
188192
// Search term hasn't changed, merge the options in.
189193
if (result && result.length > 0) {
190-
setDynamicOptions((prev) => [...prev, ...result]);
194+
setDynamicOptions((prev) => [...prev, ...result.filter(o => !optionIds.has(o.value))]);
191195
setCurrentPage(page);
192196
} else {
193197
setHasReachedEnd(true);
194198
}
195199
} else {
196200
// If the search term has changed, we need to reset the options
197-
setDynamicOptions(result);
201+
setDynamicOptions([...valueArray, ...result.filter(o => !selectedIds.has(o.value))]);
198202
setCurrentPage(1);
199203
setHasReachedEnd(false);
200204
fetchedPagesRef.current.clear();
@@ -207,7 +211,7 @@ export const ComboBox = ({
207211
setIsLoadingMore(false);
208212
}
209213
},
210-
[dataFetcher, isOpen]
214+
[dataFetcher, isOpen, selectedIds, optionIds]
211215
);
212216

213217
// Scroll the menu to the top when it's opened.

‎src/packages/end-to-end/src/__tests__/ui/postgres/combobox.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { test, expect } from '@playwright/test';
2+
import { config } from '../../../config';
3+
4+
test('Detail Panel - should allow deselecting of an entity in a one to many relationship field', async ({
5+
page,
6+
}) => {
7+
await page.goto(config.adminUiUrl);
8+
9+
await page.getByTestId('Invoice-entity-link').click();
10+
await page.getByRole('cell', { name: '1', exact: true }).click();
11+
12+
// We should begin in this state
13+
await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText(
14+
'invoiceLines2 Selected×'
15+
);
16+
17+
await page.getByTestId('detail-panel-field-invoiceLines').click();
18+
await page.getByRole('option', { name: '2', exact: true }).click();
19+
20+
await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText('invoiceLines1×');
21+
});
22+
23+
test('Filter - should be able to interact with and select options using the keyboard', async ({
24+
page,
25+
}) => {
26+
await page.goto(config.adminUiUrl);
27+
await page.getByTestId('Album-entity-link').click();
28+
29+
// First arrow press opens the list
30+
await page.getByTestId('artist-filter-input').press('ArrowDown');
31+
// Go down to the third option
32+
await page.getByTestId('artist-filter-input').press('ArrowDown');
33+
await page.getByTestId('artist-filter-input').press('ArrowDown');
34+
await page.getByTestId('artist-filter-input').press('ArrowDown');
35+
await page.getByTestId('artist-filter-input').press('Enter');
36+
await expect(page.getByTestId('artist-filter-box')).toContainText(/^A/);
37+
});
38+
39+
test('Filter - should be able to deselect options using the keyboard', async ({ page }) => {
40+
await page.goto(config.adminUiUrl);
41+
await page.getByTestId('Album-entity-link').click();
42+
43+
await page.getByTestId('artist-filter-input').press('ArrowDown');
44+
await page.getByRole('option', { name: 'AC/DC' }).click();
45+
await page.getByTestId('artist-filter-input').press('Escape');
46+
47+
await page.getByTestId('artist-filter-input').press('ArrowDown');
48+
await expect(page.getByRole('option').first()).toContainText('AC/DC');
49+
50+
// Deselect
51+
await page.getByTestId('artist-filter-input').press('ArrowDown');
52+
await page.getByTestId('artist-filter-input').press('Enter');
53+
await expect(page.getByTestId('artist-filter-box')).not.toContainText('AC/DC');
54+
});

‎src/packages/end-to-end/src/__tests__/ui/postgres/enum-editor.test.ts‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,47 +20,3 @@ test('Detail Panel - should allow editing of an entity with an enum dropdown', a
2020
'paymentStatusUNPAID×'
2121
);
2222
});
23-
24-
test('Detail Panel - should allow deselecting of an entity in a one to many relationship field', async ({
25-
page,
26-
}) => {
27-
await page.goto(config.adminUiUrl);
28-
29-
await page.getByTestId('Invoice-entity-link').click();
30-
await page.getByRole('cell', { name: '1', exact: true }).click();
31-
32-
// We should begin in this state
33-
await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText(
34-
'invoiceLines2 Selected×'
35-
);
36-
37-
await page.getByTestId('detail-panel-field-invoiceLines').click();
38-
await page.getByRole('option', { name: '2', exact: true }).click();
39-
40-
await expect(page.getByTestId('detail-panel-field-invoiceLines')).toContainText('invoiceLines1×');
41-
});
42-
43-
test('Filter - should be able to interact with and select options using the keyboard', async ({
44-
page,
45-
}) => {
46-
await page.goto(config.adminUiUrl);
47-
await page.getByTestId('Album-entity-link').click();
48-
49-
// First arrow press opens the list
50-
await page.getByTestId('artist-filter-input').press('ArrowDown');
51-
// Go down to the third option
52-
await page.getByTestId('artist-filter-input').press('ArrowDown');
53-
await page.getByTestId('artist-filter-input').press('ArrowDown');
54-
await page.getByTestId('artist-filter-input').press('ArrowDown');
55-
await page.getByTestId('artist-filter-input').press('Enter');
56-
await expect(page.getByTestId('artist-filter-box')).toContainText('Aaron Goldberg');
57-
58-
// Re-open the list, and then the selected option should now be the first in the list
59-
await page.getByTestId('artist-filter-input').press('ArrowDown');
60-
await expect(page.getByRole('option').first()).toContainText('Aaron Goldberg');
61-
62-
// Deselect
63-
await page.getByTestId('artist-filter-input').press('ArrowDown');
64-
await page.getByTestId('artist-filter-input').press('Enter');
65-
await expect(page.getByTestId('artist-filter-box')).not.toContainText('Aaron Goldberg');
66-
});

0 commit comments

Comments
 (0)