Skip to content

Commit de7cb98

Browse files
theosandersonLoculus botCopilot
authored
feat(website): use consistent and better field selector (#4072)
resolves #2064 We used to have two components for selecting fields, the old one for column/search selection: ![image](https://github.com/user-attachments/assets/8a979d03-2f46-4ace-a1ca-6f111d9e253b) And the new one for Download field selection: ![image](https://github.com/user-attachments/assets/0fac290c-279e-4f13-90a8-eb6ec3f1907c) This removes the first one and makes everything use a beefed up version of the second It also moves a test from E2E tests to integration tests as part of our general aim to eventually remove the e2e tests entirely 🚀 Preview: https://fields2.loculus.org --------- Co-authored-by: Loculus bot <bot@loculus.org> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 4d39ec7 commit de7cb98

14 files changed

Lines changed: 280 additions & 263 deletions

File tree

‎integration-tests/tests/pages/search.page.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Page, expect } from '@playwright/test';
33
export class SearchPage {
44
constructor(private page: Page) {}
55

6-
private async navigateToVirus(virus: string) {
6+
async navigateToVirus(virus: string) {
77
await this.page.goto('/');
88
await this.page.getByRole('link', { name: new RegExp(virus) }).click();
99
}
@@ -31,7 +31,7 @@ export class SearchPage {
3131
for (const label of fieldLabels) {
3232
await this.page.getByRole('checkbox', { name: label }).check();
3333
}
34-
await this.page.getByRole('button', { name: 'Close' }).click();
34+
await this.page.getByTestId('field-selector-close-button').click();
3535
}
3636

3737
async fill(fieldLabel: string, value: string) {

‎integration-tests/tests/specs/features/search/column-visibility.dependent.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { expect, test } from '@playwright/test';
2+
import { SearchPage } from '../../../pages/search.page';
3+
4+
test.describe('Column Visibility', () => {
5+
let searchPage: SearchPage;
6+
7+
test.beforeEach(async ({ page }) => {
8+
searchPage = new SearchPage(page);
9+
});
10+
11+
test('should show possibly-visible columns and hide always-hidden ones in the customization modal', async ({
12+
page,
13+
}) => {
14+
await searchPage.navigateToVirus('Test Dummy Organism');
15+
await page.getByText('Customize columns').click();
16+
17+
await page.getByRole('checkbox', { name: 'Pango lineage' }).waitFor();
18+
await expect(page.getByRole('checkbox', { name: 'Pango lineage' })).toBeVisible();
19+
await expect(page.getByRole('checkbox', { name: 'Hidden Field' })).not.toBeVisible();
20+
});
21+
});

‎integration-tests/tests/specs/features/search/download.dependent.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ test('Download metadata and check number of cols', async ({ page }) => {
1818
break;
1919
}
2020
}
21-
await page.getByRole('button', { name: 'Done' }).click();
21+
await page.getByTestId('field-selector-close-button').click();
2222

2323
const downloadPromise = page.waitForEvent('download');
2424
await page.getByTestId('start-download').click();
@@ -58,7 +58,7 @@ test('Download metadata with POST and check number of cols', async ({ page }) =>
5858
break;
5959
}
6060
}
61-
await page.getByRole('button', { name: 'Done' }).click();
61+
await page.getByTestId('field-selector-close-button').click();
6262

6363
const downloadPromise = page.waitForEvent('download');
6464
await page.getByTestId('start-download').click();

‎kubernetes/loculus/values.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,6 +1363,7 @@ defaultOrganisms:
13631363
header: "Collection Details"
13641364
- name: pangoLineage
13651365
initiallyVisible: true
1366+
displayName: "Pango lineage"
13661367
autocomplete: true
13671368
required: true
13681369
type: string

‎website/src/components/SearchPage/CustomizeModal.tsx‎

Lines changed: 0 additions & 100 deletions
This file was deleted.

‎website/src/components/SearchPage/DownloadDialog/FieldSelector/FieldSelectorModal.spec.tsx‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,6 @@ describe('FieldSelectorModal', () => {
9292
expect(screen.getByText('Field 2')).toBeInTheDocument();
9393
expect(screen.getByText('Field 3')).toBeInTheDocument();
9494
expect(screen.getByText('Field 4')).toBeInTheDocument(); // Now should be rendered
95-
expect(screen.getByText('Accession Version')).toBeInTheDocument();
96-
97-
// Check that ACCESSION_VERSION_FIELD is disabled
98-
const accversionCheckbox = screen.getByLabelText('Accession Version') as Element;
99-
const inputAccVersion = accversionCheckbox as unknown as HTMLInputElement;
100-
expect(inputAccVersion.disabled).toBe(true);
10195
});
10296

10397
it('initializes with default selected fields if no initialSelectedFields provided', () => {
@@ -108,21 +102,17 @@ describe('FieldSelectorModal', () => {
108102
const field2Checkbox = screen.getByLabelText('Field 2') as Element;
109103
const field3Checkbox = screen.getByLabelText('Field 3') as Element;
110104
const field4Checkbox = screen.getByLabelText('Field 4') as Element;
111-
const accessionVersionCheckbox = screen.getByLabelText('Accession Version') as Element;
112105

113106
// Adding type assertion to properly access the checked property
114107
const input1 = field1Checkbox as unknown as HTMLInputElement;
115108
const input2 = field2Checkbox as unknown as HTMLInputElement;
116109
const input3 = field3Checkbox as unknown as HTMLInputElement;
117110
const input4 = field4Checkbox as unknown as HTMLInputElement;
118-
const inputAccVersion = accessionVersionCheckbox as unknown as HTMLInputElement;
119111

120112
expect(input1.checked).toBe(true);
121113
expect(input2.checked).toBe(false);
122114
expect(input3.checked).toBe(true);
123115
expect(input4.checked).toBe(true);
124-
expect(inputAccVersion.checked).toBe(true);
125-
expect(inputAccVersion.disabled).toBe(true); // Should be disabled
126116
});
127117

128118
it('calls onSave immediately when a field is toggled and ACCESSION_VERSION_FIELD is always included', () => {

‎website/src/components/SearchPage/DownloadDialog/FieldSelector/FieldSelectorModal.tsx‎

Lines changed: 21 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useState, type FC } from 'react';
22

33
import { ACCESSION_VERSION_FIELD } from '../../../../settings.ts';
44
import { type Metadata } from '../../../../types/config.ts';
5-
import { BaseDialog } from '../../../common/BaseDialog.tsx';
5+
import { FieldSelectorModal as CommonFieldSelectorModal, type FieldItem } from '../../../common/FieldSelectorModal.tsx';
66

77
type FieldSelectorProps = {
88
isOpen: boolean;
@@ -27,127 +27,36 @@ export const FieldSelectorModal: FC<FieldSelectorProps> = ({
2727

2828
const [selectedFields, setSelectedFields] = useState<Set<string>>(getInitialSelectedFields());
2929

30-
const handleToggleField = (fieldName: string) => {
31-
if (fieldName === ACCESSION_VERSION_FIELD) {
32-
return;
33-
}
34-
30+
const handleFieldSelection = (fieldName: string, selected: boolean) => {
3531
const newSelectedFields = new Set(selectedFields);
36-
if (newSelectedFields.has(fieldName)) {
37-
newSelectedFields.delete(fieldName);
38-
} else {
32+
33+
if (selected) {
3934
newSelectedFields.add(fieldName);
35+
} else {
36+
newSelectedFields.delete(fieldName);
4037
}
41-
newSelectedFields.add(ACCESSION_VERSION_FIELD);
4238

4339
setSelectedFields(newSelectedFields);
4440
onSave(Array.from(newSelectedFields));
4541
};
4642

47-
const handleSelectAll = () => {
48-
const newSelectedFields = new Set<string>();
49-
metadata.forEach((field) => {
50-
newSelectedFields.add(field.name);
51-
});
52-
setSelectedFields(newSelectedFields);
53-
onSave(Array.from(newSelectedFields));
54-
};
55-
56-
const handleSelectNone = () => {
57-
const newSelectedFields = new Set<string>();
58-
newSelectedFields.add(ACCESSION_VERSION_FIELD);
59-
setSelectedFields(newSelectedFields);
60-
onSave(Array.from(newSelectedFields));
61-
};
62-
63-
// Group fields by header
64-
const fieldsByHeader = metadata.reduce<Record<string, Metadata[]>>((acc, field) => {
65-
const header = field.header ?? 'Other';
66-
acc[header] = acc[header] ?? [];
67-
acc[header].push(field);
68-
return acc;
69-
}, {});
70-
71-
// Sort headers alphabetically, but keep "Other" at the end
72-
const sortedHeaders = Object.keys(fieldsByHeader).sort((a, b) => {
73-
if (a === 'Other') return 1;
74-
if (b === 'Other') return -1;
75-
return a.localeCompare(b);
76-
});
43+
const fieldItems: FieldItem[] = metadata.map((field) => ({
44+
name: field.name,
45+
displayName: field.displayName,
46+
header: field.header,
47+
alwaysSelected: field.name === ACCESSION_VERSION_FIELD,
48+
disabled: field.name === ACCESSION_VERSION_FIELD,
49+
}));
7750

7851
return (
79-
<BaseDialog title='Select Fields to Download' isOpen={isOpen} onClose={onClose} fullWidth={false}>
80-
<div className='min-w-[1000px]'></div>
81-
<div className='mt-2 flex justify-between px-2'>
82-
<div>
83-
<button
84-
type='button'
85-
className='text-sm text-primary-600 hover:text-primary-900 font-medium mr-4'
86-
onClick={handleSelectAll}
87-
>
88-
Select All
89-
</button>
90-
<button
91-
type='button'
92-
className='text-sm text-primary-600 hover:text-primary-900 font-medium'
93-
onClick={handleSelectNone}
94-
>
95-
Select None
96-
</button>
97-
</div>
98-
</div>
99-
<div className='mt-2 max-h-[60vh] overflow-y-auto p-2'>
100-
{sortedHeaders.map((header) => (
101-
<div key={header} className='mb-6'>
102-
<h3 className='font-medium text-lg mb-2 text-gray-700'>{header}</h3>
103-
<div className='grid grid-cols-1 md:grid-cols-2 gap-x-4 gap-y-2'>
104-
{fieldsByHeader[header]
105-
.sort((a, b) => {
106-
// Sort by order property if available, otherwise alphabetically by name
107-
if (a.order !== undefined && b.order !== undefined) {
108-
return a.order - b.order;
109-
} else if (a.order !== undefined) {
110-
return -1; // a has order, b doesn't, so a comes first
111-
} else if (b.order !== undefined) {
112-
return 1; // b has order, a doesn't, so b comes first
113-
}
114-
return a.name.localeCompare(b.name);
115-
})
116-
.map((field) => (
117-
<div key={field.name} className='flex items-center'>
118-
<input
119-
type='checkbox'
120-
id={`field-${field.name}`}
121-
className={`h-4 w-4 rounded border-gray-300 text-primary-600 focus:ring-primary-600 ${
122-
field.name === ACCESSION_VERSION_FIELD
123-
? 'opacity-60 cursor-not-allowed'
124-
: ''
125-
}`}
126-
checked={
127-
selectedFields.has(field.name) || field.name === ACCESSION_VERSION_FIELD
128-
}
129-
onChange={() => handleToggleField(field.name)}
130-
disabled={field.name === ACCESSION_VERSION_FIELD}
131-
/>
132-
<label
133-
htmlFor={`field-${field.name}`}
134-
className={`ml-2 text-sm ${field.name === ACCESSION_VERSION_FIELD ? 'text-gray-500' : 'text-gray-700'}`}
135-
>
136-
{field.displayName ?? field.name}
137-
</label>
138-
</div>
139-
))}
140-
</div>
141-
</div>
142-
))}
143-
144-
<div className='mt-6 flex justify-end'>
145-
<button type='button' className='btn loculusColor text-white -py-1' onClick={onClose}>
146-
Done
147-
</button>
148-
</div>
149-
</div>
150-
</BaseDialog>
52+
<CommonFieldSelectorModal
53+
title='Select Fields to Download'
54+
isOpen={isOpen}
55+
onClose={onClose}
56+
fields={fieldItems}
57+
selectedFields={selectedFields}
58+
setFieldSelected={handleFieldSelection}
59+
/>
15160
);
15261
};
15362

0 commit comments

Comments
 (0)