From fc81db9020e152c2bcb40deba0a3c6190f1959f0 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 04:11:14 -0400 Subject: [PATCH 1/4] fix(forms): stop typing at each field's length limit instead of refusing it on submit Text inputs whose zod schema caps a length now carry a matching `maxLength`, read from the schema itself (`Schema.shape..maxLength`) so the attribute and the validation cannot come apart. That covers the organization role name, the add/invite user email, the instance role name, sign-up first/last name and email, profile first/last name, the new application name, the add/rename file names, and the import modal's database and table names. Password inputs are deliberately left without `maxLength`: a clamped paste would save a password in the password manager that never worked, so they keep showing the validation error instead. The profile form's name limits (50/50) disagreed with central manager's `validateUpdateUser` (first name 40, last name 80), so a 41-50 character first name passed the form and was refused by the server, and a 51-80 character last name that sign-up accepted could never be saved again. They now match the server and the sign-up form. Closes #1290 Co-Authored-By: Claude Opus 5.5 --- src/features/auth/ResetPassword.test.tsx | 40 +++++++++++++++ src/features/auth/SignUp.test.tsx | 11 ++++ src/features/auth/SignUp.tsx | 3 ++ .../NewApplication/CommonInstructions.tsx | 1 + .../NewApplication/NewApplication.test.tsx | 6 +++ .../modals/AddDirectoryOrFileModal.test.tsx | 50 +++++++++++++++++++ .../modals/AddDirectoryOrFileModal.tsx | 1 + .../modals/RenameFileModal.test.tsx | 6 +++ .../applications/modals/RenameFileModal.tsx | 1 + .../config/roles/modals/AddRoleModal.test.tsx | 6 +++ .../config/roles/modals/AddRoleModal.tsx | 2 +- .../ImportDataModal.permissions.test.tsx | 9 ++++ .../databases/modals/ImportDataModal.tsx | 2 + .../modals/AddOrganizationRoleModal.test.tsx | 6 +++ .../roles/modals/AddOrganizationRoleModal.tsx | 7 ++- .../users/modals/AddUserModal.email.test.tsx | 39 +++++++++++++++ .../users/modals/AddUserModal.tsx | 8 ++- src/features/profile/ProfileIndex.test.tsx | 27 ++++++++++ src/features/profile/index.tsx | 2 + .../profile/mutations/updateUserSchema.ts | 4 +- 20 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 src/features/auth/ResetPassword.test.tsx create mode 100644 src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx create mode 100644 src/features/organization/users/modals/AddUserModal.email.test.tsx diff --git a/src/features/auth/ResetPassword.test.tsx b/src/features/auth/ResetPassword.test.tsx new file mode 100644 index 000000000..dcccf6d4f --- /dev/null +++ b/src/features/auth/ResetPassword.test.tsx @@ -0,0 +1,40 @@ +/** + * @vitest-environment jsdom + */ +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => vi.fn(), + useSearch: () => ({ token: 'reset-token' }), +})); +vi.mock('sonner', () => ({ toast: { success: vi.fn(), dismiss: vi.fn() } })); +const { mutate } = vi.hoisted(() => ({ mutate: vi.fn() })); +vi.mock('./hooks/useResetPassword', () => ({ useResetPasswordMutation: () => ({ mutate, isPending: false }) })); + +import { ResetPassword } from './ResetPassword'; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ResetPassword', () => { + // maxlength would silently clamp a pasted password, leaving the password manager holding one that never worked. + it('lets an over-long password be typed in full and explains the limit instead', async () => { + render(); + const password = screen.getByLabelText('New Password') as HTMLInputElement; + const confirmation = screen.getByLabelText('Confirm Password') as HTMLInputElement; + expect(password.maxLength).toBe(-1); + expect(confirmation.maxLength).toBe(-1); + + await act(async () => { + fireEvent.change(password, { target: { value: 'p'.repeat(51) } }); + fireEvent.change(confirmation, { target: { value: 'p'.repeat(51) } }); + fireEvent.click(screen.getByRole('button', { name: 'Submit New Password' })); + }); + + expect(await screen.findByText('Password cannot be longer than 50 characters.')).toBeTruthy(); + expect(mutate).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/auth/SignUp.test.tsx b/src/features/auth/SignUp.test.tsx index a5519625c..d570cd9b8 100644 --- a/src/features/auth/SignUp.test.tsx +++ b/src/features/auth/SignUp.test.tsx @@ -98,6 +98,17 @@ beforeEach(() => { afterEach(() => vi.clearAllMocks()); describe('SignUp', () => { + it('stops names and email at their limits but lets a password run long', () => { + renderSignUp(); + const maxLengthOf = (label: string) => (screen.getByLabelText(label) as HTMLInputElement).maxLength; + + expect(maxLengthOf('First Name')).toBe(40); + expect(maxLengthOf('Last Name')).toBe(80); + expect(maxLengthOf('Email')).toBe(80); + expect(maxLengthOf('Password')).toBe(-1); + expect(maxLengthOf('Confirm Password')).toBe(-1); + }); + it('reveals each password independently without submitting or losing field values', () => { renderSignUp(); const password = screen.getByLabelText('Password') as HTMLInputElement; diff --git a/src/features/auth/SignUp.tsx b/src/features/auth/SignUp.tsx index 8e1dd30d1..7b22b2692 100644 --- a/src/features/auth/SignUp.tsx +++ b/src/features/auth/SignUp.tsx @@ -222,6 +222,7 @@ export function SignUp() { diff --git a/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx b/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx index a6acde1ec..6002d8593 100644 --- a/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx +++ b/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx @@ -39,6 +39,7 @@ export function CommonInstructions({ autoComplete="off" autoFocus={true} placeholder={defaultApplicationName} + maxLength={NewApplicationSchema.shape.applicationName.maxLength!} {...field} /> diff --git a/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx b/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx index 3078936a6..d3279dc11 100644 --- a/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx +++ b/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx @@ -98,6 +98,12 @@ describe('NewApplication', () => { expect(screen.queryByText('Please enter a URL or package reference.')).toBeNull(); }); + it('stops the application name at the 75 characters the form accepts', () => { + render(); + + expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(75); + }); + it('explains a refused application name when Enter is pressed in it', async () => { render(); await act(async () => { diff --git a/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx b/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx new file mode 100644 index 000000000..b7cc9c2da --- /dev/null +++ b/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx @@ -0,0 +1,50 @@ +/** + * @vitest-environment jsdom + */ +import type { FileEntry } from '@/features/instance/applications/context/fileEntry'; +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +const openedEntry: FileEntry = { name: 'index.js', path: 'my-app/index.js', project: 'my-app' } as FileEntry; + +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => ({ entityId: 'test-instance', entityType: 'instance' }), +})); +vi.mock('@/features/instance/applications/hooks/useEditorView', () => ({ + useEditorView: () => ({ + openedEntry, + entryExists: () => false, + reloadRootEntries: vi.fn(), + setFocusedItem: vi.fn(), + setSelectedItems: vi.fn(), + setExpandedItems: vi.fn(), + }), +})); +vi.mock('@/integrations/api/instance/applications/setComponentFile', () => ({ + useSetComponentFile: () => ({ mutate: vi.fn(), isPending: false }), +})); +vi.mock('@/lib/events/watcher', () => ({ + useWatchedValue: () => ({ value: 'file', trigger: undefined }), + setWatchedValue: vi.fn(), +})); +vi.mock('@/lib/attemptToRestoreFocus', () => ({ attemptToRestoreFocus: vi.fn() })); + +import { AddDirectoryOrFileModal } from './AddDirectoryOrFileModal'; + +beforeAll(() => { + // Radix Dialog relies on DOM APIs jsdom doesn't implement. + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; +}); + +afterEach(cleanup); + +describe('AddDirectoryOrFileModal', () => { + it('stops the name at the 50 characters the form accepts', () => { + render(); + + expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(50); + }); +}); diff --git a/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx b/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx index 84f4ce94e..051bcf713 100644 --- a/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx +++ b/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx @@ -171,6 +171,7 @@ export function AddDirectoryOrFileModal() { type="text" autoComplete="off" autoCapitalize="off" + maxLength={NewFileFolderSchema.shape.name.maxLength!} {...field} /> diff --git a/src/features/instance/applications/modals/RenameFileModal.test.tsx b/src/features/instance/applications/modals/RenameFileModal.test.tsx index d3404ab1a..c09c2416a 100644 --- a/src/features/instance/applications/modals/RenameFileModal.test.tsx +++ b/src/features/instance/applications/modals/RenameFileModal.test.tsx @@ -52,6 +52,12 @@ function renameButton() { } describe('RenameFileModal', () => { + it('stops the name at the 50 characters the form accepts', () => { + render(); + + expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(50); + }); + it('shows the reason when Enter is pressed, then clears it once the name is fixed', async () => { render(); await typeName('index(1).js'); diff --git a/src/features/instance/applications/modals/RenameFileModal.tsx b/src/features/instance/applications/modals/RenameFileModal.tsx index 6346b9b0d..4cd3d3b3c 100644 --- a/src/features/instance/applications/modals/RenameFileModal.tsx +++ b/src/features/instance/applications/modals/RenameFileModal.tsx @@ -186,6 +186,7 @@ export function RenameFileModal() { type="text" autoComplete="off" autoCapitalize="off" + maxLength={RenameFileSchema.shape.name.maxLength!} {...field} /> diff --git a/src/features/instance/config/roles/modals/AddRoleModal.test.tsx b/src/features/instance/config/roles/modals/AddRoleModal.test.tsx index 3c5dc8865..b4d5fe2e7 100644 --- a/src/features/instance/config/roles/modals/AddRoleModal.test.tsx +++ b/src/features/instance/config/roles/modals/AddRoleModal.test.tsx @@ -99,4 +99,10 @@ describe('AddRoleModal', () => { await screen.findByRole('radio', { name: /Super user/ }); expect(screen.queryByRole('radio', { name: /Cluster user/ })).toBeNull(); }); + + it('stops the role name at the 30 characters the form accepts', () => { + renderModal(); + + expect((screen.getByRole('textbox', { name: 'Name' }) as HTMLInputElement).maxLength).toBe(30); + }); }); diff --git a/src/features/instance/config/roles/modals/AddRoleModal.tsx b/src/features/instance/config/roles/modals/AddRoleModal.tsx index 6e5167aa9..6652c0443 100644 --- a/src/features/instance/config/roles/modals/AddRoleModal.tsx +++ b/src/features/instance/config/roles/modals/AddRoleModal.tsx @@ -100,7 +100,7 @@ export function AddRoleModal({ Name - + diff --git a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx index 2530bbc22..b99f2aa01 100644 --- a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx @@ -129,3 +129,12 @@ describe('ImportDataModal requested method', () => { expect(selectedMethod()).toBe('import-method-url'); }); }); + +describe('ImportDataModal name fields', () => { + it('stops the database and table names at the lengths the form accepts', () => { + renderModal({ super_user: true }); + + expect((screen.getByLabelText('Database') as HTMLInputElement).maxLength).toBe(75); + expect((screen.getByLabelText('Table') as HTMLInputElement).maxLength).toBe(250); + }); +}); diff --git a/src/features/instance/databases/modals/ImportDataModal.tsx b/src/features/instance/databases/modals/ImportDataModal.tsx index 5ea32e843..9bb95ef3a 100644 --- a/src/features/instance/databases/modals/ImportDataModal.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.tsx @@ -530,6 +530,7 @@ export function ImportDataModal({ {...field} type="text" placeholder="data" + maxLength={ImportDataFormSchema.shape.database.maxLength!} autoCapitalize="off" autoComplete="off" autoCorrect="off" @@ -549,6 +550,7 @@ export function ImportDataModal({ { expect(await screen.findByText('Role must contain only letters and underscores.')).toBeTruthy(); }); + it('stops the role name at the 30 characters the form accepts', () => { + render( {}} />); + + expect((screen.getByLabelText('Role Name') as HTMLInputElement).maxLength).toBe(30); + }); + it.each([ ['a space', 'my role', 'Role must contain only letters and underscores.'], ['more than 30 characters', 'a'.repeat(31), 'Role name cannot be longer than 30 characters.'], diff --git a/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx b/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx index c27896f99..a89616310 100644 --- a/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx +++ b/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx @@ -125,7 +125,12 @@ export function AddOrganizationRoleModal({ Role Name - + diff --git a/src/features/organization/users/modals/AddUserModal.email.test.tsx b/src/features/organization/users/modals/AddUserModal.email.test.tsx new file mode 100644 index 000000000..9096f6c53 --- /dev/null +++ b/src/features/organization/users/modals/AddUserModal.email.test.tsx @@ -0,0 +1,39 @@ +/** + * @vitest-environment jsdom + */ +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } })); +vi.mock('@tanstack/react-router', () => ({ useParams: () => ({ organizationId: 'org-1' }) })); +vi.mock('@tanstack/react-query', async (importOriginal) => ({ + ...(await importOriginal()), + useQuery: () => ({ data: [], isLoading: false }), +})); +vi.mock('@/features/organization/mutations/addUserToOrganizationRole', async (importOriginal) => ({ + ...(await importOriginal()), + useAddUserToOrganizationRole: () => ({ mutate: vi.fn(), isPending: false }), +})); +vi.mock('@/features/organization/mutations/inviteUserToOrganizationRole', () => ({ + useInviteUserToOrganizationRole: () => ({ mutate: vi.fn(), isPending: false }), +})); + +import { AddUserModal } from './AddUserModal'; + +beforeAll(() => { + // Radix Dialog relies on DOM APIs jsdom doesn't implement. + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; +}); + +afterEach(cleanup); + +describe('AddUserModal', () => { + it('stops the email at the 80 characters the form accepts', () => { + render( {}} setIsModalOpen={() => {}} />); + + expect((screen.getByLabelText('Email') as HTMLInputElement).maxLength).toBe(80); + }); +}); diff --git a/src/features/organization/users/modals/AddUserModal.tsx b/src/features/organization/users/modals/AddUserModal.tsx index 0f1a7246e..764cfe5cb 100644 --- a/src/features/organization/users/modals/AddUserModal.tsx +++ b/src/features/organization/users/modals/AddUserModal.tsx @@ -120,7 +120,13 @@ export function AddUserModal({ Email - + diff --git a/src/features/profile/ProfileIndex.test.tsx b/src/features/profile/ProfileIndex.test.tsx index d693c8f06..f92a0b2be 100644 --- a/src/features/profile/ProfileIndex.test.tsx +++ b/src/features/profile/ProfileIndex.test.tsx @@ -39,6 +39,33 @@ function saveButton() { } describe('ProfileIndex', () => { + it('stops each name at its limit but lets a new password run long', () => { + render(); + const maxLengthOf = (label: string) => (screen.getByLabelText(label) as HTMLInputElement).maxLength; + + expect(maxLengthOf('First Name')).toBe(40); + expect(maxLengthOf('Last Name')).toBe(80); + expect(maxLengthOf('New Password')).toBe(-1); + expect(maxLengthOf('Confirm New Password')).toBe(-1); + }); + + it('refuses a first name longer than central manager accepts once the field is left', async () => { + render(); + await type('First Name', 'A'.repeat(41)); + await leave('First Name'); + + expect(await screen.findByText('First name cannot be longer than 40 characters.')).toBeTruthy(); + expect(saveButton().disabled).toBe(true); + }); + + it('accepts a last name as long as sign-up allows', async () => { + render(); + await type('Last Name', 'L'.repeat(80)); + await leave('Last Name'); + + await waitFor(() => expect(saveButton().disabled).toBe(false)); + }); + it('explains a short new password when Enter is pressed in it', async () => { render(); await type('New Password', 'short'); diff --git a/src/features/profile/index.tsx b/src/features/profile/index.tsx index 9494bf21d..30598b02d 100644 --- a/src/features/profile/index.tsx +++ b/src/features/profile/index.tsx @@ -110,6 +110,7 @@ export function ProfileIndex() { type="text" className="bg-background/60" autoCapitalize="words" + maxLength={UpdateUserSchema.shape.firstname.maxLength!} {...field} /> @@ -129,6 +130,7 @@ export function ProfileIndex() { type="text" className="bg-background/60" autoCapitalize="words" + maxLength={UpdateUserSchema.shape.lastname.maxLength!} {...field} /> diff --git a/src/features/profile/mutations/updateUserSchema.ts b/src/features/profile/mutations/updateUserSchema.ts index 40e39a719..9abd86380 100644 --- a/src/features/profile/mutations/updateUserSchema.ts +++ b/src/features/profile/mutations/updateUserSchema.ts @@ -8,13 +8,13 @@ export const UpdateUserSchema = z error: 'Please enter your first name.', }) .min(2, { error: 'First name is required.' }) - .max(50, { error: 'First name must be less than 50 characters.' }), + .max(40, { error: 'First name cannot be longer than 40 characters.' }), lastname: z .string({ error: 'Please enter your last name.', }) .min(2, { error: 'Last name is required.' }) - .max(50, { error: 'Last name must be less than 50 characters.' }), + .max(80, { error: 'Last name cannot be longer than 80 characters.' }), newPassword: z .string({ error: 'Please enter your new password.', From 0a11e205ca128079a732d12dbb8a8d90e33d4e46 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 12 Oct 2026 17:38:05 -0400 Subject: [PATCH 2/4] fix(forms): say when a field hits its length limit, and when a paste was cut A hard maxLength stops typing at the limit, but it also truncates a longer paste without a word, so a password-manager or clipboard value can land cut short with nothing on screen to say so. Every field #1888 limits now says so under the input, in amber rather than the red of a validation error, since the value is valid: - "Maximum N characters reached." once the value reaches the limit, cleared again once it drops back under. A value that starts over the limit, such as a Profile name saved under the old 50-character cap, says how long it is. - "... Pasted text was P characters; only the first K fit." (or "none of it fit") when a paste was cut, measured once the browser has inserted it: from the selection where the input exposes one, and, where type="email" hides it, by reading the new value as the old one with the paste spliced in, so text already in the field never counts as pasted. It counts what actually landed, which is one short of the room when the browser won't split a surrogate pair, and it stays until the value changes. A schema without .max() gives zod's maxLength as null, which leaves the field alone rather than showing "Maximum null characters". The hint is an always-rendered aria-live="polite" region tied to the input by aria-describedby (FormControl now appends a caller's id rather than dropping it), and shows only the paste message after a cut paste rather than announcing the limit first. Password fields get neither a maxLength nor a hint. LengthLimitedInput wraps FormControl + Input for the plain fields; AuthInput uses the same useLengthLimit hook, so sign-up's names and email get it too. Co-Authored-By: Claude Opus 5.5 --- src/components/ui/form/FormControl.tsx | 4 +- .../ui/form/LengthLimitedInput.test.tsx | 163 ++++++++++++++++++ src/components/ui/form/LengthLimitedInput.tsx | 19 ++ .../ui/form/measurePasteCut.test.ts | 100 +++++++++++ src/components/ui/form/measurePasteCut.ts | 60 +++++++ src/components/ui/form/useFormField.tsx | 1 + src/components/ui/form/useLengthLimit.tsx | 94 ++++++++++ src/features/auth/AuthLayout.css | 2 +- src/features/auth/ResetPassword.test.tsx | 11 ++ src/features/auth/SignUp.test.tsx | 25 +++ src/features/auth/components/AuthInput.tsx | 41 +++-- .../NewApplication/CommonInstructions.tsx | 23 ++- .../NewApplication/NewApplication.test.tsx | 7 +- .../modals/AddDirectoryOrFileModal.test.tsx | 9 +- .../modals/AddDirectoryOrFileModal.tsx | 21 +-- .../modals/RenameFileModal.test.tsx | 7 +- .../applications/modals/RenameFileModal.tsx | 21 +-- .../config/roles/modals/AddRoleModal.test.tsx | 7 +- .../config/roles/modals/AddRoleModal.tsx | 7 +- .../ImportDataModal.permissions.test.tsx | 16 +- .../databases/modals/ImportDataModal.tsx | 39 ++--- .../modals/AddOrganizationRoleModal.test.tsx | 7 +- .../roles/modals/AddOrganizationRoleModal.tsx | 15 +- .../users/modals/AddUserModal.email.test.tsx | 9 +- .../users/modals/AddUserModal.tsx | 18 +- src/features/profile/ProfileIndex.test.tsx | 21 +++ src/features/profile/index.tsx | 33 ++-- 27 files changed, 641 insertions(+), 139 deletions(-) create mode 100644 src/components/ui/form/LengthLimitedInput.test.tsx create mode 100644 src/components/ui/form/LengthLimitedInput.tsx create mode 100644 src/components/ui/form/measurePasteCut.test.ts create mode 100644 src/components/ui/form/measurePasteCut.ts create mode 100644 src/components/ui/form/useLengthLimit.tsx diff --git a/src/components/ui/form/FormControl.tsx b/src/components/ui/form/FormControl.tsx index 92f31e466..8ce80f069 100644 --- a/src/components/ui/form/FormControl.tsx +++ b/src/components/ui/form/FormControl.tsx @@ -2,14 +2,14 @@ import { useFormField } from '@/components/ui/form/useFormField'; import { Slot } from '@radix-ui/react-slot'; import { ComponentProps } from 'react'; -export function FormControl({ ...props }: ComponentProps) { +export function FormControl({ 'aria-describedby': describedBy, ...props }: ComponentProps) { const { error, formItemId, formDescriptionId, formMessageId } = useFormField(); return ( diff --git a/src/components/ui/form/LengthLimitedInput.test.tsx b/src/components/ui/form/LengthLimitedInput.test.tsx new file mode 100644 index 000000000..6c6786338 --- /dev/null +++ b/src/components/ui/form/LengthLimitedInput.test.tsx @@ -0,0 +1,163 @@ +/** + * @vitest-environment jsdom + */ +import { Form } from '@/components/ui/form/Form'; +import { FormField } from '@/components/ui/form/FormField'; +import { FormItem } from '@/components/ui/form/FormItem'; +import { FormLabel } from '@/components/ui/form/FormLabel'; +import { FormMessage } from '@/components/ui/form/FormMessage'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { useForm } from 'react-hook-form'; +import { afterEach, describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { LengthLimitedInput } from './LengthLimitedInput'; + +function NameForm( + { type = 'text', name = '', maxLength = 10 }: { type?: 'text' | 'email'; name?: string; maxLength?: number }, +) { + const form = useForm({ defaultValues: { name } }); + return ( +
+ ( + + Name + + + + )} + /> + + ); +} + +afterEach(cleanup); + +const input = () => screen.getByLabelText('Name') as HTMLInputElement; +const hint = () => document.querySelector('[data-slot="form-length-hint"]')!; +const type = (value: string) => fireEvent.change(input(), { target: { value } }); +const description = () => + (input().getAttribute('aria-describedby') ?? '').split(' ').map(id => document.getElementById(id)?.textContent) + .join(' ').trim(); + +// jsdom has no paste default action, so `landed` stands in for what the browser inserts. +function startPaste(text: string, landed: string) { + fireEvent.paste(input(), { clipboardData: { getData: () => text } }); + type(landed); +} +async function paste(text: string, landed: string) { + startPaste(text, landed); + await act(() => new Promise(resolve => setTimeout(resolve))); +} + +describe('LengthLimitedInput', () => { + it('stops the input at the limit and says so, politely, in its accessible description', () => { + render(); + expect(input().maxLength).toBe(10); + + type('a'.repeat(10)); + + expect(hint().textContent).toBe('Maximum 10 characters reached.'); + expect(description()).toBe('Maximum 10 characters reached.'); + expect(hint().getAttribute('aria-live')).toBe('polite'); + }); + + it('says nothing below the limit, from a live region that is already in place', () => { + render(); + type('a'.repeat(9)); + + expect(hint().textContent).toBe(''); + expect(hint().getAttribute('aria-live')).toBe('polite'); + expect(input().getAttribute('aria-describedby')).toContain(hint().id); + }); + + it('clears once the value is back under the limit', () => { + render(); + type('a'.repeat(10)); + type('a'.repeat(9)); + + expect(hint().textContent).toBe(''); + }); + + it('says when a paste was cut, how long it was and how much of it fit', async () => { + render(); + await paste('p'.repeat(312), 'p'.repeat(10)); + + expect(description()).toBe( + 'Maximum 10 characters reached. Pasted text was 312 characters; only the first 10 fit.', + ); + }); + + it('counts only the room left beside what the field already held', async () => { + render(); + type('abcd'); + await paste('x'.repeat(20), `abcd${'x'.repeat(6)}`); + + expect(hint().textContent).toBe( + 'Maximum 10 characters reached. Pasted text was 20 characters; only the first 6 fit.', + ); + }); + + it('announces the cut paste alone, not the limit first and then the paste', async () => { + render(); + startPaste('p'.repeat(312), 'p'.repeat(10)); + + expect(hint().textContent).toBe(''); + await act(() => new Promise(resolve => setTimeout(resolve))); + expect(hint().textContent).toContain('Pasted text was 312 characters'); + }); + + it('keeps the paste notice while the value is what the paste left, and drops it on any change', async () => { + render(); + await paste('p'.repeat(312), 'p'.repeat(10)); + expect(hint().textContent).toContain('Pasted text was 312 characters'); + + type(`${'p'.repeat(9)}q`); + expect(hint().textContent).toBe('Maximum 10 characters reached.'); + + type('p'.repeat(9)); + expect(hint().textContent).toBe(''); + }); + + it('says none of a paste fit when the browser stopped short of a surrogate pair', async () => { + render(); + type('a'.repeat(9)); + await paste('πŸ˜€', 'a'.repeat(9)); + + expect(hint().textContent).toBe('Maximum 10 characters. Pasted text was 2 characters; none of it fit.'); + }); + + it('says nothing about a paste that fit', async () => { + render(); + await paste('abc', 'abc'); + + expect(hint().textContent).toBe(''); + }); + + it('says how far over the limit a value starts, such as a name saved before the limit was lowered', () => { + render(); + + expect(description()).toBe('Maximum 10 characters; this has 12.'); + }); + + it('leaves the field alone when its schema sets no maximum, where zod reports the limit as null', () => { + render(); + type('a'.repeat(50)); + + expect(input().maxLength).toBe(-1); + expect(document.querySelector('[data-slot="form-length-hint"]')).toBeNull(); + }); + + it('says when a paste into an email field was cut, though that field hides its selection', async () => { + render(); + expect(input().selectionStart).toBeNull(); + type('a@b.co'); + await paste(`bob@${'e'.repeat(40)}.com`, `bob@${'e'.repeat(6)}`); + + expect(hint().textContent).toBe( + 'Maximum 10 characters reached. Pasted text was 48 characters; only the first 10 fit.', + ); + }); +}); diff --git a/src/components/ui/form/LengthLimitedInput.tsx b/src/components/ui/form/LengthLimitedInput.tsx new file mode 100644 index 000000000..221c8cd62 --- /dev/null +++ b/src/components/ui/form/LengthLimitedInput.tsx @@ -0,0 +1,19 @@ +import { FormControl } from '@/components/ui/form/FormControl'; +import { useLengthLimit } from '@/components/ui/form/useLengthLimit'; +import { Input } from '@/components/ui/input'; +import { ComponentProps } from 'react'; + +export function LengthLimitedInput( + { maxLength, onPaste, ...props }: ComponentProps & { maxLength: number }, +) { + const { controlProps, inputProps, hint } = useLengthLimit(maxLength, onPaste); + + return ( + <> + + + + {hint} + + ); +} diff --git a/src/components/ui/form/measurePasteCut.test.ts b/src/components/ui/form/measurePasteCut.test.ts new file mode 100644 index 000000000..b542f0f5d --- /dev/null +++ b/src/components/ui/form/measurePasteCut.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from 'vitest'; +import { measurePasteCut } from './measurePasteCut'; + +const long = 'x'.repeat(312); + +describe('measurePasteCut', () => { + describe('with the selection known', () => { + it('reports what an empty field kept of a longer paste', () => { + expect(measurePasteCut({ text: long, before: '', after: long.slice(0, 80), selection: [0, 0], maxLength: 80 })) + .toEqual({ pasted: 312, kept: 80 }); + }); + + it('counts only the room left beside what the field already held', () => { + const before = 'a'.repeat(10); + const after = before + 'b'.repeat(70); + expect(measurePasteCut({ text: 'b'.repeat(75), before, after, selection: [10, 10], maxLength: 80 })) + .toEqual({ pasted: 75, kept: 70 }); + }); + + it('counts the selection the paste replaces as room', () => { + const before = 'a'.repeat(50); + const text = 'b'.repeat(80); + expect(measurePasteCut({ text, before, after: text, selection: [0, 50], maxLength: 80 })).toBeNull(); + }); + + it('says none of it fit when the field was already full', () => { + const full = 'a'.repeat(80); + expect(measurePasteCut({ text: 'hello', before: full, after: full, selection: [80, 80], maxLength: 80 })) + .toEqual({ pasted: 5, kept: 0 }); + }); + + it('counts what landed when the browser stopped short of a surrogate pair', () => { + const before = 'a'.repeat(79); + expect(measurePasteCut({ text: 'πŸ˜€', before, after: before, selection: [79, 79], maxLength: 80 })) + .toEqual({ pasted: 2, kept: 0 }); + }); + + it('ignores a paste that fit exactly', () => { + const text = 'b'.repeat(80); + expect(measurePasteCut({ text, before: '', after: text, selection: [0, 0], maxLength: 80 })).toBeNull(); + }); + + it('ignores the trailing line break a copied line carries, and counts each inner break as one space', () => { + const text = `${'b'.repeat(39)}\r\n${'c'.repeat(40)}\n`; + const after = `${'b'.repeat(39)} ${'c'.repeat(40)}`; + expect(measurePasteCut({ text, before: '', after, selection: [0, 0], maxLength: 80 })).toBeNull(); + }); + + it('ignores a paste that left the field short of its limit, which something other than the limit refused', () => { + expect(measurePasteCut({ text: long, before: 'abc', after: 'abc', selection: [3, 3], maxLength: 80 })).toBeNull(); + }); + }); + + describe('with the selection hidden, as type="email" keeps it', () => { + it('reports what an empty field kept', () => { + expect(measurePasteCut({ text: long, before: '', after: long.slice(0, 80), selection: null, maxLength: 80 })) + .toEqual({ pasted: 312, kept: 80 }); + }); + + it('reports the whole limit kept when the paste replaced an address sharing its start', () => { + const text = `alice@${'e'.repeat(100)}.com`; + expect( + measurePasteCut({ text, before: 'alice@x.com', after: text.slice(0, 80), selection: null, maxLength: 80 }), + ).toEqual({ pasted: text.length, kept: 80 }); + }); + + it('ignores a paste that replaced the value and fit', () => { + const text = `bob@${'e'.repeat(70)}.com`; + expect(measurePasteCut({ text, before: 'alice@x.com', after: text, selection: null, maxLength: 78 })) + .toBeNull(); + }); + + it('says none of it fit when the full value did not change, though the paste matches part of it', () => { + const full = `abc${'x'.repeat(77)}`; + expect(measurePasteCut({ text: 'abc', before: full, after: full, selection: null, maxLength: 80 })) + .toEqual({ pasted: 3, kept: 0 }); + expect( + measurePasteCut({ + text: 'admin', + before: 'admin@example.com', + after: 'admin@example.com', + selection: null, + maxLength: 17, + }), + ).toEqual({ pasted: 5, kept: 0 }); + }); + + it('does not count text already in the field as pasted', () => { + const before = `abcd${'z'.repeat(74)}`; + expect(measurePasteCut({ text: 'abcdef', before, after: `${before}ab`, selection: null, maxLength: 80 })) + .toEqual({ pasted: 6, kept: 2 }); + }); + + it('ignores the surrounding whitespace an email field trims away', () => { + const address = `${'e'.repeat(76)}.com`; + expect(measurePasteCut({ text: ` ${address}\n`, before: '', after: address, selection: null, maxLength: 80 })) + .toBeNull(); + }); + }); +}); diff --git a/src/components/ui/form/measurePasteCut.ts b/src/components/ui/form/measurePasteCut.ts new file mode 100644 index 000000000..64de1e893 --- /dev/null +++ b/src/components/ui/form/measurePasteCut.ts @@ -0,0 +1,60 @@ +export type PasteCut = { pasted: number; kept: number }; + +type PasteMeasurement = { + text: string; + before: string; + after: string; + /** `[selectionStart, selectionEnd]` at paste time, or null where the input hides it (`type="email"`). */ + selection: readonly [number, number] | null; + maxLength: number; +}; + +export function measurePasteCut({ text, before, after, selection, maxLength }: PasteMeasurement): PasteCut | null { + // What a single-line input inserts: trailing line breaks dropped, the rest turned into spaces. + const inserted = text.replace(/[\r\n]+$/, '').replace(/\r\n|\r|\n/g, ' '); + // The browser cuts only by filling the field, though it may stop one short rather than split a surrogate pair. + if (!inserted || after.length < maxLength - 1) { + return null; + } + if (selection) { + const outside = before.length - (selection[1] - selection[0]); + const room = Math.max(0, maxLength - outside); + if (room >= inserted.length) { + return null; + } + // What landed, which is short of the room when the browser stopped before a surrogate pair. + return { pasted: inserted.length, kept: Math.min(room, Math.max(0, after.length - outside)) }; + } + // An email field also trims its value. + const sought = inserted.trim(); + const kept = keptWithoutSelection(sought, before, after); + return kept < sought.length ? { pasted: sought.length, kept } : null; +} + +// Reads `after` as a start of `before`, then the start of `sought` that fit, then an end of `before`, and +// takes the longest such reading. Text already in the field therefore can't pass for pasted text, and an +// unchanged value means nothing went in, rather than an identical selection being replaced. +function keptWithoutSelection(sought: string, before: string, after: string): number { + if (after === before) { + return 0; + } + const shortest = Math.min(before.length, after.length); + let head = 0; + while (head < shortest && before[head] === after[head]) { + head++; + } + let tail = 0; + while (tail < shortest && before[before.length - 1 - tail] === after[after.length - 1 - tail]) { + tail++; + } + let kept = 0; + for (let start = 0; start <= head; start++) { + for (let end = 0; end <= tail && start + end <= shortest; end++) { + const length = after.length - start - end; + if (length > kept && length <= sought.length && after.startsWith(sought.slice(0, length), start)) { + kept = length; + } + } + } + return kept; +} diff --git a/src/components/ui/form/useFormField.tsx b/src/components/ui/form/useFormField.tsx index 9deff108f..9ee39a020 100644 --- a/src/components/ui/form/useFormField.tsx +++ b/src/components/ui/form/useFormField.tsx @@ -23,6 +23,7 @@ export function useFormField() { formItemId: `${id}-form-item`, formDescriptionId: `${id}-form-item-description`, formMessageId: `${id}-form-item-message`, + formLengthHintId: `${id}-form-item-length-hint`, error, }; } diff --git a/src/components/ui/form/useLengthLimit.tsx b/src/components/ui/form/useLengthLimit.tsx new file mode 100644 index 000000000..4d55d5232 --- /dev/null +++ b/src/components/ui/form/useLengthLimit.tsx @@ -0,0 +1,94 @@ +import { measurePasteCut, PasteCut } from '@/components/ui/form/measurePasteCut'; +import { useFormField } from '@/components/ui/form/useFormField'; +import { pluralize } from '@/lib/pluralize'; +import { TriangleAlertIcon } from 'lucide-react'; +import { ClipboardEvent, ClipboardEventHandler, useEffect, useRef, useState } from 'react'; +import { useWatch } from 'react-hook-form'; + +type PasteState = 'pending' | (PasteCut & { value: string }) | null; + +/** The browser cuts a paste to `maxLength` without a word, so this says when it has, and when the limit is reached. */ +export function useLengthLimit( + maxLength: number | null | undefined, + onPaste?: ClipboardEventHandler, +) { + const { name, formLengthHintId } = useFormField(); + const watched: unknown = useWatch({ name, disabled: maxLength == null }); + const value = typeof watched === 'string' ? watched : ''; + const [paste, setPaste] = useState(null); + const timer = useRef>(undefined); + useEffect(() => () => clearTimeout(timer.current), []); + + // zod's `.maxLength` is null on a schema without `.max()`. + if (maxLength == null) { + return { controlProps: {}, inputProps: { onPaste }, hint: null }; + } + + // Any later change, even one that stays at the limit, means the value is no longer what the paste left. + let shown = paste; + if (shown && shown !== 'pending' && value !== shown.value) { + shown = null; + setPaste(null); + } + + const handlePaste = (event: ClipboardEvent) => { + onPaste?.(event); + const input = event.currentTarget; + const text = event.clipboardData?.getData('text/plain') ?? ''; + if (event.defaultPrevented || input.readOnly || !text) { + return; + } + const before = input.value; + const { selectionStart: start, selectionEnd: end } = input; + const selection = start === null || end === null ? null : [start, end] as const; + setPaste('pending'); + clearTimeout(timer.current); + // The browser inserts the paste after this handler returns, so measure once it has. + timer.current = setTimeout(() => { + const after = input.value; + const cut = measurePasteCut({ text, before, after, selection, maxLength }); + setPaste(cut && { ...cut, value: after }); + }); + }; + + const limit = describeLimit(maxLength, value.length); + let message: string | null = null; + if (shown && shown !== 'pending') { + const fit = shown.kept ? `only the first ${shown.kept} fit` : 'none of it fit'; + message = `${limit} Pasted text was ${pluralize(shown.pasted, 'character', 'characters')}; ${fit}.`; + } else if (!shown && value.length >= maxLength) { + // Not while a paste is pending, or the limit is announced and then the paste right after it. + message = limit; + } + + return { + controlProps: { 'aria-describedby': formLengthHintId }, + inputProps: { maxLength, onPaste: handlePaste }, + hint: , + }; +} + +function describeLimit(maxLength: number, length: number) { + // A value can start over the limit, such as a name saved before the limit was lowered. + if (length > maxLength) { + return `Maximum ${maxLength} characters; this has ${length}.`; + } + // Short of it only after a paste that stopped before a surrogate pair. + return length === maxLength ? `Maximum ${maxLength} characters reached.` : `Maximum ${maxLength} characters.`; +} + +// Always rendered, so screen readers already track the live region when the message arrives. +function FormLengthHint({ id, message }: { id: string; message: string | null }) { + return ( +

