Skip to content

Commit

Permalink
Forms fixes (#8722)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
SchrodingersGat authored Dec 20, 2024
1 parent 68ac411 commit aabcf52
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 24 deletions.
18 changes: 11 additions & 7 deletions src/frontend/src/components/forms/ApiForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -482,7 +482,7 @@ export function ApiForm({
default:
// Unexpected state on form success
invalidResponse(response.status);
props.onFormError?.();
props.onFormError?.(response);
break;
}

Expand Down Expand Up @@ -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<SubmitErrorHandler<FieldValues>>(() => {
props.onFormError?.();
}, [props.onFormError]);
const onFormError = useCallback<SubmitErrorHandler<FieldValues>>(
(error: any) => {
props.onFormError?.(error);
},
[props.onFormError]
);

if (optionsLoading || initialDataQuery.isFetching) {
return (
Expand Down
9 changes: 6 additions & 3 deletions src/frontend/src/components/forms/fields/ApiFormField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,6 +89,7 @@ export type ApiFormFieldType = {
child?: ApiFormFieldType;
children?: { [key: string]: ApiFormFieldType };
required?: boolean;
error?: string;
choices?: ApiFormFieldChoice[];
hidden?: boolean;
disabled?: boolean;
Expand Down Expand Up @@ -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)}
/>
);
Expand All @@ -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}
Expand All @@ -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)}
/>
);
Expand Down Expand Up @@ -343,6 +345,7 @@ export function ApiFormField({
booleanValue,
control,
controller,
definition,
field,
fieldId,
fieldName,
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/components/forms/fields/ChoiceField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function ChoiceField({
<Select
id={fieldId}
aria-label={`choice-field-${field.name}`}
error={error?.message}
error={definition.error ?? error?.message}
radius='sm'
{...field}
onChange={onChange}
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/components/forms/fields/DateField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default function DateField({
radius='sm'
ref={field.ref}
type={undefined}
error={error?.message}
error={definition.error ?? error?.message}
value={dateValue ?? null}
clearable={!definition.required}
onChange={onChange}
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/components/forms/fields/IconField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function IconField({
label={definition.label}
description={definition.description}
required={definition.required}
error={error?.message}
error={definition.error ?? error?.message}
ref={field.ref}
component='button'
type='button'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export function RelatedModelField({
return (
<Input.Wrapper
{...fieldDefinition}
error={error?.message}
error={definition.error ?? error?.message}
styles={{ description: { paddingBottom: '5px' } }}
>
<Select
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/components/forms/fields/TableField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export function TableFieldExtraRow({
fieldName={fieldName ?? 'field'}
fieldDefinition={field}
defaultValue={defaultValue}
error={error}
error={fieldDefinition.error ?? error}
/>
</Group>
</Table.Td>
Expand Down
10 changes: 8 additions & 2 deletions src/frontend/src/components/forms/fields/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,21 @@ 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) => {
if (event.currentTarget.value != value) {
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 ? (
<IconX size='1rem' color='red' onClick={() => onTextChange('')} />
Expand Down
5 changes: 2 additions & 3 deletions src/frontend/src/hooks/UseForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 2 additions & 4 deletions src/frontend/tests/pages/pui_build.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit aabcf52

Please sign in to comment.