Skip to content
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

Focus name field on API name already exists error #2366

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Focus name field on API name already exists error #2366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6f1dae
focus name field on API name already exists error
david-crespo Aug 12, 2024
9f5401c
test passes with fix, fails without it
david-crespo Aug 12, 2024
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
4 changes: 2 additions & 2 deletions app/components/form/SideModalForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type SideModalFormProps<TFieldValues extends FieldValues> = {
* 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() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasn't being used elsewhere, and it being exported messes up HMR

return useNavigationType() === NavigationType.Push
}

Expand All @@ -80,7 +80,7 @@ export function SideModalForm<TFieldValues extends FieldValues>({
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])

Expand Down
1 change: 1 addition & 0 deletions app/ui/lib/SideModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
</div>
Expand Down
22 changes: 22 additions & 0 deletions test/e2e/silos.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Loading