+ {message &&

+ ); +} diff --git a/src/features/auth/AuthLayout.css b/src/features/auth/AuthLayout.css index 43d410b9e..64282a862 100644 --- a/src/features/auth/AuthLayout.css +++ b/src/features/auth/AuthLayout.css @@ -542,7 +542,7 @@ padding-inline: 0; } -.auth-form:not(.fabric-sign-in) p:not([role="alert"]):not([data-slot="form-message"]) { +.auth-form:not(.fabric-sign-in) p:not([role="alert"]):not([data-slot="form-message"]):not([data-slot="form-length-hint"]) { color: var(--signin-muted); line-height: 1.6; } diff --git a/src/features/auth/ResetPassword.test.tsx b/src/features/auth/ResetPassword.test.tsx index dcccf6d4f..fd526e70f 100644 --- a/src/features/auth/ResetPassword.test.tsx +++ b/src/features/auth/ResetPassword.test.tsx @@ -38,3 +38,14 @@ describe('ResetPassword', () => { expect(mutate).not.toHaveBeenCalled(); }); }); + +describe('ResetPassword password fields', () => { + it('never carry a length hint, however long the password runs', () => { + render(); + fireEvent.change(screen.getByLabelText('New Password'), { target: { value: 'p'.repeat(300) } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(300) } }); + + expect(document.querySelector('[data-slot="form-length-hint"]')).toBeNull(); + expect(screen.queryByText(/^Maximum/)).toBeNull(); + }); +}); diff --git a/src/features/auth/SignUp.test.tsx b/src/features/auth/SignUp.test.tsx index d570cd9b8..e094ef1be 100644 --- a/src/features/auth/SignUp.test.tsx +++ b/src/features/auth/SignUp.test.tsx @@ -324,6 +324,31 @@ describe('SignUp', () => { settle!(); await waitFor(() => expect(button.hasAttribute('disabled')).toBe(false)); }); + + it('says when a name or the email reaches its limit, and when a paste was cut, but never on a password', async () => { + renderSignUp(); + const descriptionOf = (label: string) => + screen.getByLabelText(label).getAttribute('aria-describedby')!.split(' ') + .map(id => document.getElementById(id)?.textContent).join(' ').trim(); + + fireEvent.change(screen.getByLabelText('First Name'), { target: { value: 'A'.repeat(40) } }); + fireEvent.change(screen.getByLabelText('Last Name'), { target: { value: 'L'.repeat(79) } }); + fireEvent.paste(screen.getByLabelText('Email'), { clipboardData: { getData: () => 'e'.repeat(312) } }); + fireEvent.change(screen.getByLabelText('Email'), { target: { value: 'e'.repeat(80) } }); + fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(300) } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(300) } }); + + expect(descriptionOf('First Name')).toBe('Maximum 40 characters reached.'); + expect(descriptionOf('Last Name')).toBe(''); + await waitFor(() => + expect(descriptionOf('Email')).toBe( + 'Maximum 80 characters reached. Pasted text was 312 characters; only the first 80 fit.', + ) + ); + expect(descriptionOf('Password')).toBe(''); + expect(descriptionOf('Confirm Password')).toBe(''); + expect(document.querySelectorAll('[data-slot="form-length-hint"]')).toHaveLength(3); + }); }); describe('SignUp β€” reCAPTCHA', () => { diff --git a/src/features/auth/components/AuthInput.tsx b/src/features/auth/components/AuthInput.tsx index e0e6f00ab..869fb898e 100644 --- a/src/features/auth/components/AuthInput.tsx +++ b/src/features/auth/components/AuthInput.tsx @@ -1,4 +1,5 @@ import { FormControl } from '@/components/ui/form/FormControl'; +import { useLengthLimit } from '@/components/ui/form/useLengthLimit'; import { Input } from '@/components/ui/input'; import { Eye, EyeOff, LockKeyhole, Mail, UserRound } from 'lucide-react'; import { ComponentProps, useState } from 'react'; @@ -8,26 +9,30 @@ type AuthInputProps = Omit, 'type'> & { passwordLabel?: string; }; -export function AuthInput({ type = 'text', passwordLabel = 'password', ...props }: AuthInputProps) { +export function AuthInput({ type = 'text', passwordLabel = 'password', maxLength, onPaste, ...props }: AuthInputProps) { const [visible, setVisible] = useState(false); + const limit = useLengthLimit(maxLength, onPaste); const Icon = type === 'email' ? Mail : type === 'password' ? LockKeyhole : UserRound; return ( -
- - - -
+ <> +
+ + + +
+ {limit.hint} + ); } diff --git a/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx b/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx index 6002d8593..54dca3cc7 100644 --- a/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx +++ b/src/features/instance/applications/components/NewApplication/CommonInstructions.tsx @@ -1,10 +1,9 @@ import { CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { Control } from 'react-hook-form'; import { z } from 'zod'; import { NewApplicationSchema } from './schema'; @@ -32,17 +31,15 @@ export function CommonInstructions({ render={({ field }) => ( Name - - - + )} diff --git a/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx b/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx index d3279dc11..d5a9c7d74 100644 --- a/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx +++ b/src/features/instance/applications/components/NewApplication/NewApplication.test.tsx @@ -98,10 +98,13 @@ describe('NewApplication', () => { expect(screen.queryByText('Please enter a URL or package reference.')).toBeNull(); }); - it('stops the application name at the 75 characters the form accepts', () => { + it('stops the application name at the 75 characters the form accepts, and says so there', () => { render(); + const name = screen.getByLabelText('Name') as HTMLInputElement; + expect(name.maxLength).toBe(75); - expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(75); + fireEvent.change(name, { target: { value: 'a'.repeat(75) } }); + expect(screen.getByText('Maximum 75 characters reached.')).toBeTruthy(); }); it('explains a refused application name when Enter is pressed in it', async () => { diff --git a/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx b/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx index b7cc9c2da..3fc768ff7 100644 --- a/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx +++ b/src/features/instance/applications/modals/AddDirectoryOrFileModal.test.tsx @@ -2,7 +2,7 @@ * @vitest-environment jsdom */ import type { FileEntry } from '@/features/instance/applications/context/fileEntry'; -import { cleanup, render, screen } from '@testing-library/react'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; const openedEntry: FileEntry = { name: 'index.js', path: 'my-app/index.js', project: 'my-app' } as FileEntry; @@ -42,9 +42,12 @@ beforeAll(() => { afterEach(cleanup); describe('AddDirectoryOrFileModal', () => { - it('stops the name at the 50 characters the form accepts', () => { + it('stops the name at the 50 characters the form accepts, and says so there', () => { render(); + const name = screen.getByLabelText('Name') as HTMLInputElement; + expect(name.maxLength).toBe(50); - expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(50); + fireEvent.change(name, { target: { value: 'a'.repeat(50) } }); + expect(screen.getByText('Maximum 50 characters reached.')).toBeTruthy(); }); }); diff --git a/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx b/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx index 051bcf713..9b6692627 100644 --- a/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx +++ b/src/features/instance/applications/modals/AddDirectoryOrFileModal.tsx @@ -8,12 +8,11 @@ import { DialogTitle, } from '@/components/ui/dialog'; import { Form } from '@/components/ui/form/Form'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { isDirectory } from '@/features/instance/applications/context/isDirectory'; import { useEditorView } from '@/features/instance/applications/hooks/useEditorView'; @@ -165,16 +164,14 @@ export function AddDirectoryOrFileModal() { render={({ field }) => ( Name - - - + )} diff --git a/src/features/instance/applications/modals/RenameFileModal.test.tsx b/src/features/instance/applications/modals/RenameFileModal.test.tsx index c09c2416a..3113fe4b9 100644 --- a/src/features/instance/applications/modals/RenameFileModal.test.tsx +++ b/src/features/instance/applications/modals/RenameFileModal.test.tsx @@ -52,10 +52,13 @@ function renameButton() { } describe('RenameFileModal', () => { - it('stops the name at the 50 characters the form accepts', () => { + it('stops the name at the 50 characters the form accepts, and says so there', () => { render(); + const name = screen.getByLabelText('Name') as HTMLInputElement; + expect(name.maxLength).toBe(50); - expect((screen.getByLabelText('Name') as HTMLInputElement).maxLength).toBe(50); + fireEvent.change(name, { target: { value: 'a'.repeat(50) } }); + expect(screen.getByText('Maximum 50 characters reached.')).toBeTruthy(); }); it('shows the reason when Enter is pressed, then clears it once the name is fixed', async () => { diff --git a/src/features/instance/applications/modals/RenameFileModal.tsx b/src/features/instance/applications/modals/RenameFileModal.tsx index 4cd3d3b3c..2170399df 100644 --- a/src/features/instance/applications/modals/RenameFileModal.tsx +++ b/src/features/instance/applications/modals/RenameFileModal.tsx @@ -8,13 +8,12 @@ import { DialogTitle, } from '@/components/ui/dialog'; import { Form } from '@/components/ui/form/Form'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { revealFieldErrorOnEnter } from '@/components/ui/form/revealFieldErrorOnEnter'; -import { Input } from '@/components/ui/input'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import type { DirectoryEntry } from '@/features/instance/applications/context/directoryEntry'; import type { FileEntry } from '@/features/instance/applications/context/fileEntry'; @@ -180,16 +179,14 @@ export function RenameFileModal() { render={({ field }) => ( Name - - - + )} diff --git a/src/features/instance/config/roles/modals/AddRoleModal.test.tsx b/src/features/instance/config/roles/modals/AddRoleModal.test.tsx index b4d5fe2e7..7f933d920 100644 --- a/src/features/instance/config/roles/modals/AddRoleModal.test.tsx +++ b/src/features/instance/config/roles/modals/AddRoleModal.test.tsx @@ -100,9 +100,12 @@ describe('AddRoleModal', () => { expect(screen.queryByRole('radio', { name: /Cluster user/ })).toBeNull(); }); - it('stops the role name at the 30 characters the form accepts', () => { + it('stops the role name at the 30 characters the form accepts, and says so there', () => { renderModal(); + const name = screen.getByRole('textbox', { name: 'Name' }) as HTMLInputElement; + expect(name.maxLength).toBe(30); - expect((screen.getByRole('textbox', { name: 'Name' }) as HTMLInputElement).maxLength).toBe(30); + fireEvent.change(name, { target: { value: 'a'.repeat(30) } }); + expect(screen.getByText('Maximum 30 characters reached.')).toBeTruthy(); }); }); diff --git a/src/features/instance/config/roles/modals/AddRoleModal.tsx b/src/features/instance/config/roles/modals/AddRoleModal.tsx index 6652c0443..a33bf34bb 100644 --- a/src/features/instance/config/roles/modals/AddRoleModal.tsx +++ b/src/features/instance/config/roles/modals/AddRoleModal.tsx @@ -8,12 +8,11 @@ import { DialogTitle, } from '@/components/ui/dialog'; import { Form } from '@/components/ui/form/Form'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { isOperationsValueUnusable, @@ -99,9 +98,7 @@ export function AddRoleModal({ render={({ field }) => ( Name - - - + )} diff --git a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx index b99f2aa01..6123f3b5d 100644 --- a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx @@ -3,7 +3,7 @@ */ import { ImportDataModal } from '@/features/instance/databases/modals/ImportDataModal'; import { LocalRolePermission } from '@/integrations/api/api.patch'; -import { cleanup, render, screen } from '@testing-library/react'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; // Only the auth store and the router are stubbed, so these run the real @@ -131,10 +131,16 @@ describe('ImportDataModal requested method', () => { }); describe('ImportDataModal name fields', () => { - it('stops the database and table names at the lengths the form accepts', () => { + it('stops the database and table names at the lengths the form accepts, and says so there', () => { renderModal({ super_user: true }); - - expect((screen.getByLabelText('Database') as HTMLInputElement).maxLength).toBe(75); - expect((screen.getByLabelText('Table') as HTMLInputElement).maxLength).toBe(250); + const database = screen.getByLabelText('Database') as HTMLInputElement; + const table = screen.getByLabelText('Table') as HTMLInputElement; + expect(database.maxLength).toBe(75); + expect(table.maxLength).toBe(250); + + fireEvent.change(database, { target: { value: 'd'.repeat(75) } }); + fireEvent.change(table, { target: { value: 't'.repeat(250) } }); + expect(screen.getByText('Maximum 75 characters reached.')).toBeTruthy(); + expect(screen.getByText('Maximum 250 characters reached.')).toBeTruthy(); }); }); diff --git a/src/features/instance/databases/modals/ImportDataModal.tsx b/src/features/instance/databases/modals/ImportDataModal.tsx index 9bb95ef3a..6a55d6699 100644 --- a/src/features/instance/databases/modals/ImportDataModal.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.tsx @@ -14,6 +14,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { Input } from '@/components/ui/input'; import { RadioGroup, RadioGroupItem } from '@/components/ui/radioGroup'; import { @@ -525,17 +526,15 @@ export function ImportDataModal({ render={({ field }) => ( Database - - - + )} @@ -546,16 +545,14 @@ export function ImportDataModal({ render={({ field }) => ( Table - - - + )} diff --git a/src/features/organization/roles/modals/AddOrganizationRoleModal.test.tsx b/src/features/organization/roles/modals/AddOrganizationRoleModal.test.tsx index 398e3b8a2..1c652ac50 100644 --- a/src/features/organization/roles/modals/AddOrganizationRoleModal.test.tsx +++ b/src/features/organization/roles/modals/AddOrganizationRoleModal.test.tsx @@ -52,10 +52,13 @@ describe('AddOrganizationRoleModal', () => { expect(await screen.findByText('Role must contain only letters and underscores.')).toBeTruthy(); }); - it('stops the role name at the 30 characters the form accepts', () => { + it('stops the role name at the 30 characters the form accepts, and says so there', () => { render( {}} />); + const name = screen.getByLabelText('Role Name') as HTMLInputElement; + expect(name.maxLength).toBe(30); - expect((screen.getByLabelText('Role Name') as HTMLInputElement).maxLength).toBe(30); + fireEvent.change(name, { target: { value: 'a'.repeat(30) } }); + expect(screen.getByText('Maximum 30 characters reached.')).toBeTruthy(); }); it.each([ diff --git a/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx b/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx index a89616310..458c8baec 100644 --- a/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx +++ b/src/features/organization/roles/modals/AddOrganizationRoleModal.tsx @@ -6,6 +6,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { revealFieldErrorOnEnter } from '@/components/ui/form/revealFieldErrorOnEnter'; import { Input } from '@/components/ui/input'; import { useAddOrganizationRole } from '@/features/organization/mutations/addOrganizationRole'; @@ -124,14 +125,12 @@ export function AddOrganizationRoleModal({ render={({ field }) => ( Role Name - - - + )} diff --git a/src/features/organization/users/modals/AddUserModal.email.test.tsx b/src/features/organization/users/modals/AddUserModal.email.test.tsx index 9096f6c53..f6031f023 100644 --- a/src/features/organization/users/modals/AddUserModal.email.test.tsx +++ b/src/features/organization/users/modals/AddUserModal.email.test.tsx @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { cleanup, render, screen } from '@testing-library/react'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } })); @@ -31,9 +31,12 @@ beforeAll(() => { afterEach(cleanup); describe('AddUserModal', () => { - it('stops the email at the 80 characters the form accepts', () => { + it('stops the email at the 80 characters the form accepts, and says so there', () => { render( {}} setIsModalOpen={() => {}} />); + const email = screen.getByLabelText('Email') as HTMLInputElement; + expect(email.maxLength).toBe(80); - expect((screen.getByLabelText('Email') as HTMLInputElement).maxLength).toBe(80); + fireEvent.change(email, { target: { value: 'a'.repeat(80) } }); + expect(screen.getByText('Maximum 80 characters reached.')).toBeTruthy(); }); }); diff --git a/src/features/organization/users/modals/AddUserModal.tsx b/src/features/organization/users/modals/AddUserModal.tsx index 764cfe5cb..c29da2c53 100644 --- a/src/features/organization/users/modals/AddUserModal.tsx +++ b/src/features/organization/users/modals/AddUserModal.tsx @@ -15,7 +15,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { Select, SelectContent, @@ -119,15 +119,13 @@ export function AddUserModal({ render={({ field }) => ( Email - - - + )} diff --git a/src/features/profile/ProfileIndex.test.tsx b/src/features/profile/ProfileIndex.test.tsx index f92a0b2be..566178ef5 100644 --- a/src/features/profile/ProfileIndex.test.tsx +++ b/src/features/profile/ProfileIndex.test.tsx @@ -128,4 +128,25 @@ describe('ProfileIndex', () => { await waitFor(() => expect(screen.queryByText('Passwords do not match')).toBeNull()); expect(saveButton().disabled).toBe(false); }); + + it('says when a name reaches its limit, but never on a password', async () => { + render(); + const descriptionOf = (label: string) => + screen.getByLabelText(label).getAttribute('aria-describedby')!.split(' ') + .map(id => document.getElementById(id)?.textContent).join(' ').trim(); + + await type('First Name', 'A'.repeat(40)); + await type('Last Name', 'L'.repeat(80)); + await type('New Password', 'p'.repeat(300)); + await type('Confirm New Password', 'p'.repeat(300)); + + expect(descriptionOf('First Name')).toBe('Maximum 40 characters reached.'); + expect(descriptionOf('Last Name')).toBe('Maximum 80 characters reached.'); + expect(descriptionOf('New Password')).not.toContain('Maximum'); + expect(descriptionOf('Confirm New Password')).not.toContain('Maximum'); + expect(document.querySelectorAll('[data-slot="form-length-hint"]')).toHaveLength(2); + + await type('Last Name', 'L'.repeat(79)); + expect(descriptionOf('Last Name')).toBe(''); + }); }); diff --git a/src/features/profile/index.tsx b/src/features/profile/index.tsx index 30598b02d..33d6ed483 100644 --- a/src/features/profile/index.tsx +++ b/src/features/profile/index.tsx @@ -7,6 +7,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { revealFieldErrorOnEnter } from '@/components/ui/form/revealFieldErrorOnEnter'; import { Input } from '@/components/ui/input'; import { logoutOnSuccess } from '@/features/auth/handlers/logoutOnSuccess'; @@ -105,15 +106,13 @@ export function ProfileIndex() { render={({ field }) => ( First Name - - - + )} @@ -125,15 +124,13 @@ export function ProfileIndex() { render={({ field }) => ( Last Name - - - + )} From c460f66a7de7bbb81cc68e3c3f34cb76eb27339d Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 12 Oct 2026 18:57:49 -0400 Subject: [PATCH 3/4] fix(forms): leave email fields uncapped, and say when the older capped fields hit their limit A cut email address can still be a valid one: an 81-character ...@example.com pasted into the organization invite or sign-up became ...@example.co, passed z.email(), and the invite would have gone to that mailbox. So the two email fields now work like the password fields: no maxLength and no length hint, and the schema's .max(80) refuses an over-long address on submit ("Email cannot be longer than 80 characters."). That leaves no capped field with type="email", the one input type that hides its selection, so measurePasteCut keeps only the selection-known path. The hook makes no paste claim when the selection is hidden. NewOrgForm (name, subdomain), ClusterName, ClusterAbbreviatedName and CreateNewTableModal (table name, primary key, database name) already had a maxLength that cut silently. They now go through LengthLimitedInput, so they say when the limit is reached and when a paste was cut, like the rest. Co-Authored-By: Claude Opus 5.5 --- .../ui/form/LengthLimitedInput.test.tsx | 30 +++-- .../ui/form/measurePasteCut.test.ts | 119 ++++++------------ src/components/ui/form/measurePasteCut.ts | 49 ++------ src/components/ui/form/useLengthLimit.tsx | 10 +- src/features/auth/SignUp.test.tsx | 35 ++++-- src/features/auth/SignUp.tsx | 1 - .../upsert/fields/ClusterAbbreviatedName.tsx | 25 ++-- .../clusters/upsert/fields/ClusterName.tsx | 21 ++-- .../upsert/fields/ClusterNameFields.test.tsx | 50 ++++++++ .../modals/CreateNewTableModal.test.tsx | 48 +++++++ .../databases/modals/CreateNewTableModal.tsx | 63 +++++----- .../users/modals/AddUserModal.email.test.tsx | 25 ++-- .../users/modals/AddUserModal.tsx | 12 +- .../components/NewOrgForm.test.tsx | 40 ++++++ .../organizations/components/NewOrgForm.tsx | 39 +++--- 15 files changed, 320 insertions(+), 247 deletions(-) create mode 100644 src/features/clusters/upsert/fields/ClusterNameFields.test.tsx create mode 100644 src/features/instance/databases/modals/CreateNewTableModal.test.tsx create mode 100644 src/features/organizations/components/NewOrgForm.test.tsx diff --git a/src/components/ui/form/LengthLimitedInput.test.tsx b/src/components/ui/form/LengthLimitedInput.test.tsx index 6c6786338..4e9736bfd 100644 --- a/src/components/ui/form/LengthLimitedInput.test.tsx +++ b/src/components/ui/form/LengthLimitedInput.test.tsx @@ -6,16 +6,26 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { useEffect } from 'react'; import { useForm } from 'react-hook-form'; import { afterEach, describe, expect, it } from 'vitest'; import { z } from 'zod'; import { LengthLimitedInput } from './LengthLimitedInput'; -function NameForm( - { type = 'text', name = '', maxLength = 10 }: { type?: 'text' | 'email'; name?: string; maxLength?: number }, -) { +function NameForm({ type = 'text', name = '', maxLength = 10, focus = false }: { + type?: 'text' | 'email'; + name?: string; + maxLength?: number; + focus?: boolean; +}) { const form = useForm({ defaultValues: { name } }); + const { setFocus } = form; + useEffect(() => { + if (focus) { + setFocus('name'); + } + }, [focus, setFocus]); return (
{ expect(hint().getAttribute('aria-live')).toBe('polite'); }); + it("passes react-hook-form the input's ref, so a refused field can still be focused", async () => { + render(); + + await waitFor(() => expect(document.activeElement).toBe(input())); + }); + it('says nothing below the limit, from a live region that is already in place', () => { render(); type('a'.repeat(9)); @@ -150,14 +166,12 @@ describe('LengthLimitedInput', () => { expect(document.querySelector('[data-slot="form-length-hint"]')).toBeNull(); }); - it('says when a paste into an email field was cut, though that field hides its selection', async () => { + it('makes no claim about a paste into a field that hides its selection, as type="email" does', async () => { render(); expect(input().selectionStart).toBeNull(); type('a@b.co'); await paste(`bob@${'e'.repeat(40)}.com`, `bob@${'e'.repeat(6)}`); - expect(hint().textContent).toBe( - 'Maximum 10 characters reached. Pasted text was 48 characters; only the first 10 fit.', - ); + expect(hint().textContent).toBe('Maximum 10 characters reached.'); }); }); diff --git a/src/components/ui/form/measurePasteCut.test.ts b/src/components/ui/form/measurePasteCut.test.ts index b542f0f5d..737de3098 100644 --- a/src/components/ui/form/measurePasteCut.test.ts +++ b/src/components/ui/form/measurePasteCut.test.ts @@ -4,97 +4,48 @@ import { measurePasteCut } from './measurePasteCut'; const long = 'x'.repeat(312); describe('measurePasteCut', () => { - describe('with the selection known', () => { - it('reports what an empty field kept of a longer paste', () => { - expect(measurePasteCut({ text: long, before: '', after: long.slice(0, 80), selection: [0, 0], maxLength: 80 })) - .toEqual({ pasted: 312, kept: 80 }); - }); - - it('counts only the room left beside what the field already held', () => { - const before = 'a'.repeat(10); - const after = before + 'b'.repeat(70); - expect(measurePasteCut({ text: 'b'.repeat(75), before, after, selection: [10, 10], maxLength: 80 })) - .toEqual({ pasted: 75, kept: 70 }); - }); - - it('counts the selection the paste replaces as room', () => { - const before = 'a'.repeat(50); - const text = 'b'.repeat(80); - expect(measurePasteCut({ text, before, after: text, selection: [0, 50], maxLength: 80 })).toBeNull(); - }); - - it('says none of it fit when the field was already full', () => { - const full = 'a'.repeat(80); - expect(measurePasteCut({ text: 'hello', before: full, after: full, selection: [80, 80], maxLength: 80 })) - .toEqual({ pasted: 5, kept: 0 }); - }); - - it('counts what landed when the browser stopped short of a surrogate pair', () => { - const before = 'a'.repeat(79); - expect(measurePasteCut({ text: 'πŸ˜€', before, after: before, selection: [79, 79], maxLength: 80 })) - .toEqual({ pasted: 2, kept: 0 }); - }); - - it('ignores a paste that fit exactly', () => { - const text = 'b'.repeat(80); - expect(measurePasteCut({ text, before: '', after: text, selection: [0, 0], maxLength: 80 })).toBeNull(); - }); - - it('ignores the trailing line break a copied line carries, and counts each inner break as one space', () => { - const text = `${'b'.repeat(39)}\r\n${'c'.repeat(40)}\n`; - const after = `${'b'.repeat(39)} ${'c'.repeat(40)}`; - expect(measurePasteCut({ text, before: '', after, selection: [0, 0], maxLength: 80 })).toBeNull(); - }); + it('reports what an empty field kept of a longer paste', () => { + expect(measurePasteCut({ text: long, before: '', after: long.slice(0, 80), selection: [0, 0], maxLength: 80 })) + .toEqual({ pasted: 312, kept: 80 }); + }); - it('ignores a paste that left the field short of its limit, which something other than the limit refused', () => { - expect(measurePasteCut({ text: long, before: 'abc', after: 'abc', selection: [3, 3], maxLength: 80 })).toBeNull(); - }); + it('counts only the room left beside what the field already held', () => { + const before = 'a'.repeat(10); + const after = before + 'b'.repeat(70); + expect(measurePasteCut({ text: 'b'.repeat(75), before, after, selection: [10, 10], maxLength: 80 })) + .toEqual({ pasted: 75, kept: 70 }); }); - describe('with the selection hidden, as type="email" keeps it', () => { - it('reports what an empty field kept', () => { - expect(measurePasteCut({ text: long, before: '', after: long.slice(0, 80), selection: null, maxLength: 80 })) - .toEqual({ pasted: 312, kept: 80 }); - }); + it('counts the selection the paste replaces as room', () => { + const before = 'a'.repeat(50); + const text = 'b'.repeat(80); + expect(measurePasteCut({ text, before, after: text, selection: [0, 50], maxLength: 80 })).toBeNull(); + }); - it('reports the whole limit kept when the paste replaced an address sharing its start', () => { - const text = `alice@${'e'.repeat(100)}.com`; - expect( - measurePasteCut({ text, before: 'alice@x.com', after: text.slice(0, 80), selection: null, maxLength: 80 }), - ).toEqual({ pasted: text.length, kept: 80 }); - }); + it('says none of it fit when the field was already full', () => { + const full = 'a'.repeat(80); + expect(measurePasteCut({ text: 'hello', before: full, after: full, selection: [80, 80], maxLength: 80 })) + .toEqual({ pasted: 5, kept: 0 }); + }); - it('ignores a paste that replaced the value and fit', () => { - const text = `bob@${'e'.repeat(70)}.com`; - expect(measurePasteCut({ text, before: 'alice@x.com', after: text, selection: null, maxLength: 78 })) - .toBeNull(); - }); + it('counts what landed when the browser stopped short of a surrogate pair', () => { + const before = 'a'.repeat(79); + expect(measurePasteCut({ text: 'πŸ˜€', before, after: before, selection: [79, 79], maxLength: 80 })) + .toEqual({ pasted: 2, kept: 0 }); + }); - it('says none of it fit when the full value did not change, though the paste matches part of it', () => { - const full = `abc${'x'.repeat(77)}`; - expect(measurePasteCut({ text: 'abc', before: full, after: full, selection: null, maxLength: 80 })) - .toEqual({ pasted: 3, kept: 0 }); - expect( - measurePasteCut({ - text: 'admin', - before: 'admin@example.com', - after: 'admin@example.com', - selection: null, - maxLength: 17, - }), - ).toEqual({ pasted: 5, kept: 0 }); - }); + it('ignores a paste that fit exactly', () => { + const text = 'b'.repeat(80); + expect(measurePasteCut({ text, before: '', after: text, selection: [0, 0], maxLength: 80 })).toBeNull(); + }); - it('does not count text already in the field as pasted', () => { - const before = `abcd${'z'.repeat(74)}`; - expect(measurePasteCut({ text: 'abcdef', before, after: `${before}ab`, selection: null, maxLength: 80 })) - .toEqual({ pasted: 6, kept: 2 }); - }); + it('ignores the trailing line break a copied line carries, and counts each inner break as one space', () => { + const text = `${'b'.repeat(39)}\r\n${'c'.repeat(40)}\n`; + const after = `${'b'.repeat(39)} ${'c'.repeat(40)}`; + expect(measurePasteCut({ text, before: '', after, selection: [0, 0], maxLength: 80 })).toBeNull(); + }); - it('ignores the surrounding whitespace an email field trims away', () => { - const address = `${'e'.repeat(76)}.com`; - expect(measurePasteCut({ text: ` ${address}\n`, before: '', after: address, selection: null, maxLength: 80 })) - .toBeNull(); - }); + it('ignores a paste that left the field short of its limit, which something other than the limit refused', () => { + expect(measurePasteCut({ text: long, before: 'abc', after: 'abc', selection: [3, 3], maxLength: 80 })).toBeNull(); }); }); diff --git a/src/components/ui/form/measurePasteCut.ts b/src/components/ui/form/measurePasteCut.ts index 64de1e893..dcb254025 100644 --- a/src/components/ui/form/measurePasteCut.ts +++ b/src/components/ui/form/measurePasteCut.ts @@ -4,8 +4,7 @@ type PasteMeasurement = { text: string; before: string; after: string; - /** `[selectionStart, selectionEnd]` at paste time, or null where the input hides it (`type="email"`). */ - selection: readonly [number, number] | null; + selection: readonly [number, number]; maxLength: number; }; @@ -16,45 +15,11 @@ export function measurePasteCut({ text, before, after, selection, maxLength }: P if (!inserted || after.length < maxLength - 1) { return null; } - if (selection) { - const outside = before.length - (selection[1] - selection[0]); - const room = Math.max(0, maxLength - outside); - if (room >= inserted.length) { - return null; - } - // What landed, which is short of the room when the browser stopped before a surrogate pair. - return { pasted: inserted.length, kept: Math.min(room, Math.max(0, after.length - outside)) }; - } - // An email field also trims its value. - const sought = inserted.trim(); - const kept = keptWithoutSelection(sought, before, after); - return kept < sought.length ? { pasted: sought.length, kept } : null; -} - -// Reads `after` as a start of `before`, then the start of `sought` that fit, then an end of `before`, and -// takes the longest such reading. Text already in the field therefore can't pass for pasted text, and an -// unchanged value means nothing went in, rather than an identical selection being replaced. -function keptWithoutSelection(sought: string, before: string, after: string): number { - if (after === before) { - return 0; - } - const shortest = Math.min(before.length, after.length); - let head = 0; - while (head < shortest && before[head] === after[head]) { - head++; - } - let tail = 0; - while (tail < shortest && before[before.length - 1 - tail] === after[after.length - 1 - tail]) { - tail++; - } - let kept = 0; - for (let start = 0; start <= head; start++) { - for (let end = 0; end <= tail && start + end <= shortest; end++) { - const length = after.length - start - end; - if (length > kept && length <= sought.length && after.startsWith(sought.slice(0, length), start)) { - kept = length; - } - } + const outside = before.length - (selection[1] - selection[0]); + const room = Math.max(0, maxLength - outside); + if (room >= inserted.length) { + return null; } - return kept; + // What landed, which is short of the room when the browser stopped before a surrogate pair. + return { pasted: inserted.length, kept: Math.min(room, Math.max(0, after.length - outside)) }; } diff --git a/src/components/ui/form/useLengthLimit.tsx b/src/components/ui/form/useLengthLimit.tsx index 4d55d5232..8d4884431 100644 --- a/src/components/ui/form/useLengthLimit.tsx +++ b/src/components/ui/form/useLengthLimit.tsx @@ -7,7 +7,7 @@ import { useWatch } from 'react-hook-form'; type PasteState = 'pending' | (PasteCut & { value: string }) | null; -/** The browser cuts a paste to `maxLength` without a word, so this says when it has, and when the limit is reached. */ +/** The browser cuts a paste to `maxLength` without a word. */ export function useLengthLimit( maxLength: number | null | undefined, onPaste?: ClipboardEventHandler, @@ -35,12 +35,14 @@ export function useLengthLimit( onPaste?.(event); const input = event.currentTarget; const text = event.clipboardData?.getData('text/plain') ?? ''; - if (event.defaultPrevented || input.readOnly || !text) { + const { selectionStart: start, selectionEnd: end } = input; + // `type="email"` hides its selection, so there is no cut to measure. Email fields are left uncapped + // anyway, since a cut address can still be a valid one, just not the user's. + if (event.defaultPrevented || input.readOnly || !text || start === null || end === null) { return; } const before = input.value; - const { selectionStart: start, selectionEnd: end } = input; - const selection = start === null || end === null ? null : [start, end] as const; + const selection = [start, end] as const; setPaste('pending'); clearTimeout(timer.current); // The browser inserts the paste after this handler returns, so measure once it has. diff --git a/src/features/auth/SignUp.test.tsx b/src/features/auth/SignUp.test.tsx index e094ef1be..db938fdf7 100644 --- a/src/features/auth/SignUp.test.tsx +++ b/src/features/auth/SignUp.test.tsx @@ -98,13 +98,13 @@ beforeEach(() => { afterEach(() => vi.clearAllMocks()); describe('SignUp', () => { - it('stops names and email at their limits but lets a password run long', () => { + it('stops names at their limits but lets an email or a password run long', () => { renderSignUp(); const maxLengthOf = (label: string) => (screen.getByLabelText(label) as HTMLInputElement).maxLength; expect(maxLengthOf('First Name')).toBe(40); expect(maxLengthOf('Last Name')).toBe(80); - expect(maxLengthOf('Email')).toBe(80); + expect(maxLengthOf('Email')).toBe(-1); expect(maxLengthOf('Password')).toBe(-1); expect(maxLengthOf('Confirm Password')).toBe(-1); }); @@ -325,29 +325,44 @@ describe('SignUp', () => { await waitFor(() => expect(button.hasAttribute('disabled')).toBe(false)); }); - it('says when a name or the email reaches its limit, and when a paste was cut, but never on a password', async () => { + it('says when a name reaches its limit, and when a paste was cut, but never on the email or a password', async () => { renderSignUp(); const descriptionOf = (label: string) => screen.getByLabelText(label).getAttribute('aria-describedby')!.split(' ') .map(id => document.getElementById(id)?.textContent).join(' ').trim(); - fireEvent.change(screen.getByLabelText('First Name'), { target: { value: 'A'.repeat(40) } }); - fireEvent.change(screen.getByLabelText('Last Name'), { target: { value: 'L'.repeat(79) } }); - fireEvent.paste(screen.getByLabelText('Email'), { clipboardData: { getData: () => 'e'.repeat(312) } }); - fireEvent.change(screen.getByLabelText('Email'), { target: { value: 'e'.repeat(80) } }); + fireEvent.change(screen.getByLabelText('First Name'), { target: { value: 'A'.repeat(39) } }); + fireEvent.paste(screen.getByLabelText('Last Name'), { clipboardData: { getData: () => 'l'.repeat(312) } }); + fireEvent.change(screen.getByLabelText('Last Name'), { target: { value: 'l'.repeat(80) } }); + fireEvent.change(screen.getByLabelText('Email'), { target: { value: `${'e'.repeat(300)}@example.com` } }); fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(300) } }); fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(300) } }); + expect(descriptionOf('First Name')).toBe(''); + fireEvent.change(screen.getByLabelText('First Name'), { target: { value: 'A'.repeat(40) } }); expect(descriptionOf('First Name')).toBe('Maximum 40 characters reached.'); - expect(descriptionOf('Last Name')).toBe(''); await waitFor(() => - expect(descriptionOf('Email')).toBe( + expect(descriptionOf('Last Name')).toBe( 'Maximum 80 characters reached. Pasted text was 312 characters; only the first 80 fit.', ) ); + expect(descriptionOf('Email')).toBe(''); expect(descriptionOf('Password')).toBe(''); expect(descriptionOf('Confirm Password')).toBe(''); - expect(document.querySelectorAll('[data-slot="form-length-hint"]')).toHaveLength(3); + expect(document.querySelectorAll('[data-slot="form-length-hint"]')).toHaveLength(2); + }); + + it('lets an over-long email be typed in full and refuses it before sending, as it does a password', async () => { + renderSignUp(); + fillValidForm(); + const email = screen.getByLabelText('Email') as HTMLInputElement; + const address = `${'u'.repeat(69)}@example.com`; + fireEvent.change(email, { target: { value: address } }); + submit(); + + expect(email.value).toHaveLength(81); + expect(await screen.findByText('Email cannot be longer than 80 characters.')).toBeTruthy(); + expect(post).not.toHaveBeenCalled(); }); }); diff --git a/src/features/auth/SignUp.tsx b/src/features/auth/SignUp.tsx index 7b22b2692..e2edaf1a0 100644 --- a/src/features/auth/SignUp.tsx +++ b/src/features/auth/SignUp.tsx @@ -262,7 +262,6 @@ export function SignUp() { disabled={!!searchEmail} autoComplete="email" autoCapitalize="none" - maxLength={SignUpSchema.shape.email.maxLength!} {...field} /> diff --git a/src/features/clusters/upsert/fields/ClusterAbbreviatedName.tsx b/src/features/clusters/upsert/fields/ClusterAbbreviatedName.tsx index c0e37a52b..e5d6f334b 100644 --- a/src/features/clusters/upsert/fields/ClusterAbbreviatedName.tsx +++ b/src/features/clusters/upsert/fields/ClusterAbbreviatedName.tsx @@ -1,10 +1,9 @@ import { HostnamePreview } from '@/components/HostnamePreview'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { specifiedAbbreviatedName, UpsertClusterSchema } from '@/features/clusters/upsert/upsertClusterSchema'; import { useId } from 'react'; import { UseFormReturn } from 'react-hook-form'; @@ -28,18 +27,16 @@ export function ClusterAbbreviatedName({ render={({ field }) => ( Host Name - - - + )} diff --git a/src/features/clusters/upsert/fields/ClusterName.tsx b/src/features/clusters/upsert/fields/ClusterName.tsx index 51ff132db..1eae7785e 100644 --- a/src/features/clusters/upsert/fields/ClusterName.tsx +++ b/src/features/clusters/upsert/fields/ClusterName.tsx @@ -1,9 +1,8 @@ -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { UpsertClusterSchema } from '@/features/clusters/upsert/upsertClusterSchema'; import { UseFormReturn } from 'react-hook-form'; import { z } from 'zod'; @@ -24,16 +23,14 @@ export function ClusterName({ render={({ field }) => ( Cluster Name - - - + )} diff --git a/src/features/clusters/upsert/fields/ClusterNameFields.test.tsx b/src/features/clusters/upsert/fields/ClusterNameFields.test.tsx new file mode 100644 index 000000000..7e1c69022 --- /dev/null +++ b/src/features/clusters/upsert/fields/ClusterNameFields.test.tsx @@ -0,0 +1,50 @@ +/** + * @vitest-environment jsdom + */ +import { Form } from '@/components/ui/form/Form'; +import { UpsertClusterSchema } from '@/features/clusters/upsert/upsertClusterSchema'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { useForm } from 'react-hook-form'; +import { afterEach, describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { ClusterAbbreviatedName } from './ClusterAbbreviatedName'; +import { ClusterName } from './ClusterName'; + +function ClusterNameFields() { + const form = useForm>({ + defaultValues: { clusterName: '', abbreviatedName: '' }, + }); + return ( + + + + + ); +} + +afterEach(cleanup); + +describe('ClusterName', () => { + it('stops the cluster name at the 255 characters the form accepts, and says so there', () => { + render(); + const name = screen.getByLabelText('Cluster Name') as HTMLInputElement; + expect(name.maxLength).toBe(255); + + fireEvent.change(name, { target: { value: 'c'.repeat(255) } }); + expect(screen.getByText('Maximum 255 characters reached.')).toBeTruthy(); + }); +}); + +describe('ClusterAbbreviatedName', () => { + it('stops the host name at the 20 characters the form accepts, and says so there', () => { + render(); + const hostName = screen.getByLabelText('Host Name') as HTMLInputElement; + expect(hostName.maxLength).toBe(20); + + fireEvent.change(hostName, { target: { value: 'h'.repeat(20) } }); + expect(screen.getByText('Maximum 20 characters reached.')).toBeTruthy(); + }); +}); diff --git a/src/features/instance/databases/modals/CreateNewTableModal.test.tsx b/src/features/instance/databases/modals/CreateNewTableModal.test.tsx new file mode 100644 index 000000000..0e1218fa7 --- /dev/null +++ b/src/features/instance/databases/modals/CreateNewTableModal.test.tsx @@ -0,0 +1,48 @@ +/** + * @vitest-environment jsdom + */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +vi.mock('@tanstack/react-router', () => ({ useRouter: () => ({ invalidate: vi.fn() }) })); +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => ({ entityId: 'test-instance', entityType: 'instance' }), +})); +vi.mock('@/integrations/api/instance/database/createTable', () => ({ + useCreateTableMutation: () => ({ mutate: vi.fn() }), +})); + +import { CreateNewTableModal } from './CreateNewTableModal'; + +beforeAll(() => { + // Radix Dialog relies on DOM APIs jsdom doesn't implement. + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; +}); + +afterEach(cleanup); + +describe('CreateNewTableModal', () => { + it('stops the table name, primary key and database name at the lengths the form accepts, and says so there', () => { + render( + + {}} databaseName={undefined} onCreated={() => {}} /> + , + ); + const table = screen.getByLabelText('Table Name') as HTMLInputElement; + const primaryKey = screen.getByLabelText('Primary Key') as HTMLInputElement; + const database = screen.getByLabelText('Database Name') as HTMLInputElement; + expect(table.maxLength).toBe(250); + expect(primaryKey.maxLength).toBe(250); + expect(database.maxLength).toBe(75); + + fireEvent.change(table, { target: { value: 't'.repeat(250) } }); + fireEvent.change(primaryKey, { target: { value: 'k'.repeat(250) } }); + fireEvent.change(database, { target: { value: 'd'.repeat(75) } }); + expect(screen.getAllByText('Maximum 250 characters reached.')).toHaveLength(2); + expect(screen.getByText('Maximum 75 characters reached.')).toBeTruthy(); + }); +}); diff --git a/src/features/instance/databases/modals/CreateNewTableModal.tsx b/src/features/instance/databases/modals/CreateNewTableModal.tsx index d7a5bbcb7..b58ca0808 100644 --- a/src/features/instance/databases/modals/CreateNewTableModal.tsx +++ b/src/features/instance/databases/modals/CreateNewTableModal.tsx @@ -8,12 +8,11 @@ import { DialogTitle, } from '@/components/ui/dialog'; import { Form } from '@/components/ui/form/Form'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { useCreateTableMutation } from '@/integrations/api/instance/database/createTable'; import { databaseNameSchema } from '@/integrations/api/instance/database/databaseNameSchema'; @@ -108,17 +107,15 @@ export function CreateNewTableModal({ isModalOpen, setIsModalOpen, databaseName, render={({ field }) => ( Table Name - - - + )} @@ -129,17 +126,15 @@ export function CreateNewTableModal({ isModalOpen, setIsModalOpen, databaseName, render={({ field }) => ( Primary Key - - - + )} @@ -150,17 +145,15 @@ export function CreateNewTableModal({ isModalOpen, setIsModalOpen, databaseName, render={({ field }) => ( Database Name - - - + )} diff --git a/src/features/organization/users/modals/AddUserModal.email.test.tsx b/src/features/organization/users/modals/AddUserModal.email.test.tsx index f6031f023..1d06ef374 100644 --- a/src/features/organization/users/modals/AddUserModal.email.test.tsx +++ b/src/features/organization/users/modals/AddUserModal.email.test.tsx @@ -1,9 +1,11 @@ /** * @vitest-environment jsdom */ -import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +const { addUser, inviteUser } = vi.hoisted(() => ({ addUser: vi.fn(), inviteUser: vi.fn() })); + vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } })); vi.mock('@tanstack/react-router', () => ({ useParams: () => ({ organizationId: 'org-1' }) })); vi.mock('@tanstack/react-query', async (importOriginal) => ({ @@ -12,10 +14,10 @@ vi.mock('@tanstack/react-query', async (importOriginal) => ({ })); vi.mock('@/features/organization/mutations/addUserToOrganizationRole', async (importOriginal) => ({ ...(await importOriginal()), - useAddUserToOrganizationRole: () => ({ mutate: vi.fn(), isPending: false }), + useAddUserToOrganizationRole: () => ({ mutate: addUser, isPending: false }), })); vi.mock('@/features/organization/mutations/inviteUserToOrganizationRole', () => ({ - useInviteUserToOrganizationRole: () => ({ mutate: vi.fn(), isPending: false }), + useInviteUserToOrganizationRole: () => ({ mutate: inviteUser, isPending: false }), })); import { AddUserModal } from './AddUserModal'; @@ -31,12 +33,21 @@ beforeAll(() => { afterEach(cleanup); describe('AddUserModal', () => { - it('stops the email at the 80 characters the form accepts, and says so there', () => { + // A cut address can still be a valid one, and the invite would go to it, so the email is refused rather than cut. + it('lets an over-long email be typed in full and refuses it on submit, as a password field would', async () => { render( {}} setIsModalOpen={() => {}} />); const email = screen.getByLabelText('Email') as HTMLInputElement; - expect(email.maxLength).toBe(80); + expect(email.maxLength).toBe(-1); + + await act(async () => { + fireEvent.change(email, { target: { value: `${'u'.repeat(69)}@example.com` } }); + fireEvent.click(screen.getByRole('button', { name: /Add User/ })); + }); - fireEvent.change(email, { target: { value: 'a'.repeat(80) } }); - expect(screen.getByText('Maximum 80 characters reached.')).toBeTruthy(); + expect(email.value).toHaveLength(81); + expect(await screen.findByText('Email cannot be longer than 80 characters.')).toBeTruthy(); + expect(document.querySelector('[data-slot="form-length-hint"]')).toBeNull(); + expect(addUser).not.toHaveBeenCalled(); + expect(inviteUser).not.toHaveBeenCalled(); }); }); diff --git a/src/features/organization/users/modals/AddUserModal.tsx b/src/features/organization/users/modals/AddUserModal.tsx index c29da2c53..0f1a7246e 100644 --- a/src/features/organization/users/modals/AddUserModal.tsx +++ b/src/features/organization/users/modals/AddUserModal.tsx @@ -15,7 +15,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; +import { Input } from '@/components/ui/input'; import { Select, SelectContent, @@ -119,13 +119,9 @@ export function AddUserModal({ render={({ field }) => ( Email - + + + )} diff --git a/src/features/organizations/components/NewOrgForm.test.tsx b/src/features/organizations/components/NewOrgForm.test.tsx new file mode 100644 index 000000000..ee7379ba9 --- /dev/null +++ b/src/features/organizations/components/NewOrgForm.test.tsx @@ -0,0 +1,40 @@ +/** + * @vitest-environment jsdom + */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@tanstack/react-router', () => ({ useNavigate: () => vi.fn() })); +vi.mock('@/hooks/useAuth', () => ({ useCloudAuth: () => ({ user: { firstname: 'Ada', lastname: 'Lovelace' } }) })); +vi.mock('@/features/auth/store/authStore', () => ({ authStore: { reloadUser: vi.fn() }, OverallAppSignIn: 'app' })); +vi.mock('@/features/organizations/hooks/useCreateNewOrganization', () => ({ + useCreateNewOrganizationMutation: () => ({ mutate: vi.fn(), isPending: false }), +})); + +import { NewOrgForm } from './NewOrgForm'; + +afterEach(cleanup); + +function renderForm() { + render( + + + , + ); +} + +describe('NewOrgForm', () => { + it('stops the name and subdomain at the lengths the form accepts, and says so there', () => { + renderForm(); + const name = screen.getByLabelText('Name') as HTMLInputElement; + const subdomain = screen.getByLabelText('Subdomain') as HTMLInputElement; + expect(name.maxLength).toBe(255); + expect(subdomain.maxLength).toBe(62); + + fireEvent.change(name, { target: { value: 'n'.repeat(255) } }); + fireEvent.change(subdomain, { target: { value: 's'.repeat(62) } }); + expect(screen.getByText('Maximum 255 characters reached.')).toBeTruthy(); + expect(screen.getByText('Maximum 62 characters reached.')).toBeTruthy(); + }); +}); diff --git a/src/features/organizations/components/NewOrgForm.tsx b/src/features/organizations/components/NewOrgForm.tsx index 223fda5d0..5dbd3bd1c 100644 --- a/src/features/organizations/components/NewOrgForm.tsx +++ b/src/features/organizations/components/NewOrgForm.tsx @@ -2,13 +2,12 @@ import { HostnamePreview } from '@/components/HostnamePreview'; import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader } from '@/components/ui/card'; import { Form } from '@/components/ui/form/Form'; -import { FormControl } from '@/components/ui/form/FormControl'; import { FormDescription } from '@/components/ui/form/FormDescription'; import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { Input } from '@/components/ui/input'; +import { LengthLimitedInput } from '@/components/ui/form/LengthLimitedInput'; import { currentUserQueryKey } from '@/features/auth/queries/getCurrentUser'; import { authStore, OverallAppSignIn } from '@/features/auth/store/authStore'; import { useCreateNewOrganizationMutation } from '@/features/organizations/hooks/useCreateNewOrganization'; @@ -97,16 +96,14 @@ export function NewOrgForm() { render={({ field }) => ( Name - - - + Use a name your team will recognize. @@ -119,16 +116,14 @@ export function NewOrgForm() { render={({ field }) => ( Subdomain - - - + Lowercase letters, numbers, and dashes. Leave blank to use the suggested subdomain. From b8b4519cdf88ccd3a72353a2dcaa39e6325d9a5b Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 04:31:11 -0400 Subject: [PATCH 4/4] fix(auth): accept the same passwords on reset, sign-up and Profile Reset Password refused anything over 50 characters, while sign-up and Profile set no limit at all. None of those matched central manager: its only password cap is `VALIDATION_CONSTRAINTS.USER.PASSWORD_MAX_LENGTH` (256), applied by `validateAddUser` to sign-up, while `validateResetPassword` and `validateUpdateUser` take any length. So a password manager's 64-character password worked for sign-up and Profile but could not be reset to, and a sign-up password over 256 passed the form only to be refused by the server. All three forms now cap the password at one shared `ACCOUNT_PASSWORD_MAX_LENGTH` (256) with the same message. Password inputs still have no `maxLength`, so an over-long paste is shown in full with the error rather than silently clamped. Closes #1883 Co-Authored-By: Claude Opus 5.5 --- src/features/auth/ResetPassword.test.tsx | 30 ++++++++++++++----- src/features/auth/ResetPassword.tsx | 4 +-- src/features/auth/SignUp.test.tsx | 23 ++++++++++++++ src/features/auth/SignUp.tsx | 5 ++-- src/features/profile/ProfileIndex.test.tsx | 20 +++++++++++++ .../profile/mutations/updateUserSchema.ts | 2 ++ src/lib/zod/password.ts | 6 ++++ 7 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/features/auth/ResetPassword.test.tsx b/src/features/auth/ResetPassword.test.tsx index fd526e70f..4ddadc243 100644 --- a/src/features/auth/ResetPassword.test.tsx +++ b/src/features/auth/ResetPassword.test.tsx @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; vi.mock('@tanstack/react-router', () => ({ @@ -19,6 +19,14 @@ afterEach(() => { vi.clearAllMocks(); }); +async function submitNewPassword(value: string) { + await act(async () => { + fireEvent.change(screen.getByLabelText('New Password'), { target: { value } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value } }); + fireEvent.click(screen.getByRole('button', { name: 'Submit New Password' })); + }); +} + describe('ResetPassword', () => { // maxlength would silently clamp a pasted password, leaving the password manager holding one that never worked. it('lets an over-long password be typed in full and explains the limit instead', async () => { @@ -28,15 +36,23 @@ describe('ResetPassword', () => { expect(password.maxLength).toBe(-1); expect(confirmation.maxLength).toBe(-1); - await act(async () => { - fireEvent.change(password, { target: { value: 'p'.repeat(51) } }); - fireEvent.change(confirmation, { target: { value: 'p'.repeat(51) } }); - fireEvent.click(screen.getByRole('button', { name: 'Submit New Password' })); - }); + await submitNewPassword('p'.repeat(257)); - expect(await screen.findByText('Password cannot be longer than 50 characters.')).toBeTruthy(); + expect(password.value).toHaveLength(257); + expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy(); expect(mutate).not.toHaveBeenCalled(); }); + + it.each([51, 256])('accepts a %i-character password, as sign-up does', async (length) => { + render(); + const value = 'p'.repeat(length); + + await submitNewPassword(value); + + await waitFor(() => expect(mutate).toHaveBeenCalledTimes(1)); + expect(mutate.mock.calls[0][0]).toEqual({ token: 'reset-token', password: value }); + expect(screen.queryByText(/cannot be longer than/)).toBeNull(); + }); }); describe('ResetPassword password fields', () => { diff --git a/src/features/auth/ResetPassword.tsx b/src/features/auth/ResetPassword.tsx index 36bfa3bcc..792cdbedd 100644 --- a/src/features/auth/ResetPassword.tsx +++ b/src/features/auth/ResetPassword.tsx @@ -4,7 +4,7 @@ import { FormField } from '@/components/ui/form/FormField'; import { FormItem } from '@/components/ui/form/FormItem'; import { FormLabel } from '@/components/ui/form/FormLabel'; import { FormMessage } from '@/components/ui/form/FormMessage'; -import { zodRequirePassword } from '@/lib/zod/password'; +import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR, zodRequirePassword } from '@/lib/zod/password'; import { zodResolver } from '@hookform/resolvers/zod'; import { useNavigate, useSearch } from '@tanstack/react-router'; import { useCallback, useEffect } from 'react'; @@ -18,7 +18,7 @@ const ResetPasswordSchema = z .object({ password: zodRequirePassword .min(8, { error: 'Password must be at least 8 characters long.' }) - .max(50, { error: 'Password cannot be longer than 50 characters.' }), + .max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR }), confirmPassword: z.string(), }) .refine((data) => data.password === data.confirmPassword, { diff --git a/src/features/auth/SignUp.test.tsx b/src/features/auth/SignUp.test.tsx index db938fdf7..d61e53874 100644 --- a/src/features/auth/SignUp.test.tsx +++ b/src/features/auth/SignUp.test.tsx @@ -109,6 +109,29 @@ describe('SignUp', () => { expect(maxLengthOf('Confirm Password')).toBe(-1); }); + it('refuses a password longer than central manager accepts before sending it', async () => { + renderSignUp(); + fillValidForm(); + fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(257) } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(257) } }); + submit(); + + expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy(); + expect(post).not.toHaveBeenCalled(); + }); + + it('sends a password right at central manager’s limit', async () => { + post.mockResolvedValue({ data: { id: 'usr-1', email: 'taken@example.com' } }); + renderSignUp(); + fillValidForm(); + fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'p'.repeat(256) } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'p'.repeat(256) } }); + submit(); + + await waitFor(() => expect(post).toHaveBeenCalledTimes(1)); + expect(post.mock.calls[0][1]).toMatchObject({ password: 'p'.repeat(256) }); + }); + it('reveals each password independently without submitting or losing field values', () => { renderSignUp(); const password = screen.getByLabelText('Password') as HTMLInputElement; diff --git a/src/features/auth/SignUp.tsx b/src/features/auth/SignUp.tsx index e2edaf1a0..f3d59a32a 100644 --- a/src/features/auth/SignUp.tsx +++ b/src/features/auth/SignUp.tsx @@ -11,7 +11,7 @@ import { parseCompanyFromEmail } from '@/lib/string/parseCompanyFromEmail'; import { personNameRegex } from '@/lib/string/regex/personNameRegex'; import { clearUtmParamsFromUrl } from '@/lib/urls/clearUtmParams'; import { zodRequireEmail } from '@/lib/zod/email'; -import { zodRequirePassword } from '@/lib/zod/password'; +import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR, zodRequirePassword } from '@/lib/zod/password'; import { zodResolver } from '@hookform/resolvers/zod'; import { Link, useNavigate, useSearch } from '@tanstack/react-router'; import { MouseEvent, useCallback, useEffect, useState } from 'react'; @@ -53,7 +53,8 @@ const SignUpSchema = z.object({ ) .max(80, { error: 'Last name cannot be longer than 80 characters.' }), password: zodRequirePassword - .min(8, { error: 'Password must be at least 8 characters long.' }), + .min(8, { error: 'Password must be at least 8 characters long.' }) + .max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR }), confirmPassword: z.string(), acceptTerms: z.boolean().refine(val => val === true, { message: 'You must accept the Privacy Policy and Terms of Service.', diff --git a/src/features/profile/ProfileIndex.test.tsx b/src/features/profile/ProfileIndex.test.tsx index 566178ef5..6aefffa4b 100644 --- a/src/features/profile/ProfileIndex.test.tsx +++ b/src/features/profile/ProfileIndex.test.tsx @@ -66,6 +66,26 @@ describe('ProfileIndex', () => { await waitFor(() => expect(saveButton().disabled).toBe(false)); }); + it('refuses a new password longer than sign-up accepts once the field is left', async () => { + render(); + await type('New Password', 'p'.repeat(257)); + await leave('New Password'); + + expect(await screen.findByText('Password cannot be longer than 256 characters.')).toBeTruthy(); + expect(saveButton().disabled).toBe(true); + }); + + it('accepts a new password as long as sign-up allows', async () => { + render(); + await type('New Password', 'p'.repeat(256)); + await leave('New Password'); + await type('Confirm New Password', 'p'.repeat(256)); + await leave('Confirm New Password'); + + await waitFor(() => expect(saveButton().disabled).toBe(false)); + expect(screen.queryByText(/cannot be longer than/)).toBeNull(); + }); + it('explains a short new password when Enter is pressed in it', async () => { render(); await type('New Password', 'short'); diff --git a/src/features/profile/mutations/updateUserSchema.ts b/src/features/profile/mutations/updateUserSchema.ts index 9abd86380..af2a3dec7 100644 --- a/src/features/profile/mutations/updateUserSchema.ts +++ b/src/features/profile/mutations/updateUserSchema.ts @@ -1,3 +1,4 @@ +import { ACCOUNT_PASSWORD_MAX_LENGTH, ACCOUNT_PASSWORD_TOO_LONG_ERROR } from '@/lib/zod/password'; import { z } from 'zod'; export const UpdateUserSchema = z @@ -20,6 +21,7 @@ export const UpdateUserSchema = z error: 'Please enter your new password.', }) .min(8, { error: 'Password must be 8 characters or more.' }) + .max(ACCOUNT_PASSWORD_MAX_LENGTH, { error: ACCOUNT_PASSWORD_TOO_LONG_ERROR }) .or(z.string().max(0)), confirmNewPassword: z .string() diff --git a/src/lib/zod/password.ts b/src/lib/zod/password.ts index 09039c187..c4afd3378 100644 --- a/src/lib/zod/password.ts +++ b/src/lib/zod/password.ts @@ -3,3 +3,9 @@ import { z } from 'zod'; export const zodRequirePassword = z .string() .nonempty({ error: 'Please enter your password.' }); + +// Central manager's `PASSWORD_MAX_LENGTH`, which it enforces only on sign-up. Reset and Profile use it +// too, so every form that sets a Fabric account password accepts the same passwords. +export const ACCOUNT_PASSWORD_MAX_LENGTH = 256; +export const ACCOUNT_PASSWORD_TOO_LONG_ERROR = + `Password cannot be longer than ${ACCOUNT_PASSWORD_MAX_LENGTH} characters.`;