From cc306c875f67752ae954ae19eae5c26c9a6fee7c Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 11 Dec 2024 08:32:07 +0100 Subject: [PATCH 1/4] :fire: [#4859] Remove useSynchronizeSelect hook It's obsolete now we're using react-select. The react-select component does accurately reflect the state of option selection, even if no value is selected while leaving the select blank is not allowed. --- .../variables/VariablesEditor.stories.js | 12 ++++---- ...opyConfigurationFromRegistrationBackend.js | 12 +++----- .../forms/objects_api/ObjectTypeSelect.js | 4 --- .../objects_api/ObjectTypeVersionSelect.js | 5 ---- .../admin/forms/objects_api/hooks.js | 29 ------------------- 5 files changed, 9 insertions(+), 53 deletions(-) delete mode 100644 src/openforms/js/components/admin/forms/objects_api/hooks.js diff --git a/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js b/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js index 16387356ca..37e58a80cc 100644 --- a/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js +++ b/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js @@ -711,6 +711,7 @@ export const ConfigurePrefillObjectsAPIWithCopyButton = { objectsApiGroup: 1, objecttype: '209e0341-834d-4060-bd19-a3419d19ed74', objecttypeVersion: 2, + authAttributePath: ['path', 'to', 'bsn'], variablesMapping: [ { variableKey: 'formioComponent', @@ -751,7 +752,7 @@ export const ConfigurePrefillObjectsAPIWithCopyButton = { expect(copyButton).toBeDisabled(); const copyDropdown = await modal.findByLabelText('Registratie-instellingen overnemen'); expect(copyDropdown).toBeVisible(); - await rsSelect(copyDropdown, 'Example Objects API reg.'); + await rsSelect(copyDropdown, 'Other Objects API registration with a long name'); expect(copyButton).toBeVisible(); expect(copyButton).not.toBeDisabled(); @@ -764,9 +765,7 @@ export const ConfigurePrefillObjectsAPIWithCopyButton = { const modalForm = await canvas.findByTestId('modal-form'); expect(modalForm).toBeVisible(); - const propertyDropdowns = await modal.findAllByLabelText( - 'Selecteer een attribuut uit het objecttype' - ); + await modal.findAllByLabelText('Selecteer een attribuut uit het objecttype'); // Wait until the API call to retrieve the prefillAttributes is done await modal.findByText('path > to > bsn', undefined, {timeout: 2000}); @@ -775,11 +774,10 @@ export const ConfigurePrefillObjectsAPIWithCopyButton = { () => { expect(modalForm).toHaveFormValues({ 'options.objectsApiGroup': '1', - 'options.objecttypeUuid': '2c77babf-a967-4057-9969-0200320d23f1', + 'options.objecttypeUuid': '209e0341-834d-4060-bd19-a3419d19ed74', 'options.objecttypeVersion': '2', 'options.authAttributePath': JSON.stringify(['path', 'to', 'bsn']), - 'options.variablesMapping.0.targetPath': serializeValue(['height']), - 'options.variablesMapping.1.targetPath': serializeValue(['species']), + 'options.variablesMapping.0.targetPath': serializeValue(['path', 'to.the', 'target']), }); }, {timeout: 5000} diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/CopyConfigurationFromRegistrationBackend.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/CopyConfigurationFromRegistrationBackend.js index 156a0a40a3..1fe96abead 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/CopyConfigurationFromRegistrationBackend.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/CopyConfigurationFromRegistrationBackend.js @@ -18,11 +18,7 @@ const CopyConfigurationFromRegistrationBackend = ({backends, setShowCopyButton}) const [fieldProps] = useField(name); const {value} = fieldProps; const selectedBackend = backends.find(elem => elem.key === value); - const { - ConfirmationModal: CopyConfigurationConfirmationModal, - confirmationModalProps: copyConfigurationConfirmationModalProps, - openConfirmationModal: openCopyConfigurationConfirmationModal, - } = useConfirm(); + const {ConfirmationModal, confirmationModalProps, openConfirmationModal} = useConfirm(); return ( { e.preventDefault(); - const confirmSwitch = await openCopyConfigurationConfirmationModal(); + const confirmSwitch = await openConfirmationModal(); if (confirmSwitch) { setValues(prevValues => ({ ...prevValues, @@ -93,8 +89,8 @@ const CopyConfigurationFromRegistrationBackend = ({backends, setShowCopyButton}) - { const response = await get(OBJECTS_API_OBJECTTYPES_ENDPOINT, {objects_api_group: apiGroupID}); if (!response.ok) { @@ -55,8 +53,6 @@ const ObjectTypeSelect = ({ ]); const options = choices.map(([value, label]) => ({value, label})); - useSynchronizeSelect(name, loading, choices); - const previousValue = usePrevious(value); // when a different object type is selected, ensure that the version is reset diff --git a/src/openforms/js/components/admin/forms/objects_api/ObjectTypeVersionSelect.js b/src/openforms/js/components/admin/forms/objects_api/ObjectTypeVersionSelect.js index e33e7b3c64..cd9518b0e5 100644 --- a/src/openforms/js/components/admin/forms/objects_api/ObjectTypeVersionSelect.js +++ b/src/openforms/js/components/admin/forms/objects_api/ObjectTypeVersionSelect.js @@ -1,6 +1,5 @@ import {useFormikContext} from 'formik'; import PropTypes from 'prop-types'; -import {FormattedMessage} from 'react-intl'; import useAsync from 'react-use/esm/useAsync'; import Field from 'components/admin/forms/Field'; @@ -8,8 +7,6 @@ import FormRow from 'components/admin/forms/FormRow'; import ReactSelect from 'components/admin/forms/ReactSelect'; import {get} from 'utils/fetch'; -import {useSynchronizeSelect} from './hooks'; - const getObjecttypeVersionsEndpoint = uuid => { const bits = ['/api/v2/objects-api/object-types', encodeURIComponent(uuid), 'versions']; return bits.join('/'); @@ -50,8 +47,6 @@ const ObjectTypeVersionSelect = ({ ? [] : versions.map(version => [version.version, `${version.version} (${version.status})`]); - useSynchronizeSelect(name, loading, choices); - const options = choices.map(([value, label]) => ({value, label})); return ( diff --git a/src/openforms/js/components/admin/forms/objects_api/hooks.js b/src/openforms/js/components/admin/forms/objects_api/hooks.js deleted file mode 100644 index 038cce653f..0000000000 --- a/src/openforms/js/components/admin/forms/objects_api/hooks.js +++ /dev/null @@ -1,29 +0,0 @@ -import {useFormikContext} from 'formik'; -import {useEffect} from 'react'; - -/** - * Ensure that if no valid value is set, the first possible option is selected. - * - * This synchronizes the form field state with the UI state, since selects with no - * empty/blank option display the first option as if it was selected, which isn't - * guaranteed to be the case in the form state. - */ -// synchronizing the UI state back to the form state, because a select displays the -// first possible option as if it's actually selected) -export const useSynchronizeSelect = (name, loading, choices) => { - const {getFieldProps, getFieldHelpers} = useFormikContext(); - - const {value: currentValue} = getFieldProps(name); - const {setValue} = getFieldHelpers(name); - - useEffect(() => { - // do nothing if no options have been loaded - if (loading || !choices.length) return; - // check if a valid option is selected, if this is the case -> do nothing - const isOptionPresent = choices.find(([optionValue]) => optionValue === currentValue); - if (isOptionPresent) return; - - // otherwise select the first possible option and persist that back into the state - setValue(choices[0][0]); - }); -}; From 433eb27bea5f493ea3fe8aec0ab734776fb9a34f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 11 Dec 2024 08:48:00 +0100 Subject: [PATCH 2/4] :clown_face: [#4859] Fix story mocks --- .../variables/VariablesEditor.stories.js | 21 +++++++++++++++++++ .../prefill/objects_api/ObjectsAPIFields.js | 1 + 2 files changed, 22 insertions(+) diff --git a/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js b/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js index 37e58a80cc..629bd8fe21 100644 --- a/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js +++ b/src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js @@ -192,6 +192,20 @@ export default { {targetPath: ['species'], jsonSchema: {type: 'string', description: 'Species'}}, ], }, + '209e0341-834d-4060-bd19-a3419d19ed74': { + 1: [ + { + targetPath: ['path', 'to.the', 'target'], + jsonSchema: {type: 'string', description: 'Path to the target'}, + }, + ], + 2: [ + { + targetPath: ['path', 'to.the', 'target'], + jsonSchema: {type: 'string', description: 'Path to the target'}, + }, + ], + }, }), ], objectsAPIPrefill: [ @@ -210,6 +224,13 @@ export default { namePlural: 'Persons', dataClassification: 'open', }, + { + url: 'https://objecttypen.nl/api/v1/objecttypes/209e0341-834d-4060-bd19-a3419d19ed74', + uuid: '209e0341-834d-4060-bd19-a3419d19ed74', + name: 'Other objecttype', + namePlural: 'Other objecttypes', + dataClassification: 'open', + }, ]), mockObjecttypeVersionsGet([ {version: 1, status: 'published'}, diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js index 141060eb59..24509cde62 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js @@ -67,6 +67,7 @@ const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { ]); const {values, setFieldValue, setValues} = useFormikContext(); + console.log(values); const { plugin, options: {objecttypeUuid, objecttypeVersion, objectsApiGroup}, From 7d48879bdb380614b4105a5e4822383b15f010dd Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 11 Dec 2024 09:28:49 +0100 Subject: [PATCH 3/4] :recycle: [#4859] Give form state reset control to parent component Handling the reset of dependent fields in the ObjectTypeSelect component is too naive - there's no guarantee that the fields must always be reset, for example with the copy-config-from-registration button. Clicking that button updates a bunch of form state at once, and it already manages the dependent fields. So, providing a callback to handle the reset allows us to handle the dependent fields still, while having to option to incorporate extra context. --- .../objectsapi/LegacyConfigFields.js | 10 ++ .../ObjectsApiOptionsFormFields.stories.js | 105 ++++++------------ .../objectsapi/V2ConfigFields.js | 10 ++ .../registrations/objectsapi/mocks.js | 9 +- .../prefill/objects_api/ObjectsAPIFields.js | 16 ++- .../forms/objects_api/ObjectTypeSelect.js | 56 ++++++---- .../forms/objects_api/ObjectsAPIGroup.js | 25 +++-- 7 files changed, 119 insertions(+), 112 deletions(-) diff --git a/src/openforms/js/components/admin/form_design/registrations/objectsapi/LegacyConfigFields.js b/src/openforms/js/components/admin/form_design/registrations/objectsapi/LegacyConfigFields.js index 6f37658e0e..acb7a3eb98 100644 --- a/src/openforms/js/components/admin/form_design/registrations/objectsapi/LegacyConfigFields.js +++ b/src/openforms/js/components/admin/form_design/registrations/objectsapi/LegacyConfigFields.js @@ -32,6 +32,15 @@ const onApiGroupChange = prevValues => ({ authAttributePath: undefined, }); +/** + * Callback to invoke when the Object Type changes - used to reset the dependent fields. + */ +const onObjectTypeChange = prevValues => ({ + ...prevValues, + objecttypeVersion: undefined, + authAttributePath: undefined, +}); + const LegacyConfigFields = ({apiGroupChoices}) => { const { values: { @@ -68,6 +77,7 @@ const LegacyConfigFields = ({apiGroupChoices}) => { defaultMessage="The registration result will be an object from the selected type." /> } + onObjectTypeChange={onObjectTypeChange} /> { + play: async ({canvasElement, step}) => { const canvas = within(canvasElement); - const v2Tab = canvas.getByRole('tab', {name: 'Variabelekoppelingen'}); - await userEvent.click(v2Tab); - - await waitFor(async () => { + await step('Activate v2 tab', async () => { + const v2Tab = canvas.getByRole('tab', {name: 'Variabelekoppelingen'}); + await userEvent.click(v2Tab); // Close the confirmation modal await userEvent.click( - within(await canvas.findByRole('dialog')).getByRole('button', { - name: 'Accepteren', - }) + within(await canvas.findByRole('dialog')).getByRole('button', {name: 'Accepteren'}) ); - - // Expect v2Tab to be the selected tab - expect(v2Tab).toHaveAttribute('aria-selected', 'true'); + // Wait for v2Tab to be the selected tab + await waitFor(() => { + expect(v2Tab).toHaveAttribute('aria-selected', 'true'); + }); }); const groupSelect = canvas.getByLabelText('API-groep'); await rsSelect(groupSelect, 'Objects API group 1'); - const testForm = await canvas.findByTestId('test-form'); + const objectTypeSelect = canvas.getByLabelText('Objecttype'); await waitFor(() => { - expect(testForm).toHaveFormValues({ - objecttype: '2c77babf-a967-4057-9969-0200320d23f1', - objecttypeVersion: '2', - }); + expect(objectTypeSelect).toBeVisible(); }); - expect(canvas.getByText('Tree (open)')).toBeVisible(); - expect(canvas.getByText('2 (draft)')).toBeVisible(); - - const v1Tab = canvas.getByRole('tab', {name: 'Verouderd (sjabloon)'}); - await userEvent.click(v1Tab); + await rsSelect(objectTypeSelect, 'Tree (open)'); - await waitFor(async () => { - // Close the confirmation modal - await userEvent.click( - within(await canvas.findByRole('dialog')).getByRole('button', { - name: 'Accepteren', - }) - ); - - // Expect v1Tab to be the selected tab - expect(v1Tab).toHaveAttribute('aria-selected', 'true'); + const objectTypeVersionSelect = canvas.getByLabelText('Versie'); + await waitFor(() => { + expect(objectTypeVersionSelect).toBeVisible(); }); + await rsSelect(objectTypeVersionSelect, '2 (draft)'); - await waitFor(() => { - expect(testForm).toHaveFormValues({ - objecttype: '2c77babf-a967-4057-9969-0200320d23f1', - objecttypeVersion: '2', - }); + const testForm = await canvas.findByTestId('test-form'); + expect(testForm).toHaveFormValues({ + objecttype: '2c77babf-a967-4057-9969-0200320d23f1', + objecttypeVersion: '2', }); }, }; @@ -206,53 +191,27 @@ export const SwitchToV2NonExisting = { objecttypeVersion: 1, }, }, - play: async ({canvasElement}) => { + play: async ({canvasElement, step}) => { const canvas = within(canvasElement); - const v2Tab = canvas.getByRole('tab', {name: 'Variabelekoppelingen'}); - await userEvent.click(v2Tab); - - await waitFor(async () => { + await step('Activate v2 tab', async () => { + const v2Tab = canvas.getByRole('tab', {name: 'Variabelekoppelingen'}); + await userEvent.click(v2Tab); // Close the confirmation modal await userEvent.click( - within(await canvas.findByRole('dialog')).getByRole('button', { - name: 'Accepteren', - }) + within(await canvas.findByRole('dialog')).getByRole('button', {name: 'Accepteren'}) ); - - // Expect v2Tab to be the selected tab - expect(v2Tab).toHaveAttribute('aria-selected', 'true'); - }); - - const testForm = await canvas.findByTestId('test-form'); - await waitFor(() => { - expect(testForm).toHaveFormValues({ - objecttype: '2c77babf-a967-4057-9969-0200320d23f1', - objecttypeVersion: '2', + // Wait for v2Tab to be the selected tab + await waitFor(() => { + expect(v2Tab).toHaveAttribute('aria-selected', 'true'); }); }); - expect(canvas.getByText('Tree (open)')).toBeVisible(); - expect(canvas.getByText('2 (draft)')).toBeVisible(); - - const v1Tab = canvas.getByRole('tab', {name: 'Verouderd (sjabloon)'}); - await userEvent.click(v1Tab); - - await waitFor(async () => { - // Close the confirmation modal - await userEvent.click( - within(await canvas.findByRole('dialog')).getByRole('button', { - name: 'Accepteren', - }) - ); - - // Expect v2Tab to be the selected tab - expect(v1Tab).toHaveAttribute('aria-selected', 'true'); - }); + const testForm = await canvas.findByTestId('test-form'); await waitFor(() => { expect(testForm).toHaveFormValues({ - objecttype: '2c77babf-a967-4057-9969-0200320d23f1', - objecttypeVersion: '2', + objecttype: '', + objecttypeVersion: '', }); }); }, diff --git a/src/openforms/js/components/admin/form_design/registrations/objectsapi/V2ConfigFields.js b/src/openforms/js/components/admin/form_design/registrations/objectsapi/V2ConfigFields.js index a3f2e2dac1..49f55bf670 100644 --- a/src/openforms/js/components/admin/form_design/registrations/objectsapi/V2ConfigFields.js +++ b/src/openforms/js/components/admin/form_design/registrations/objectsapi/V2ConfigFields.js @@ -31,6 +31,15 @@ const onApiGroupChange = prevValues => ({ variablesMapping: [], }); +/** + * Callback to invoke when the Object Type changes - used to reset the dependent fields. + */ +const onObjectTypeChange = prevValues => ({ + ...prevValues, + objecttypeVersion: undefined, + authAttributePath: undefined, +}); + const V2ConfigFields = ({apiGroupChoices}) => { const { values: { @@ -96,6 +105,7 @@ const V2ConfigFields = ({apiGroupChoices}) => { setFieldValue('variablesMapping', []); return true; }} + onObjectTypeChange={onObjectTypeChange} /> diff --git a/src/openforms/js/components/admin/form_design/registrations/objectsapi/mocks.js b/src/openforms/js/components/admin/form_design/registrations/objectsapi/mocks.js index 0ff842274f..5279ccfe63 100644 --- a/src/openforms/js/components/admin/form_design/registrations/objectsapi/mocks.js +++ b/src/openforms/js/components/admin/form_design/registrations/objectsapi/mocks.js @@ -6,9 +6,12 @@ export const mockObjecttypesGet = objecttypes => http.get(`${API_BASE_URL}/api/v2/objects-api/object-types`, () => HttpResponse.json(objecttypes)); export const mockObjecttypeVersionsGet = versions => - http.get(`${API_BASE_URL}/api/v2/objects-api/object-types/:uuid/versions`, () => - HttpResponse.json(versions) - ); + http.get(`${API_BASE_URL}/api/v2/objects-api/object-types/:uuid/versions`, ({params}) => { + if (params.uuid === 'a-non-existing-uuid') { + return HttpResponse.json([]); + } + return HttpResponse.json(versions); + }); export const mockObjecttypesError = () => http.all(`${API_BASE_URL}/api/v2/*`, () => diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js index 24509cde62..f634d1e27a 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js @@ -45,6 +45,19 @@ const onApiGroupChange = prevValues => ({ }, }); +/** + * Callback to invoke when the Object Type changes - used to reset the dependent fields. + */ +const onObjectTypeChange = prevValues => ({ + ...prevValues, + options: { + ...prevValues.options, + objecttypeVersion: undefined, + authAttributePath: undefined, + variablesMapping: [], + }, +}); + // Load the possible prefill properties // XXX: this would benefit from client-side caching const getProperties = async (objectsApiGroup, objecttypeUuid, objecttypeVersion) => { @@ -67,7 +80,6 @@ const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { ]); const {values, setFieldValue, setValues} = useFormikContext(); - console.log(values); const { plugin, options: {objecttypeUuid, objecttypeVersion, objectsApiGroup}, @@ -168,7 +180,6 @@ const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { { })); return true; }} + onObjectTypeChange={onObjectTypeChange} /> diff --git a/src/openforms/js/components/admin/forms/objects_api/ObjectTypeSelect.js b/src/openforms/js/components/admin/forms/objects_api/ObjectTypeSelect.js index fc33aa3e0a..1d3e4c03c6 100644 --- a/src/openforms/js/components/admin/forms/objects_api/ObjectTypeSelect.js +++ b/src/openforms/js/components/admin/forms/objects_api/ObjectTypeSelect.js @@ -1,6 +1,6 @@ import {useField, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; -import {usePrevious, useUpdateEffect} from 'react-use'; +import {flushSync} from 'react-dom'; import useAsync from 'react-use/esm/useAsync'; import {OBJECTS_API_OBJECTTYPES_ENDPOINT} from 'components/admin/form_design/constants'; @@ -23,16 +23,11 @@ const ObjectTypeSelect = ({ onChangeCheck, label, helpText, - versionFieldName = 'objecttypeVersion', + onObjectTypeChange = undefined, }) => { - const [fieldProps, , fieldHelpers] = useField(name); - const { - setFieldValue, - getFieldProps, - initialValues: {objecttype: initialObjecttype}, - } = useFormikContext(); + const [{value}, , fieldHelpers] = useField(name); + const {values, getFieldProps, setValues} = useFormikContext(); const objectsApiGroup = getFieldProps(apiGroupFieldName).value ?? null; - const {value} = fieldProps; const {setValue} = fieldHelpers; const { @@ -53,15 +48,6 @@ const ObjectTypeSelect = ({ ]); const options = choices.map(([value, label]) => ({value, label})); - const previousValue = usePrevious(value); - - // when a different object type is selected, ensure that the version is reset - useUpdateEffect(() => { - if (loading) return; - if (value === initialObjecttype || value === previousValue) return; - setFieldValue(versionFieldName, undefined); // clears the value - }, [loading, value]); - return ( @@ -72,8 +58,20 @@ const ObjectTypeSelect = ({ isDisabled={!objectsApiGroup} required onChange={async selectedOption => { + const hasChanged = selectedOption.value !== value; + // don't trigger Formik state changes and dependent effects if the user + // selects the same value that's already selected. + if (!hasChanged) return; + const okToProceed = onChangeCheck === undefined || (await onChangeCheck()); - if (okToProceed) setValue(selectedOption.value); + if (okToProceed) { + // flush sync needed to ensure that onApiGroupChange gets the updated + // state + flushSync(() => { + setValue(selectedOption.value); + }); + if (onObjectTypeChange) setValues(onObjectTypeChange); + } }} /> @@ -99,16 +97,26 @@ ObjectTypeSelect.propTypes = { * The help text to explain what the field is for */ helpText: PropTypes.node.isRequired, + /** + * Callback to invoke when the object type value changes, e.g. to reset any dependent + * fields. + * + * The function will be called with Formik's previous values so you can construct a new + * values state from that. + * + * **NOTE** + * + * It's best to define this callback at the module level, or make use of `useCallback` + * to obtain a stable reference to the callback, otherwise the callback will likely + * fire unexpectedly during re-renders. + */ + onObjectTypeChange: PropTypes.func, + /** * Name of the field holding the selected API group. The value is used in the API * call to get the available object types. */ apiGroupFieldName: PropTypes.string, - /** - * Name of the field to select the object type version. When the selected object type - * changes, the version will be reset/unset. - */ - versionFieldName: PropTypes.string, }; export default ObjectTypeSelect; diff --git a/src/openforms/js/components/admin/forms/objects_api/ObjectsAPIGroup.js b/src/openforms/js/components/admin/forms/objects_api/ObjectsAPIGroup.js index ca00a8be71..91aad9c8dd 100644 --- a/src/openforms/js/components/admin/forms/objects_api/ObjectsAPIGroup.js +++ b/src/openforms/js/components/admin/forms/objects_api/ObjectsAPIGroup.js @@ -1,7 +1,7 @@ import {useField, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; +import {flushSync} from 'react-dom'; import {FormattedMessage} from 'react-intl'; -import {useUpdateEffect} from 'react-use'; import Field from 'components/admin/forms/Field'; import FormRow from 'components/admin/forms/FormRow'; @@ -19,12 +19,6 @@ const ObjectsAPIGroup = ({ const {setValues} = useFormikContext(); const {value} = fieldProps; - // Call `onApiGroupChange` to get the 'reset' values whenever the API group changes. - useUpdateEffect(() => { - if (!onApiGroupChange) return; - setValues(onApiGroupChange); - }, [setValues, onApiGroupChange, value]); - const options = apiGroupChoices.map(([value, label]) => ({value, label})); // React doesn't like null/undefined as it leads to uncontrolled component warnings, @@ -60,11 +54,22 @@ const ObjectsAPIGroup = ({ value={normalizedOptions.find(option => option.value === normalizedValue)} required={required} onChange={async selectedOption => { + // Normalize empty string values back to null + const newValue = selectedOption ? selectedOption.value : null; + const hasChanged = newValue !== value; + // don't trigger Formik state changes and dependent effects if the user + // selects the same value that's already selected. + if (!hasChanged) return; + const okToProceed = onChangeCheck === undefined || (await onChangeCheck()); if (okToProceed) { - // normalize empty string back to null - const newValue = selectedOption ? selectedOption.value : null; - setValue(newValue); + // flush sync needed to ensure that onApiGroupChange gets the updated + // state + flushSync(() => { + setValue(newValue); + }); + // Call `onApiGroupChange` to get the 'reset' values whenever the API group changes. + if (onApiGroupChange) setValues(onApiGroupChange); } }} isClearable={isClearable} From 7291cfa4f98363479322d5dfd82e2acd0aa21101 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 11 Dec 2024 12:34:46 +0100 Subject: [PATCH 4/4] :recycle: [#4859] Display copy toggle only if there are relevant backends The toggle is not displayed if there are no backends to copy from, so the UI can be de-cluttered. Additionally, the mechanism/slot for the extra controls has been refactored to be more generic - anything can now be rendered there that may be relevant, and the state is specific to the UI being rendered so that has been removed from the generic parent component. We use the formik status to track form-specific state, through a small hook that normalizes the shape of the status (since it can be anything). --- .../prefill/PrefillConfigurationForm.js | 31 +++++-------------- .../variables/prefill/constants.js | 10 ++++-- ...opyConfigurationFromRegistrationBackend.js | 6 ++-- .../prefill/objects_api/ObjectsAPIFields.js | 18 +++++------ .../prefill/objects_api/ToggleCopyButton.js | 23 ++++++++++++-- .../prefill/objects_api/useStatus.js | 22 +++++++++++++ 6 files changed, 69 insertions(+), 41 deletions(-) create mode 100644 src/openforms/js/components/admin/form_design/variables/prefill/objects_api/useStatus.js diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js b/src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js index 1fe974a474..2f33e284be 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/PrefillConfigurationForm.js @@ -1,6 +1,5 @@ import {Formik} from 'formik'; import PropTypes from 'prop-types'; -import {useState} from 'react'; import {FormattedMessage} from 'react-intl'; import {SubmitAction} from 'components/admin/forms/ActionButton'; @@ -51,19 +50,10 @@ const PrefillConfigurationForm = ({ }} > {({handleSubmit, values}) => { - const PluginFormComponent = - PLUGIN_COMPONENT_MAPPING[values.plugin]?.component ?? - PLUGIN_COMPONENT_MAPPING.default.component; - const ToggleCopyComponent = - PLUGIN_COMPONENT_MAPPING[values.plugin]?.toggleCopyComponent ?? - PLUGIN_COMPONENT_MAPPING.default.toggleCopyComponent; - - const [showCopyButton, setShowCopyButton] = useState(false); - - const handleToggle = event => { - event.preventDefault(); - setShowCopyButton(!showCopyButton); - }; + const pluginConfiguration = + PLUGIN_COMPONENT_MAPPING[values.plugin] ?? PLUGIN_COMPONENT_MAPPING.default; + const {component: PluginFormComponent, pluginFieldExtra: PluginFieldExtra = null} = + pluginConfiguration; return ( <> @@ -80,22 +70,17 @@ const PrefillConfigurationForm = ({ > <> - {ToggleCopyComponent ? ( + {PluginFieldExtra && (
- +
- ) : null} + )} - + { +const CopyConfigurationFromRegistrationBackend = ({backends, onCopyDone}) => { const name = 'copyConfigurationFromBackend'; const {setFieldValue, setValues} = useFormikContext(); const options = backends.map(elem => ({value: elem.key, label: elem.name})); @@ -67,9 +67,7 @@ const CopyConfigurationFromRegistrationBackend = ({backends, setShowCopyButton}) variablesMapping: selectedBackend.options.variablesMapping, }, })); - - // Collapse the registration backend selection row - setShowCopyButton(false); + onCopyDone(); } }} disabled={!selectedBackend} diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js index f634d1e27a..6be8dbbd0f 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js @@ -4,7 +4,6 @@ * Most other plugins can be configured with the generic form in `./DefaultFields`. */ import {useFormikContext} from 'formik'; -import PropTypes from 'prop-types'; import {useContext, useEffect} from 'react'; import {FormattedMessage, useIntl} from 'react-intl'; import useAsync from 'react-use/esm/useAsync'; @@ -15,6 +14,7 @@ import Fieldset from 'components/admin/forms/Fieldset'; import FormRow from 'components/admin/forms/FormRow'; import {LOADING_OPTION} from 'components/admin/forms/Select'; import {ValidationErrorContext} from 'components/admin/forms/ValidationErrors'; +import ValidationErrorsProvider from 'components/admin/forms/ValidationErrors'; import VariableMapping from 'components/admin/forms/VariableMapping'; import { AuthAttributePath, @@ -26,8 +26,8 @@ import {FAIcon} from 'components/admin/icons'; import ErrorBoundary from 'components/errors/ErrorBoundary'; import {get} from 'utils/fetch'; -import ValidationErrorsProvider from '../../../../forms/ValidationErrors'; import CopyConfigurationFromRegistrationBackend from './CopyConfigurationFromRegistrationBackend'; +import useStatus from './useStatus'; const PLUGIN_ID = 'objects_api'; @@ -69,7 +69,7 @@ const getProperties = async (objectsApiGroup, objecttypeUuid, objecttypeVersion) return response.data.map(property => [property.targetPath, property.targetPath.join(' > ')]); }; -const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { +const ObjectsAPIFields = () => { const intl = useIntl(); // Object with keys the plugin/attribute/options, we process these further to set up // the required context for the fields. @@ -84,6 +84,7 @@ const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { plugin, options: {objecttypeUuid, objecttypeVersion, objectsApiGroup}, } = values; + const {showCopyButton, toggleShowCopyButton} = useStatus(); const defaults = { objectsApiGroup: null, @@ -138,12 +139,12 @@ const ObjectsAPIFields = ({showCopyButton, setShowCopyButton}) => { return ( - {showCopyButton ? ( + {showCopyButton && ( - ) : null} + )}
{ ); }; -ObjectsAPIFields.propTypes = { - showCopyButton: PropTypes.bool.isRequired, - setShowCopyButton: PropTypes.func.isRequired, -}; +ObjectsAPIFields.propTypes = {}; export default ObjectsAPIFields; diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ToggleCopyButton.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ToggleCopyButton.js index 998fa8e67d..d3123b0047 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ToggleCopyButton.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ToggleCopyButton.js @@ -1,8 +1,25 @@ +import {useContext} from 'react'; import {FormattedMessage} from 'react-intl'; -const ToggleCopyButton = ({handleToggle}) => { +import {FormContext} from 'components/admin/form_design/Context'; + +import useStatus from './useStatus'; + +const ToggleCopyButton = () => { + const {registrationBackends = []} = useContext(FormContext); + const {toggleShowCopyButton} = useStatus(); + const backends = registrationBackends.filter(elem => elem.backend === 'objects_api'); + // don't render a toggle if there's nothing to copy from + if (!backends.length) return null; + return ( - + { + event.preventDefault(); + toggleShowCopyButton(); + }} + > { ); }; +ToggleCopyButton.propTypes = {}; + export default ToggleCopyButton; diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/useStatus.js b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/useStatus.js new file mode 100644 index 0000000000..cdbb1a6ff7 --- /dev/null +++ b/src/openforms/js/components/admin/form_design/variables/prefill/objects_api/useStatus.js @@ -0,0 +1,22 @@ +import {useFormikContext} from 'formik'; + +/** + * Convenience hook that wraps around Formik's status. + * + * This centralizes the shape of the status tracked in the Formik state. We use status + * to track some configuration/state that affects the configuration modal without it + * directly being a form field or requiring to be managed at a higher level. + */ +const useStatus = () => { + const {status = {}, setStatus} = useFormikContext(); + const {showCopyButton = false} = status; + const toggleShowCopyButton = () => { + setStatus({...status, showCopyButton: !showCopyButton}); + }; + return { + showCopyButton, + toggleShowCopyButton, + }; +}; + +export default useStatus;