From aabcf52cd26a15c8338d47c8337d2587d8c2d37e Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 20 Dec 2024 14:53:39 +1100 Subject: [PATCH] Forms fixes (#8722) * Refactor form fields - Allow error message to be passed through via field definition - Return error information to onFormError * Fix debounce issue for text fields * Fix for useForm hook * Badge fix - Fix badge rendering for SalesOrderShipment * Cleanup unit test --- src/frontend/src/components/forms/ApiForm.tsx | 18 +++++++++++------- .../components/forms/fields/ApiFormField.tsx | 9 ++++++--- .../components/forms/fields/ChoiceField.tsx | 2 +- .../src/components/forms/fields/DateField.tsx | 2 +- .../src/components/forms/fields/IconField.tsx | 2 +- .../forms/fields/RelatedModelField.tsx | 2 +- .../src/components/forms/fields/TableField.tsx | 2 +- .../src/components/forms/fields/TextField.tsx | 10 ++++++++-- src/frontend/src/hooks/UseForm.tsx | 5 ++--- src/frontend/tests/pages/pui_build.spec.ts | 6 ++---- 10 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index fa1c07b5a1..eaa5cb15cf 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -94,7 +94,7 @@ export interface ApiFormProps { postFormContent?: JSX.Element; successMessage?: string; onFormSuccess?: (data: any) => void; - onFormError?: () => void; + onFormError?: (response: any) => void; processFormData?: (data: any) => any; table?: TableState; modelType?: ModelType; @@ -482,7 +482,7 @@ export function ApiForm({ default: // Unexpected state on form success invalidResponse(response.status); - props.onFormError?.(); + props.onFormError?.(response); break; } @@ -534,26 +534,30 @@ export function ApiForm({ processErrors(error.response.data); setNonFieldErrors(_nonFieldErrors); + props.onFormError?.(error); break; default: // Unexpected state on form error invalidResponse(error.response.status); - props.onFormError?.(); + props.onFormError?.(error); break; } } else { showTimeoutNotification(); - props.onFormError?.(); + props.onFormError?.(error); } return error; }); }; - const onFormError = useCallback>(() => { - props.onFormError?.(); - }, [props.onFormError]); + const onFormError = useCallback>( + (error: any) => { + props.onFormError?.(error); + }, + [props.onFormError] + ); if (optionsLoading || initialDataQuery.isFetching) { return ( diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx index 7ca2506dc0..aa78ec2d42 100644 --- a/src/frontend/src/components/forms/fields/ApiFormField.tsx +++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx @@ -46,6 +46,7 @@ export type ApiFormFieldChoice = { * @param required : Whether the field is required * @param hidden : Whether the field is hidden * @param disabled : Whether the field is disabled + * @param error : Optional error message to display * @param exclude : Whether to exclude the field from the submitted data * @param placeholder : The placeholder text to display * @param description : The description to display for the field @@ -88,6 +89,7 @@ export type ApiFormFieldType = { child?: ApiFormFieldType; children?: { [key: string]: ApiFormFieldType }; required?: boolean; + error?: string; choices?: ApiFormFieldChoice[]; hidden?: boolean; disabled?: boolean; @@ -256,7 +258,7 @@ export function ApiFormField({ aria-label={`boolean-field-${fieldName}`} radius='lg' size='sm' - error={error?.message} + error={definition.error ?? error?.message} onChange={(event) => onChange(event.currentTarget.checked)} /> ); @@ -277,7 +279,7 @@ export function ApiFormField({ id={fieldId} aria-label={`number-field-${field.name}`} value={numericalValue} - error={error?.message} + error={definition.error ?? error?.message} decimalScale={definition.field_type == 'integer' ? 0 : 10} onChange={(value: number | string | null) => onChange(value)} step={1} @@ -299,7 +301,7 @@ export function ApiFormField({ ref={field.ref} radius='sm' value={value} - error={error?.message} + error={definition.error ?? error?.message} onChange={(payload: File | null) => onChange(payload)} /> ); @@ -343,6 +345,7 @@ export function ApiFormField({ booleanValue, control, controller, + definition, field, fieldId, fieldName, diff --git a/src/frontend/src/components/forms/fields/ChoiceField.tsx b/src/frontend/src/components/forms/fields/ChoiceField.tsx index 6a574a0ee3..432875fee0 100644 --- a/src/frontend/src/components/forms/fields/ChoiceField.tsx +++ b/src/frontend/src/components/forms/fields/ChoiceField.tsx @@ -63,7 +63,7 @@ export function ChoiceField({ diff --git a/src/frontend/src/components/forms/fields/TextField.tsx b/src/frontend/src/components/forms/fields/TextField.tsx index 9ebaa796ff..98761716db 100644 --- a/src/frontend/src/components/forms/fields/TextField.tsx +++ b/src/frontend/src/components/forms/fields/TextField.tsx @@ -56,7 +56,7 @@ export default function TextField({ aria-label={`text-field-${field.name}`} type={definition.field_type} value={rawText || ''} - error={error?.message} + error={definition.error ?? error?.message} radius='sm' onChange={(event) => onTextChange(event.currentTarget.value)} onBlur={(event) => { @@ -64,7 +64,13 @@ export default function TextField({ onChange(event.currentTarget.value); } }} - onKeyDown={(event) => onKeyDown(event.code)} + onKeyDown={(event) => { + if (event.code === 'Enter') { + // Bypass debounce on enter key + onChange(event.currentTarget.value); + } + onKeyDown(event.code); + }} rightSection={ value && !definition.required ? ( onTextChange('')} /> diff --git a/src/frontend/src/hooks/UseForm.tsx b/src/frontend/src/hooks/UseForm.tsx index 085ea6d978..bbbcdad4f7 100644 --- a/src/frontend/src/hooks/UseForm.tsx +++ b/src/frontend/src/hooks/UseForm.tsx @@ -48,9 +48,8 @@ export function useApiFormModal(props: ApiFormModalProps) { modalClose.current(); props.onFormSuccess?.(data); }, - onFormError: () => { - modalClose.current(); - props.onFormError?.(); + onFormError: (error: any) => { + props.onFormError?.(error); } }), [props] diff --git a/src/frontend/tests/pages/pui_build.spec.ts b/src/frontend/tests/pages/pui_build.spec.ts index ad2e7e38a6..2d0bf5558a 100644 --- a/src/frontend/tests/pages/pui_build.spec.ts +++ b/src/frontend/tests/pages/pui_build.spec.ts @@ -90,10 +90,8 @@ test('Build Order - Basic Tests', async ({ page }) => { test('Build Order - Build Outputs', async ({ page }) => { await doQuickLogin(page); - await page.goto(`${baseUrl}/part/`); - - // Navigate to the correct build order - await page.getByRole('tab', { name: 'Manufacturing', exact: true }).click(); + await page.goto(`${baseUrl}/manufacturing/index/`); + await page.getByRole('tab', { name: 'Build Orders', exact: true }).click(); // We have now loaded the "Build Order" table. Check for some expected texts await page.getByText('On Hold').first().waitFor();