From d6f1dae73ea3963e105eb9c269ad9ebf8efd2829 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 12 Aug 2024 15:03:15 -0500 Subject: [PATCH 1/2] focus name field on API name already exists error --- app/components/form/SideModalForm.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx index e0e02b7cbe..8367a4d981 100644 --- a/app/components/form/SideModalForm.tsx +++ b/app/components/form/SideModalForm.tsx @@ -56,7 +56,7 @@ type SideModalFormProps = { * slightly awkward but it also makes some sense. I do not believe there is * any way to distinguish between fresh pageload and back/forward. */ -export function useShouldAnimateModal() { +function useShouldAnimateModal() { return useNavigationType() === NavigationType.Push } @@ -80,7 +80,7 @@ export function SideModalForm({ useEffect(() => { if (submitError?.errorCode === 'ObjectAlreadyExists' && 'name' in form.getValues()) { // @ts-expect-error - form.setError('name', { message: 'Name already exists' }) + form.setError('name', { message: 'Name already exists' }, { shouldFocus: true }) } }, [submitError, form]) From 9f5401c8e957df1a086d07accba19e70df83161f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 12 Aug 2024 16:35:07 -0500 Subject: [PATCH 2/2] test passes with fix, fails without it --- app/ui/lib/SideModal.tsx | 1 + test/e2e/silos.e2e.ts | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx index 9f461aad97..fd587f2c48 100644 --- a/app/ui/lib/SideModal.tsx +++ b/app/ui/lib/SideModal.tsx @@ -161,6 +161,7 @@ function SideModalBody({ children }: { children?: ReactNode }) { 'body relative h-full overflow-y-auto pb-12 pt-8', !scrollStart && 'border-t border-t-secondary' )} + data-testid="sidemodal-scroll-container" > {children} diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 445483d49f..1c912251b1 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -261,3 +261,25 @@ test('Silo IP pools link pool', async ({ page }) => { await expect(modal).toBeHidden() await expectRowVisible(table, { name: 'ip-pool-3', Default: '' }) }) + +// just a convenient form to test this with because it's tall +test('form scrolls to name field on already exists error', async ({ page }) => { + await page.setViewportSize({ width: 800, height: 400 }) + await page.goto('/system/silos-new') + + const nameField = page.getByRole('textbox', { name: 'Name', exact: true }) + await expect(nameField).toBeInViewport() + + await nameField.fill('maze-war') + + // scroll all the way down so the name field is not visible + await page + .getByTestId('sidemodal-scroll-container') + .evaluate((el: HTMLElement, to) => el.scrollTo(0, to), 800) + await expect(nameField).not.toBeInViewport() + + await page.getByRole('button', { name: 'Create silo' }).click() + + await expect(nameField).toBeInViewport() + await expect(page.getByText('name already exists').nth(0)).toBeVisible() +})