[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
This commit is contained in:
Oliver
2026-07-10 16:54:13 +10:00
committed by GitHub
parent 9fd074b202
commit a38c3963f2
18 changed files with 355 additions and 97 deletions
+53 -13
View File
@@ -41,6 +41,7 @@ import type {
ApiFormProps ApiFormProps
} from '@lib/types/Forms'; } from '@lib/types/Forms';
import { useApi } from '../../contexts/ApiContext'; import { useApi } from '../../contexts/ApiContext';
import { isEquivalent } from '../../functions/comparison';
import { constructField, extractAvailableFields } from '../../functions/forms'; import { constructField, extractAvailableFields } from '../../functions/forms';
import { KeepFormOpenSwitch } from './KeepFormOpenSwitch'; import { KeepFormOpenSwitch } from './KeepFormOpenSwitch';
import { ApiFormField } from './fields/ApiFormField'; import { ApiFormField } from './fields/ApiFormField';
@@ -122,15 +123,23 @@ export function OptionsApiForm({
} }
}, [opened]); }, [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<ApiFormFieldSet>({});
const formProps: ApiFormProps = useMemo(() => { const formProps: ApiFormProps = useMemo(() => {
const _props = { ...props }; const _props = { ...props };
if (!_props.fields) return _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)) { for (const [k, v] of Object.entries(_props.fields)) {
_props.fields[k] = constructField({ const constructed = constructField({
field: v, field: v,
definition: optionsQuery?.data?.[k] definition: optionsQuery?.data?.[k]
}); });
@@ -139,10 +148,17 @@ export function OptionsApiForm({
const value = _props?.initialData?.[k]; const value = _props?.initialData?.[k];
if (value) { 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; return _props;
}, [optionsQuery.data, 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]); }, [props.fields, props.initialData, defaultValues, initialDataQuery.data]);
// Fetch initial data on form load // Fetch initial data on form load
@@ -600,6 +630,24 @@ export function ApiForm({
[props.onFormError] [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. // 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. // An empty tagsToIgnore list allows the hotkey to fire from within form inputs.
useInvenTreeHotkeys( useInvenTreeHotkeys(
@@ -685,15 +733,7 @@ export function ApiForm({
url={url} url={url}
navigate={navigate} navigate={navigate}
setFields={setFields} setFields={setFields}
onKeyDown={(value) => { onKeyDown={onFieldKeyDown}
if (
value == 'Enter' &&
!isLoading &&
(!props.fetchInitialData || isDirty)
) {
form.handleSubmit(submitForm, onFormError)();
}
}}
/> />
); );
})} })}
@@ -3,12 +3,12 @@ import { t } from '@lingui/core/macro';
import { DateTimePicker } from '@mantine/dates'; import { DateTimePicker } from '@mantine/dates';
import dayjs from 'dayjs'; import dayjs from 'dayjs';
import customParseFormat from 'dayjs/plugin/customParseFormat'; 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'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
dayjs.extend(customParseFormat); dayjs.extend(customParseFormat);
export default function DateTimeField({ function DateTimeField({
controller, controller,
definition definition
}: Readonly<{ }: Readonly<{
@@ -73,3 +73,5 @@ export default function DateTimeField({
/> />
); );
} }
export default memo(DateTimeField);
@@ -121,6 +121,15 @@ export function ApiFormField({
[fieldName, definition] [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 // Construct the individual field
const fieldInstance = useMemo(() => { const fieldInstance = useMemo(() => {
switch (fieldDefinition.field_type) { switch (fieldDefinition.field_type) {
@@ -177,9 +186,7 @@ export function ApiFormField({
controller={controller} controller={controller}
fieldName={fieldName} fieldName={fieldName}
onChange={onChange} onChange={onChange}
onKeyDown={(value) => { onKeyDown={safeOnKeyDown}
onKeyDown?.(value);
}}
/> />
); );
case 'password': case 'password':
@@ -189,9 +196,7 @@ export function ApiFormField({
controller={controller} controller={controller}
fieldName={fieldName} fieldName={fieldName}
onChange={onChange} onChange={onChange}
onKeyDown={(value) => { onKeyDown={safeOnKeyDown}
onKeyDown?.(value);
}}
/> />
); );
case 'icon': case 'icon':
@@ -204,9 +209,7 @@ export function ApiFormField({
controller={controller} controller={controller}
definition={reducedDefinition} definition={reducedDefinition}
fieldName={fieldName} fieldName={fieldName}
onChange={(value: boolean) => { onChange={onChange}
onChange(value);
}}
/> />
); );
case 'date': case 'date':
@@ -231,9 +234,7 @@ export function ApiFormField({
fieldDefinition.placeholderWarningCompare ?? undefined fieldDefinition.placeholderWarningCompare ?? undefined
} }
placeholderWarning={fieldDefinition.placeholderWarning ?? undefined} placeholderWarning={fieldDefinition.placeholderWarning ?? undefined}
onChange={(value: any) => { onChange={onChange}
onChange(value);
}}
/> />
); );
case 'choice': case 'choice':
@@ -311,7 +312,7 @@ export function ApiFormField({
fieldName, fieldName,
fieldDefinition, fieldDefinition,
onChange, onChange,
onKeyDown, safeOnKeyDown,
reducedDefinition, reducedDefinition,
ref, ref,
setFields, setFields,
@@ -2,10 +2,10 @@ import { isTrue } from '@lib/functions/Conversion';
import type { ApiFormFieldType } from '@lib/types/Forms'; import type { ApiFormFieldType } from '@lib/types/Forms';
import { Switch } from '@mantine/core'; import { Switch } from '@mantine/core';
import { useId } from '@mantine/hooks'; 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'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
export function BooleanField({ function BooleanFieldComponent({
controller, controller,
definition, definition,
fieldName, fieldName,
@@ -37,6 +37,11 @@ export function BooleanField({
return isTrue(value ?? definition.default ?? false); return isTrue(value ?? definition.default ?? false);
}, [value]); }, [value]);
const handleChange = useCallback(
(event: any) => onChange(event.currentTarget.checked || false),
[onChange]
);
return ( return (
<Switch <Switch
{...definition} {...definition}
@@ -47,7 +52,9 @@ export function BooleanField({
radius='lg' radius='lg'
size='sm' size='sm'
error={definition.error ?? error?.message} error={definition.error ?? error?.message}
onChange={(event: any) => onChange(event.currentTarget.checked || false)} onChange={handleChange}
/> />
); );
} }
export const BooleanField = memo(BooleanFieldComponent);
@@ -1,13 +1,13 @@
import type { ApiFormFieldType } from '@lib/types/Forms'; import type { ApiFormFieldType } from '@lib/types/Forms';
import { Select } from '@mantine/core'; import { Select } from '@mantine/core';
import { useId } from '@mantine/hooks'; 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'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
/** /**
* Render a 'select' field for selecting from a list of choices * Render a 'select' field for selecting from a list of choices
*/ */
export function ChoiceField({ function ChoiceFieldComponent({
controller, controller,
definition, definition,
fieldName fieldName
@@ -64,6 +64,8 @@ export function ChoiceField({
} }
}, [value]); }, [value]);
const onDropdownOpen = useCallback(() => inputRef?.current?.select(), []);
return ( return (
<Select <Select
id={fieldId} id={fieldId}
@@ -72,7 +74,7 @@ export function ChoiceField({
radius='sm' radius='sm'
{...field} {...field}
ref={inputRef} ref={inputRef}
onDropdownOpen={() => inputRef?.current?.select()} onDropdownOpen={onDropdownOpen}
onChange={onChange} onChange={onChange}
data={choices} data={choices}
value={choiceValue} value={choiceValue}
@@ -88,3 +90,5 @@ export function ChoiceField({
/> />
); );
} }
export const ChoiceField = memo(ChoiceFieldComponent);
@@ -3,12 +3,12 @@ import { t } from '@lingui/core/macro';
import { DateInput } from '@mantine/dates'; import { DateInput } from '@mantine/dates';
import dayjs from 'dayjs'; import dayjs from 'dayjs';
import customParseFormat from 'dayjs/plugin/customParseFormat'; 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'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
dayjs.extend(customParseFormat); dayjs.extend(customParseFormat);
export default function DateField({ function DateField({
controller, controller,
definition definition
}: Readonly<{ }: Readonly<{
@@ -76,3 +76,5 @@ export default function DateField({
/> />
); );
} }
export default memo(DateField);
@@ -1,4 +1,4 @@
import { useEffect, useMemo } from 'react'; import { memo, useEffect, useMemo } from 'react';
import { import {
type Control, type Control,
type FieldValues, type FieldValues,
@@ -13,7 +13,7 @@ import {
} from '../../../functions/forms'; } from '../../../functions/forms';
import { ApiFormField } from './ApiFormField'; import { ApiFormField } from './ApiFormField';
export function DependentField({ function DependentFieldComponent({
control, control,
fieldName, fieldName,
definition, definition,
@@ -51,14 +51,15 @@ export function DependentField({
extractAvailableFields(res, 'POST'); extractAvailableFields(res, 'POST');
// update the fields in the form state with the new fields // update the fields in the form state with the new fields
// (preserve the reference for any field that did not actually change,
// so unrelated fields don't re-render on every dependent-field update)
setFields((prevFields) => { setFields((prevFields) => {
const newFields: Record<string, ReturnType<typeof constructField>> = {}; const newFields: ApiFormFieldSet = {};
for (const [k, v] of Object.entries(prevFields)) { for (const [k, v] of Object.entries(prevFields)) {
newFields[k] = constructField({ newFields[k] = fields?.[k]
field: v, ? constructField({ field: v, definition: fields[k] })
definition: fields?.[k] : v;
});
} }
return newFields; return newFields;
@@ -89,3 +90,5 @@ export function DependentField({
/> />
); );
} }
export const DependentField = memo(DependentFieldComponent);
@@ -17,7 +17,14 @@ import {
import { useDebouncedValue, useElementSize } from '@mantine/hooks'; import { useDebouncedValue, useElementSize } from '@mantine/hooks';
import { IconX } from '@tabler/icons-react'; import { IconX } from '@tabler/icons-react';
import Fuse from 'fuse.js'; import Fuse from 'fuse.js';
import { startTransition, useEffect, useMemo, useRef, useState } from 'react'; import {
memo,
startTransition,
useEffect,
useMemo,
useRef,
useState
} from 'react';
import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
import { FixedSizeGrid as Grid } from 'react-window'; import { FixedSizeGrid as Grid } from 'react-window';
@@ -26,7 +33,7 @@ import { useShallow } from 'zustand/react/shallow';
import { useIconState } from '../../../states/IconState'; import { useIconState } from '../../../states/IconState';
import { ApiIcon } from '../../items/ApiIcon'; import { ApiIcon } from '../../items/ApiIcon';
export default function IconField({ function IconField({
controller, controller,
definition definition
}: Readonly<{ }: Readonly<{
@@ -53,6 +60,7 @@ export default function IconField({
description={definition.description} description={definition.description}
required={definition.required} required={definition.required}
error={definition.error ?? error?.message} error={definition.error ?? error?.message}
aria-label={`icon-field-${field.name}`}
ref={field.ref} ref={field.ref}
component='button' component='button'
type='button' type='button'
@@ -99,6 +107,8 @@ export default function IconField({
); );
} }
export default memo(IconField);
type RenderIconType = { type RenderIconType = {
package: string; package: string;
name: string; name: string;
@@ -1,9 +1,10 @@
import type { ApiFormFieldSet, ApiFormFieldType } from '@lib/types/Forms'; import type { ApiFormFieldSet, ApiFormFieldType } from '@lib/types/Forms';
import { Accordion, Divider, Stack, Text } from '@mantine/core'; import { Accordion, Divider, Stack, Text } from '@mantine/core';
import { memo } from 'react';
import type { Control, FieldValues } from 'react-hook-form'; import type { Control, FieldValues } from 'react-hook-form';
import { ApiFormField } from './ApiFormField'; import { ApiFormField } from './ApiFormField';
export function NestedObjectField({ function NestedObjectFieldComponent({
control, control,
fieldName, fieldName,
definition, definition,
@@ -43,3 +44,5 @@ export function NestedObjectField({
</Accordion> </Accordion>
); );
} }
export const NestedObjectField = memo(NestedObjectFieldComponent);
@@ -1,5 +1,5 @@
import { NumberInput } from '@mantine/core'; import { NumberInput } from '@mantine/core';
import { useId, useMemo } from 'react'; import { memo, useCallback, useId, useMemo } from 'react';
import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
import AutoFillRightSection, { AutoFillWarning } from './AutoFillRightSection'; import AutoFillRightSection, { AutoFillWarning } from './AutoFillRightSection';
@@ -7,7 +7,7 @@ import AutoFillRightSection, { AutoFillWarning } from './AutoFillRightSection';
* Custom implementation of the mantine <NumberInput> component, * Custom implementation of the mantine <NumberInput> component,
* used for rendering numerical input fields in forms. * used for rendering numerical input fields in forms.
*/ */
export default function NumberField({ function NumberField({
controller, controller,
fieldName, fieldName,
definition, definition,
@@ -97,6 +97,17 @@ export default function NumberField({
onChange onChange
]); ]);
const handleChange = useCallback(
(value: number | string | null) => {
if (value != null && value.toString().trim() === '') {
onChange(null);
} else {
onChange(value);
}
},
[onChange]
);
return ( return (
<NumberInput <NumberInput
{...definition} {...definition}
@@ -108,14 +119,10 @@ export default function NumberField({
value={numericalValue === null ? '' : numericalValue} value={numericalValue === null ? '' : numericalValue}
decimalScale={definition.field_type == 'integer' ? 0 : 10} decimalScale={definition.field_type == 'integer' ? 0 : 10}
step={1} step={1}
onChange={(value: number | string | null) => { onChange={handleChange}
if (value != null && value.toString().trim() === '') {
onChange(null);
} else {
onChange(value);
}
}}
rightSection={rightSection} rightSection={rightSection}
/> />
); );
} }
export default memo(NumberField);
@@ -10,6 +10,7 @@ import { useDebouncedValue, useId } from '@mantine/hooks';
import { useQuery } from '@tanstack/react-query'; import { useQuery } from '@tanstack/react-query';
import { import {
type ReactNode, type ReactNode,
memo,
useCallback, useCallback,
useEffect, useEffect,
useMemo, useMemo,
@@ -47,7 +48,7 @@ import { RenderInstance } from '../../render/Instance';
/** /**
* Render a 'select' field for searching the database against a particular model type * Render a 'select' field for searching the database against a particular model type
*/ */
export function RelatedModelField({ function RelatedModelFieldComponent({
controller, controller,
fieldName, fieldName,
definition, definition,
@@ -285,6 +286,13 @@ export function RelatedModelField({
field.value field.value
]); ]);
// Track which id we've already requested (or are currently requesting),
// so that if this effect re-fires for an unrelated reason - e.g. a new
// `definition.filters`/`definition.api_url` reference - before the
// in-flight request for the same id resolves and updates `pk`, it doesn't
// issue a duplicate API call for that same id.
const requestedIdRef = useRef<number | string | null>(null);
// If an initial value is provided, load from the API // If an initial value is provided, load from the API
useEffect(() => { useEffect(() => {
// If the value is unchanged, do nothing // If the value is unchanged, do nothing
@@ -293,8 +301,11 @@ export function RelatedModelField({
const id = pk || field.value; const id = pk || field.value;
if (id !== null && id !== undefined && id !== '') { if (id !== null && id !== undefined && id !== '') {
if (requestedIdRef.current === id) return;
requestedIdRef.current = id;
fetchSingleField(id); fetchSingleField(id);
} else { } else {
requestedIdRef.current = null;
setPk(null); setPk(null);
} }
}, [ }, [
@@ -571,6 +582,8 @@ export function RelatedModelField({
); );
} }
export const RelatedModelField = memo(RelatedModelFieldComponent);
function InlineCreateButton({ function InlineCreateButton({
definition, definition,
modelInfo, modelInfo,
@@ -26,6 +26,7 @@ import {
import { AddItemButton } from '@lib/components/AddItemButton'; import { AddItemButton } from '@lib/components/AddItemButton';
import { identifierString } from '@lib/functions/Conversion'; import { identifierString } from '@lib/functions/Conversion';
import type { ApiFormFieldType } from '@lib/types/Forms'; import type { ApiFormFieldType } from '@lib/types/Forms';
import { isEquivalent } from '../../../functions/comparison';
import { InvenTreeIcon } from '../../../functions/icons'; import { InvenTreeIcon } from '../../../functions/icons';
import { StandaloneField } from '../StandaloneField'; import { StandaloneField } from '../StandaloneField';
@@ -97,42 +98,13 @@ function TableFieldRow({
} }
// Memoize each table field row, so that we don't re-render the entire table when a single row is updated // Memoize each table field row, so that we don't re-render the entire table when a single row is updated
function areShallowEqual(previousValue: any, nextValue: any): boolean {
if (previousValue === nextValue) {
return true;
}
if (!previousValue || !nextValue) {
return false;
}
if (typeof previousValue !== 'object' || typeof nextValue !== 'object') {
return previousValue === nextValue;
}
const previousKeys = Object.keys(previousValue);
const nextKeys = Object.keys(nextValue);
if (previousKeys.length !== nextKeys.length) {
return false;
}
for (const key of previousKeys) {
if (previousValue[key] !== nextValue[key]) {
return false;
}
}
return true;
}
const MemoizedTableFieldRow = memo( const MemoizedTableFieldRow = memo(
TableFieldRow, TableFieldRow,
(previousProps, nextProps) => { (previousProps, nextProps) => {
return ( return (
previousProps.rowId === nextProps.rowId && previousProps.rowId === nextProps.rowId &&
areShallowEqual(previousProps.item, nextProps.item) && isEquivalent(previousProps.item, nextProps.item) &&
areShallowEqual(previousProps.rowErrors, nextProps.rowErrors) && isEquivalent(previousProps.rowErrors, nextProps.rowErrors) &&
previousProps.modelRenderer === nextProps.modelRenderer && previousProps.modelRenderer === nextProps.modelRenderer &&
previousProps.changeFn === nextProps.changeFn && previousProps.changeFn === nextProps.changeFn &&
previousProps.removeFn === nextProps.removeFn && previousProps.removeFn === nextProps.removeFn &&
@@ -388,8 +360,8 @@ export const TableField = memo(
return ( return (
previousProps.definition === nextProps.definition && previousProps.definition === nextProps.definition &&
previousProps.fieldName === nextProps.fieldName && previousProps.fieldName === nextProps.fieldName &&
areShallowEqual(previousProps.value, nextProps.value) && isEquivalent(previousProps.value, nextProps.value) &&
areShallowEqual(previousProps.error, nextProps.error) && isEquivalent(previousProps.error, nextProps.error) &&
previousProps.onChange === nextProps.onChange previousProps.onChange === nextProps.onChange
); );
} }
@@ -1,7 +1,7 @@
import { TagsInput } from '@mantine/core'; import { TagsInput } from '@mantine/core';
import { useDebouncedValue } from '@mantine/hooks'; import { useDebouncedValue } from '@mantine/hooks';
import { useQuery } from '@tanstack/react-query'; import { useQuery } from '@tanstack/react-query';
import { useCallback, useEffect, useMemo, useState } from 'react'; import { memo, useCallback, useEffect, useMemo, useState } from 'react';
import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
import { ApiEndpoints } from '@lib/enums/ApiEndpoints'; import { ApiEndpoints } from '@lib/enums/ApiEndpoints';
@@ -9,7 +9,7 @@ import { apiUrl } from '@lib/functions/Api';
import type { ApiFormFieldType } from '@lib/types/Forms'; import type { ApiFormFieldType } from '@lib/types/Forms';
import { api } from '../../../App'; import { api } from '../../../App';
export default function TagsField({ function TagsField({
controller, controller,
definition definition
}: Readonly<{ }: Readonly<{
@@ -79,3 +79,5 @@ export default function TagsField({
/> />
); );
} }
export default memo(TagsField);
@@ -1,5 +1,5 @@
import { TextInput } from '@mantine/core'; import { TextInput } from '@mantine/core';
import { useCallback, useEffect, useId, useMemo, useState } from 'react'; import { memo, useCallback, useEffect, useId, useMemo, useState } from 'react';
import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
import AutoFillRightSection from './AutoFillRightSection'; import AutoFillRightSection from './AutoFillRightSection';
@@ -8,7 +8,7 @@ import AutoFillRightSection from './AutoFillRightSection';
* used for rendering text input fields in forms. * used for rendering text input fields in forms.
* Uses a debounced value to prevent excessive re-renders. * Uses a debounced value to prevent excessive re-renders.
*/ */
export default function TextField({ function TextField({
controller, controller,
fieldName, fieldName,
definition, definition,
@@ -29,7 +29,7 @@ export default function TextField({
fieldState: { error } fieldState: { error }
} = controller; } = controller;
const { value } = useMemo(() => field, [field]); const { value } = field;
const [textValue, setTextValue] = useState<string>(value || ''); const [textValue, setTextValue] = useState<string>(value || '');
@@ -91,3 +91,5 @@ export default function TextField({
/> />
); );
} }
export default memo(TextField);
@@ -15,7 +15,7 @@ import {
IconX IconX
} from '@tabler/icons-react'; } from '@tabler/icons-react';
import { useQuery } from '@tanstack/react-query'; import { useQuery } from '@tanstack/react-query';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import type { FieldValues, UseControllerReturn } from 'react-hook-form';
import type { NavigateFunction } from 'react-router-dom'; import type { NavigateFunction } from 'react-router-dom';
@@ -41,7 +41,7 @@ import { ModelHoverCard } from '../../render/ModelHoverCard';
* endpoint. Supports server-side search, lazy child loading, and (when a model * endpoint. Supports server-side search, lazy child loading, and (when a model
* type is provided) barcode scanning and a hover-card navigate link. * type is provided) barcode scanning and a hover-card navigate link.
*/ */
export function TreeField({ function TreeFieldComponent({
controller, controller,
definition, definition,
fieldName, fieldName,
@@ -497,3 +497,5 @@ export function TreeField({
</Input.Wrapper> </Input.Wrapper>
); );
} }
export const TreeField = memo(TreeFieldComponent);
+55
View File
@@ -0,0 +1,55 @@
/** Generic value-comparison helpers, used to keep memoized props/state stable */
function isReactElement(value: any): boolean {
return (
value != null &&
typeof value === 'object' &&
(value.$$typeof === Symbol.for('react.element') ||
value.$$typeof === Symbol.for('react.transitional.element'))
);
}
/**
* Recursively compare two values for structural equivalence.
*
* Useful when comparing generated data (e.g. field definitions rebuilt by a
* `useMemo`, or table row data) that may get a fresh object/array/JSX-element
* reference on every recompute even though nothing meaningful changed -
* without this, every consumer downstream would be treated as "changed" and
* re-render unnecessarily.
*
* Functions are deliberately compared by reference only - unlike a JSX icon,
* a callback's behavior can depend on values it closes over, so we can't
* assume two function instances are interchangeable.
*/
export function isEquivalent(a: any, b: any): boolean {
if (Object.is(a, b)) {
return true;
}
if (isReactElement(a) && isReactElement(b)) {
return a.type === b.type && isEquivalent(a.props, b.props);
}
if (typeof a === 'function' || typeof b === 'function') {
return false;
}
if (Array.isArray(a) || Array.isArray(b)) {
if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) {
return false;
}
return a.every((item, idx) => isEquivalent(item, b[idx]));
}
if (a && b && typeof a === 'object' && typeof b === 'object') {
const aKeys = Object.keys(a);
const bKeys = Object.keys(b);
if (aKeys.length !== bKeys.length) {
return false;
}
return aKeys.every((key) => isEquivalent(a[key], b[key]));
}
return false;
}
@@ -516,6 +516,26 @@ test('Purchase Orders - Receive Items', async ({ browser }) => {
await page.getByText('Room 101').first().waitFor(); await page.getByText('Room 101').first().waitFor();
await page.getByText('Mechanical Lab').first().waitFor(); await page.getByText('Mechanical Lab').first().waitFor();
// Editing the quantity for one row should not affect any other row
// (regression test for per-row TableField memoization)
const quantityInputs = page.getByRole('textbox', {
name: 'number-field-quantity'
});
// .count() does not auto-wait, so explicitly wait for the second row's
// input to be ready before relying on the total count being stable
await expect(quantityInputs.nth(1)).toBeVisible();
const rowCount = await quantityInputs.count();
expect(rowCount).toBeGreaterThanOrEqual(2);
await quantityInputs.nth(0).fill('11');
await quantityInputs.nth(1).fill('22');
await page.waitForTimeout(250);
await expect(quantityInputs.nth(0)).toHaveValue('11');
await expect(quantityInputs.nth(1)).toHaveValue('22');
await page.getByRole('button', { name: 'Cancel' }).click(); await page.getByRole('button', { name: 'Cancel' }).click();
// Let's actually receive an item (with custom values) // Let's actually receive an item (with custom values)
+114 -1
View File
@@ -2,7 +2,7 @@ import { createApi } from './api';
/** Unit tests for form validation, rendering, etc */ /** Unit tests for form validation, rendering, etc */
import { expect, test } from './baseFixtures'; import { expect, test } from './baseFixtures';
import { stevenuser } from './defaults'; import { stevenuser } from './defaults';
import { navigate, openDetailAction } from './helpers'; import { clickOnRowMenu, loadTab, navigate, openDetailAction } from './helpers';
import { doCachedLogin } from './login'; import { doCachedLogin } from './login';
// Test hover form action in related fields // Test hover form action in related fields
@@ -202,3 +202,116 @@ test('Forms - Keep form open option', async ({ browser }) => {
await page.getByText('Item Created').waitFor(); await page.getByText('Item Created').waitFor();
await expect(page.getByRole('dialog')).toBeHidden(); await expect(page.getByRole('dialog')).toBeHidden();
}); });
test('Forms - DateTime Field', async ({ browser }) => {
// The "started_datetime" / "finished_datetime" test-result fields are only
// shown when the "TEST_STATION_DATA" global setting is enabled
const api = await createApi({});
const settingUrl = 'settings/global/TEST_STATION_DATA/';
const enableResponse = await api.patch(settingUrl, {
data: { value: 'true' }
});
expect(enableResponse.status()).toBe(200);
const page = await doCachedLogin(browser, {
user: stevenuser,
url: 'stock/item/1194/test_results'
});
await page.waitForURL('**/web/stock/**');
await loadTab(page, 'Test Results');
const cell = page.getByText('395c6d5586e5fb656901d047be27e1f7');
await clickOnRowMenu(cell);
await page.getByRole('menuitem', { name: 'Edit' }).click();
// Set a value via the Mantine DateTimePicker popup - a button which opens a
// calendar (for the date) plus separate hour/minute/second spinbuttons
const setDateTime = async (
fieldLabel: string,
day: string,
hour: string,
minute: string
) => {
const field = page.getByLabel(fieldLabel);
await expect(field).toBeVisible();
await field.click();
await page.getByRole('button', { name: day }).click();
const spinbuttons = page.getByRole('spinbutton');
await spinbuttons.nth(0).fill(hour);
await spinbuttons.nth(1).fill(minute);
await spinbuttons.nth(2).fill('00');
await page.keyboard.press('Escape');
};
await setDateTime(
'date-time-field-started_datetime',
'10 March 2024',
'10',
'30'
);
await setDateTime(
'date-time-field-finished_datetime',
'11 March 2024',
'11',
'45'
);
await expect(page.getByLabel('date-time-field-started_datetime')).toHaveText(
'2024-03-10 10:30:00'
);
await expect(page.getByLabel('date-time-field-finished_datetime')).toHaveText(
'2024-03-11 11:45:00'
);
await page.getByRole('button', { name: 'Submit' }).click();
await page.getByText('Test result updated').waitFor();
// Restore the global setting to its default value
const disableResponse = await api.patch(settingUrl, {
data: { value: 'false' }
});
expect(disableResponse.status()).toBe(200);
});
// Test the "nested object" field type, via the "Initial Supplier" section
// of the "Create Part" form
test('Forms - Nested Object Field', async ({ browser }) => {
const page = await doCachedLogin(browser, {
user: stevenuser,
url: 'part/category/index/parts'
});
await page.waitForURL('**/part/category/index/**');
await page.getByRole('button', { name: 'action-menu-add-parts' }).click();
await page
.getByRole('menuitem', { name: 'action-menu-add-parts-create-part' })
.click();
// Generate a unique part name
const partName = `Test Part ${new Date().getTime()}`;
await page.getByLabel('text-field-name', { exact: true }).fill(partName);
// The "Initial Supplier" nested-object field should be visible and expanded by default
await page.getByText('Supplier Information').waitFor();
const supplierField = page.getByLabel(
'related-field-initial_supplier.supplier'
);
await expect(supplierField).toBeVisible();
await supplierField.fill('Mouser');
await page.getByText('Mouser Electronics').first().click();
const skuValue = `SKU-${new Date().getTime()}`;
await page
.getByLabel('text-field-initial_supplier.sku', { exact: true })
.fill(skuValue);
await page.getByRole('button', { name: 'Submit' }).click();
await page.getByText('Item Created').waitFor();
// Confirm the part was created with the expected name
await page.getByText(partName).first().waitFor();
});