From a56942abcf985e4665c38cc069ff2f9aacbd389c Mon Sep 17 00:00:00 2001 From: Gregor Vostrak Date: Tue, 30 Jun 2026 16:28:09 +0200 Subject: [PATCH] Add virtualizer to ProjectDropdown, ClientDropdown and Reporting Comboboxes; Remove redundant focus loop on Project/ClientDropdown --- e2e/projects.spec.ts | 37 ++++++++ e2e/reporting.spec.ts | 31 +++++++ e2e/tasks.spec.ts | 43 +++++++++ .../Common/Project/ProjectDropdown.vue | 49 +++++----- .../packages/ui/src/Client/ClientDropdown.vue | 89 +++++++++---------- .../ui/src/Input/MultiselectDropdown.vue | 78 +++++++++------- 6 files changed, 227 insertions(+), 100 deletions(-) diff --git a/e2e/projects.spec.ts b/e2e/projects.spec.ts index 24ec45b2..0d6a1442 100644 --- a/e2e/projects.spec.ts +++ b/e2e/projects.spec.ts @@ -117,6 +117,43 @@ test('test that archiving and unarchiving projects works', async ({ page, ctx }) await expect(page.getByText(newProjectName)).toBeVisible(); }); +test('test that the client can be changed in the edit project modal', async ({ page, ctx }) => { + const projectName = 'Edit Client Project ' + Math.floor(1 + Math.random() * 100000); + const clientName = 'Assigned Client ' + Math.floor(1 + Math.random() * 100000); + await createProjectViaApi(ctx, { name: projectName }); + const client = await createClientViaApi(ctx, { name: clientName }); + + await page.goto(PLAYWRIGHT_BASE_URL + '/projects'); + await expect(page.getByText(projectName)).toBeVisible({ timeout: 10000 }); + + // Open the project's Edit modal. + await page.getByRole('row').first().getByRole('button').click(); + await page.getByRole('menuitem').getByText('Edit').first().click(); + await expect(page.getByRole('dialog')).toBeVisible(); + + // Open the client dropdown (currently "No Client"), confirm it focuses, and pick the client. + await page.getByRole('dialog').getByRole('button', { name: 'No Client' }).click(); + const clientSearch = page.getByPlaceholder('Search for a client...'); + await expect(clientSearch).toBeFocused(); + await clientSearch.fill(clientName); + await page.getByRole('option', { name: clientName }).click(); + + // The trigger updates to the chosen client. + await expect(page.getByRole('dialog').getByRole('button', { name: clientName })).toBeVisible(); + + // Saving persists the client assignment. + await Promise.all([ + page.getByRole('button', { name: 'Update Project' }).click(), + page.waitForResponse( + async (response) => + response.url().includes('/projects/') && + response.request().method() === 'PUT' && + response.status() === 200 && + (await response.json()).data.client_id === client.id + ), + ]); +}); + test('test that updating billable rate works with existing time entries', async ({ page, ctx }) => { const newProjectName = 'New Project ' + Math.floor(1 + Math.random() * 10000); const newBillableRate = Math.round(Math.random() * 10000); diff --git a/e2e/reporting.spec.ts b/e2e/reporting.spec.ts index 85cf77b5..d3d43852 100644 --- a/e2e/reporting.spec.ts +++ b/e2e/reporting.spec.ts @@ -96,6 +96,37 @@ test('test that project multiselect search filters the option list', async ({ pa await page.keyboard.press('Escape'); }); +test('test that the project filter virtualizes a long list (renders only a window)', async ({ + page, + ctx, +}) => { + // Create many projects so the dropdown must virtualize rather than render all of them. + const projectNames = Array.from( + { length: 80 }, + (_, i) => `VirtProj ${String(i).padStart(2, '0')}` + ); + await Promise.all(projectNames.map((name) => createProjectViaApi(ctx, { name }))); + + await goToReporting(page); + await expect(page.getByRole('button', { name: 'Export' })).toBeVisible(); + await page.getByRole('button', { name: 'Projects' }).first().click(); + + // Only a small window of options is mounted, far fewer than the 80+ projects that exist. + await expect(page.getByRole('option').first()).toBeVisible(); + const renderedCount = await page.getByRole('option').count(); + expect(renderedCount).toBeGreaterThan(0); + expect(renderedCount).toBeLessThan(60); + + // Virtualization must not drop options: searching narrows the list to the one deep match. + // Wait for the filtered count to settle to 1 before asserting — checking the option while + // the virtualizer is still re-rendering can transiently match a stale row (Firefox CI flake). + await page.getByPlaceholder('Search for a Project...').fill('VirtProj 79'); + await expect(page.getByRole('option')).toHaveCount(1); + await expect(page.getByRole('option')).toContainText('VirtProj 79'); + + await page.keyboard.press('Escape'); +}); + test('test that selecting multiple projects shows correct badge count', async ({ page, ctx }) => { const project1Name = 'MultiProj1 ' + Math.floor(Math.random() * 10000); const project2Name = 'MultiProj2 ' + Math.floor(Math.random() * 10000); diff --git a/e2e/tasks.spec.ts b/e2e/tasks.spec.ts index d8abdf53..a5bfaa21 100644 --- a/e2e/tasks.spec.ts +++ b/e2e/tasks.spec.ts @@ -152,6 +152,49 @@ test('test that editing a task name works', async ({ page, ctx }) => { await expect(page.getByTestId('task_table')).not.toContainText(originalTaskName); }); +test('test that the project can be searched and changed in the create task modal', async ({ + page, + ctx, +}) => { + const sourceProject = 'Source Project ' + Math.floor(1 + Math.random() * 100000); + const targetProject = 'Target Project ' + Math.floor(1 + Math.random() * 100000); + await createProjectViaApi(ctx, { name: sourceProject }); + const target = await createProjectViaApi(ctx, { name: targetProject }); + + await goToProjectsOverview(page); + await page.getByText(sourceProject).first().click(); + await page.getByRole('button', { name: 'Create Task' }).click(); + await expect(page.getByRole('dialog')).toBeVisible(); + + // The project dropdown is pre-filled with the source project; open it. + await page.getByRole('dialog').getByRole('button', { name: sourceProject }).click(); + + // Opening the dropdown focuses the search input; searching narrows it to the target project. + const projectSearch = page.getByPlaceholder('Search for a project...'); + await expect(projectSearch).toBeFocused(); + await projectSearch.fill('Target Project'); + await page.getByRole('option', { name: targetProject }).click(); + + // Selecting closes the dropdown and updates the trigger to the chosen project. + await expect( + page.getByRole('dialog').getByRole('button', { name: targetProject }) + ).toBeVisible(); + + // The new selection is what gets used when the task is created. + const taskName = 'Switched Task ' + Math.floor(1 + Math.random() * 100000); + await page.getByPlaceholder('Task Name').fill(taskName); + await Promise.all([ + page.getByRole('dialog').getByRole('button', { name: 'Create Task' }).click(), + page.waitForResponse( + async (response) => + response.url().includes('/tasks') && + response.request().method() === 'POST' && + response.status() === 201 && + (await response.json()).data.project_id === target.id + ), + ]); +}); + test('test that creating a project with an existing client works', async ({ page, ctx }) => { const clientName = 'Existing Client ' + Math.floor(1 + Math.random() * 10000); const projectName = 'Project With Client ' + Math.floor(1 + Math.random() * 10000); diff --git a/resources/js/Components/Common/Project/ProjectDropdown.vue b/resources/js/Components/Common/Project/ProjectDropdown.vue index bacceb8e..c37de296 100644 --- a/resources/js/Components/Common/Project/ProjectDropdown.vue +++ b/resources/js/Components/Common/Project/ProjectDropdown.vue @@ -9,10 +9,10 @@ import { ComboboxItem, ComboboxRoot, ComboboxViewport, -} from 'radix-vue'; + ComboboxVirtualizer, +} from 'reka-ui'; import { Check, Plus } from '@lucide/vue'; import type { CreateClientBody, CreateProjectBody, Project } from '@/packages/api/src'; -import { UseFocusTrap } from '@vueuse/integrations/useFocusTrap/component'; import ProjectCreateModal from '@/packages/ui/src/Project/ProjectCreateModal.vue'; import { useProjectsStore } from '@/utils/useProjects'; import { useClientsStore } from '@/utils/useClients'; @@ -103,40 +103,45 @@ function updateValue(project: Project) { (null); const open = ref(false); const searchValue = ref(''); -function isClientSelected(id: string) { - return model.value === id; -} - watch(open, (isOpen) => { if (isOpen) { nextTick(() => { @@ -58,15 +54,23 @@ async function addClientIfNoneExists() { } } +const NO_CLIENT: { id: string | null; name: string } = { id: null, name: 'No Client' }; + const currentClient = computed(() => { - return ( - props.clients.find((client) => client.id === model.value) ?? { - id: null, - name: 'No Client', - } - ); + return props.clients.find((client) => client.id === model.value) ?? NO_CLIENT; }); +type ClientRow = Client | typeof NO_CLIENT; + +// Fold the "No Client" entry in as the first row so the whole list virtualizes through one +// ComboboxVirtualizer. NO_CLIENT is a shared constant so currentClient and the row reference +// the same object and single-select highlighting still matches. +const clientRows = computed(() => [NO_CLIENT, ...filteredClients.value]); + +function clientRowName(row: ClientRow) { + return row.name; +} + const emit = defineEmits(['update:modelValue', 'changed']); function updateValue(client: { id: string | null; name: string }) { @@ -81,56 +85,51 @@ function updateValue(client: { id: string | null; name: string }) { diff --git a/resources/js/packages/ui/src/Input/MultiselectDropdown.vue b/resources/js/packages/ui/src/Input/MultiselectDropdown.vue index 969cd41d..9fd314b8 100644 --- a/resources/js/packages/ui/src/Input/MultiselectDropdown.vue +++ b/resources/js/packages/ui/src/Input/MultiselectDropdown.vue @@ -9,10 +9,16 @@ import { ComboboxItem, ComboboxRoot, ComboboxViewport, -} from 'radix-vue'; + ComboboxVirtualizer, +} from 'reka-ui'; const NONE_ID = 'none'; +// height of one row (px-2 py-1.5 text-sm → 12px padding + 20px line box). +// Rows are uniform single-line, so a fixed size is exact enough for the virtualizer and avoids +// any per-row DOM measurement. +const ROW_HEIGHT = 32; + const model = defineModel({ default: [], }); @@ -56,6 +62,23 @@ const showNoItem = computed(() => { return props.noItemLabel.toLowerCase().includes(search); }); +// A single flat list for the virtualizer. The optional "no item" entry is folded in as the +// first row so the whole list (including it) is virtualized through one ComboboxVirtualizer. +type Row = { kind: 'none' } | { kind: 'item'; item: T }; + +const rows = computed(() => { + const itemRows = filteredItems.value.map((item): Row => ({ kind: 'item', item })); + return showNoItem.value ? [{ kind: 'none' }, ...itemRows] : itemRows; +}); + +function keyForRow(row: Row): string { + return row.kind === 'none' ? NONE_ID : props.getKeyFromItem(row.item); +} + +function nameForRow(row: Row): string { + return row.kind === 'none' ? (props.noItemLabel ?? '') : props.getNameForItem(row.item); +} + function toggleItem(id: string) { if (model.value.includes(id)) { model.value = model.value.filter((itemId) => itemId !== id); @@ -74,46 +97,35 @@ const emit = defineEmits(['update:modelValue', 'changed', 'submit']);