From a38c3963f27ac23616ba04b6c63384d941cd82f0 Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 10 Jul 2026 16:54:13 +1000 Subject: [PATCH] [UI] Form fix (#12348) * Refactor API form fields - Prevent reconstruction of all fields when single value changes - Memoize components more intelligently - Fix enter key callback ref * memoize other field types too * More memo * Prevent duplicate API calls for related model field * Add aria-label for IconField * add playwright tests for form field coverage --- src/frontend/src/components/forms/ApiForm.tsx | 66 ++++++++-- .../src/components/forms/DateTimeField.tsx | 6 +- .../components/forms/fields/ApiFormField.tsx | 27 ++-- .../components/forms/fields/BooleanField.tsx | 13 +- .../components/forms/fields/ChoiceField.tsx | 10 +- .../src/components/forms/fields/DateField.tsx | 6 +- .../forms/fields/DependentField.tsx | 17 +-- .../src/components/forms/fields/IconField.tsx | 14 ++- .../forms/fields/NestedObjectField.tsx | 5 +- .../components/forms/fields/NumberField.tsx | 25 ++-- .../forms/fields/RelatedModelField.tsx | 15 ++- .../components/forms/fields/TableField.tsx | 38 +----- .../src/components/forms/fields/TagsField.tsx | 6 +- .../src/components/forms/fields/TextField.tsx | 8 +- .../src/components/forms/fields/TreeField.tsx | 6 +- src/frontend/src/functions/comparison.tsx | 55 +++++++++ .../tests/pages/pui_purchasing.spec.ts | 20 +++ src/frontend/tests/pui_forms.spec.ts | 115 +++++++++++++++++- 18 files changed, 355 insertions(+), 97 deletions(-) create mode 100644 src/frontend/src/functions/comparison.tsx diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 1d65b5784f..8d03f9c9b8 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -41,6 +41,7 @@ import type { ApiFormProps } from '@lib/types/Forms'; import { useApi } from '../../contexts/ApiContext'; +import { isEquivalent } from '../../functions/comparison'; import { constructField, extractAvailableFields } from '../../functions/forms'; import { KeepFormOpenSwitch } from './KeepFormOpenSwitch'; import { ApiFormField } from './fields/ApiFormField'; @@ -122,15 +123,23 @@ export function OptionsApiForm({ } }, [opened]); + // Preserve the previous reference for any field whose constructed + // definition did not actually change, so unrelated fields don't force a + // fields-object replacement (and cascade a re-render to every field) + // whenever this recomputes for an unrelated reason (e.g. options data or + // the caller's own fields hook re-running with equivalent output) + const previousConstructedFieldsRef = useRef({}); + const formProps: ApiFormProps = useMemo(() => { const _props = { ...props }; if (!_props.fields) return _props; - _props.fields = { ..._props.fields }; + const prevFields = previousConstructedFieldsRef.current; + const nextFields: ApiFormFieldSet = {}; for (const [k, v] of Object.entries(_props.fields)) { - _props.fields[k] = constructField({ + const constructed = constructField({ field: v, definition: optionsQuery?.data?.[k] }); @@ -139,10 +148,17 @@ export function OptionsApiForm({ const value = _props?.initialData?.[k]; if (value) { - _props.fields[k].value = value; + constructed.value = value; } + + const prev = prevFields[k]; + nextFields[k] = + prev && isEquivalent(prev, constructed) ? prev : constructed; } + previousConstructedFieldsRef.current = nextFields; + _props.fields = nextFields; + return _props; }, [optionsQuery.data, props]); @@ -314,7 +330,21 @@ export function ApiForm({ } } - setFields(_fields); + // Preserve the previous reference for any field whose definition did not + // actually change, so unrelated fields don't re-render just because the + // fields hook rebuilt its entire output (e.g. in response to some other + // field's onValueChange updating local state elsewhere in that hook) + setFields((prevFields) => { + const nextFields: ApiFormFieldSet = {}; + + for (const k of Object.keys(_fields)) { + const prev = prevFields[k]; + const next = _fields[k]; + nextFields[k] = prev && isEquivalent(prev, next) ? prev : next; + } + + return nextFields; + }); }, [props.fields, props.initialData, defaultValues, initialDataQuery.data]); // Fetch initial data on form load @@ -600,6 +630,24 @@ export function ApiForm({ [props.onFormError] ); + // Submit the form when "Enter" is pressed in a field. + // Routed through a ref so that the callback identity passed down to every + // field stays stable across renders - isValid / isDirty change on every + // keystroke anywhere in the form, and a fresh onKeyDown reference here + // would force every field (not just the one being edited) to re-render. + const submitOnEnterRef = useRef<() => void>(() => {}); + submitOnEnterRef.current = () => { + if (!isLoading && (!props.fetchInitialData || isDirty)) { + form.handleSubmit(submitForm, onFormError)(); + } + }; + + const onFieldKeyDown = useCallback((value: any) => { + if (value === 'Enter') { + submitOnEnterRef.current(); + } + }, []); + // Submit the form with Ctrl+Enter (Cmd+Enter on Mac) while it is open. // An empty tagsToIgnore list allows the hotkey to fire from within form inputs. useInvenTreeHotkeys( @@ -685,15 +733,7 @@ export function ApiForm({ url={url} navigate={navigate} setFields={setFields} - onKeyDown={(value) => { - if ( - value == 'Enter' && - !isLoading && - (!props.fetchInitialData || isDirty) - ) { - form.handleSubmit(submitForm, onFormError)(); - } - }} + onKeyDown={onFieldKeyDown} /> ); })} diff --git a/src/frontend/src/components/forms/DateTimeField.tsx b/src/frontend/src/components/forms/DateTimeField.tsx index 7c888f72af..0854f5c00c 100644 --- a/src/frontend/src/components/forms/DateTimeField.tsx +++ b/src/frontend/src/components/forms/DateTimeField.tsx @@ -3,12 +3,12 @@ import { t } from '@lingui/core/macro'; import { DateTimePicker } from '@mantine/dates'; import dayjs from 'dayjs'; import customParseFormat from 'dayjs/plugin/customParseFormat'; -import { useCallback, useId, useMemo } from 'react'; +import { memo, useCallback, useId, useMemo } from 'react'; import type { FieldValues, UseControllerReturn } from 'react-hook-form'; dayjs.extend(customParseFormat); -export default function DateTimeField({ +function DateTimeField({ controller, definition }: Readonly<{ @@ -73,3 +73,5 @@ export default function DateTimeField({ /> ); } + +export default memo(DateTimeField); diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx index 1d76affc79..3212c3ed89 100644 --- a/src/frontend/src/components/forms/fields/ApiFormField.tsx +++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx @@ -121,6 +121,15 @@ export function ApiFormField({ [fieldName, definition] ); + // Stable wrapper so the identity passed to leaf field components does not + // change unless onKeyDown itself changes (onKeyDown may be undefined) + const safeOnKeyDown = useCallback( + (value: any) => { + onKeyDown?.(value); + }, + [onKeyDown] + ); + // Construct the individual field const fieldInstance = useMemo(() => { switch (fieldDefinition.field_type) { @@ -177,9 +186,7 @@ export function ApiFormField({ controller={controller} fieldName={fieldName} onChange={onChange} - onKeyDown={(value) => { - onKeyDown?.(value); - }} + onKeyDown={safeOnKeyDown} /> ); case 'password': @@ -189,9 +196,7 @@ export function ApiFormField({ controller={controller} fieldName={fieldName} onChange={onChange} - onKeyDown={(value) => { - onKeyDown?.(value); - }} + onKeyDown={safeOnKeyDown} /> ); case 'icon': @@ -204,9 +209,7 @@ export function ApiFormField({ controller={controller} definition={reducedDefinition} fieldName={fieldName} - onChange={(value: boolean) => { - onChange(value); - }} + onChange={onChange} /> ); case 'date': @@ -231,9 +234,7 @@ export function ApiFormField({ fieldDefinition.placeholderWarningCompare ?? undefined } placeholderWarning={fieldDefinition.placeholderWarning ?? undefined} - onChange={(value: any) => { - onChange(value); - }} + onChange={onChange} /> ); case 'choice': @@ -311,7 +312,7 @@ export function ApiFormField({ fieldName, fieldDefinition, onChange, - onKeyDown, + safeOnKeyDown, reducedDefinition, ref, setFields, diff --git a/src/frontend/src/components/forms/fields/BooleanField.tsx b/src/frontend/src/components/forms/fields/BooleanField.tsx index 033e58cf30..7fc01aa065 100644 --- a/src/frontend/src/components/forms/fields/BooleanField.tsx +++ b/src/frontend/src/components/forms/fields/BooleanField.tsx @@ -2,10 +2,10 @@ import { isTrue } from '@lib/functions/Conversion'; import type { ApiFormFieldType } from '@lib/types/Forms'; import { Switch } from '@mantine/core'; import { useId } from '@mantine/hooks'; -import { useEffect, useMemo } from 'react'; +import { memo, useCallback, useEffect, useMemo } from 'react'; import type { FieldValues, UseControllerReturn } from 'react-hook-form'; -export function BooleanField({ +function BooleanFieldComponent({ controller, definition, fieldName, @@ -37,6 +37,11 @@ export function BooleanField({ return isTrue(value ?? definition.default ?? false); }, [value]); + const handleChange = useCallback( + (event: any) => onChange(event.currentTarget.checked || false), + [onChange] + ); + return ( onChange(event.currentTarget.checked || false)} + onChange={handleChange} /> ); } + +export const BooleanField = memo(BooleanFieldComponent); diff --git a/src/frontend/src/components/forms/fields/ChoiceField.tsx b/src/frontend/src/components/forms/fields/ChoiceField.tsx index 38258d2d86..e648320dce 100644 --- a/src/frontend/src/components/forms/fields/ChoiceField.tsx +++ b/src/frontend/src/components/forms/fields/ChoiceField.tsx @@ -1,13 +1,13 @@ import type { ApiFormFieldType } from '@lib/types/Forms'; import { Select } from '@mantine/core'; import { useId } from '@mantine/hooks'; -import { useCallback, useMemo, useRef } from 'react'; +import { memo, useCallback, useMemo, useRef } from 'react'; import type { FieldValues, UseControllerReturn } from 'react-hook-form'; /** * Render a 'select' field for selecting from a list of choices */ -export function ChoiceField({ +function ChoiceFieldComponent({ controller, definition, fieldName @@ -64,6 +64,8 @@ export function ChoiceField({ } }, [value]); + const onDropdownOpen = useCallback(() => inputRef?.current?.select(), []); + return (