From 2a31434a55bd77fe87cbf86753e6e6ea72de269e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 7 Apr 2025 11:24:51 -0300 Subject: [PATCH] fix: prevent multiple submits while creating units [FC-0083] (#1776) Fixes a bug where the form is submitted multiple times when the user presses Enter on the Unit create form. The issue was that when the user hit Enter, the form was submitted without calling the button's onClick, allowing multiple calls. Also, because the onClick was not called, we had to add the isLoading property to the LoadingButton to display the status correctly. --- .../loading-button/LoadingButton.test.tsx | 20 +++++ src/generic/loading-button/index.tsx | 31 ++++++-- .../CreateCollectionModal.tsx | 77 +++++++++---------- .../create-unit/CreateUnitModal.tsx | 30 ++++---- 4 files changed, 96 insertions(+), 62 deletions(-) diff --git a/src/generic/loading-button/LoadingButton.test.tsx b/src/generic/loading-button/LoadingButton.test.tsx index dbcd5fdad..871fce6ee 100644 --- a/src/generic/loading-button/LoadingButton.test.tsx +++ b/src/generic/loading-button/LoadingButton.test.tsx @@ -72,4 +72,24 @@ describe('', () => { expect(buttonElement).not.toHaveAttribute('aria-disabled', 'true'); expect(container.getElementsByClassName('icon-spin').length).toBe(0); }); + + it('doesnt run the onClick function if the button is loading (disabled)', async () => { + let resolver: () => void; + const promise = new Promise((resolve) => { + resolver = resolve; + }); + const longFunction = jest.fn().mockReturnValue(promise); + const { getByRole } = render(RootWrapper(longFunction)); + const buttonElement = getByRole('button'); + fireEvent.click(buttonElement); + // First click should call the function + expect(longFunction).toHaveBeenCalledTimes(1); + fireEvent.click(buttonElement); + // Second click should not call the function, because the button is disabled + expect(longFunction).toHaveBeenCalledTimes(1); + await act(async () => { resolver(); }); + // After the promise is resolved, the button should be enabled + fireEvent.click(buttonElement); + expect(longFunction).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/generic/loading-button/index.tsx b/src/generic/loading-button/index.tsx index 5b9cbd4a4..89d4caf40 100644 --- a/src/generic/loading-button/index.tsx +++ b/src/generic/loading-button/index.tsx @@ -8,13 +8,12 @@ import { StatefulButton, } from '@openedx/paragon'; -interface LoadingButtonProps { +interface LoadingButtonProps extends React.ButtonHTMLAttributes { label: string; onClick?: (e: any) => (Promise | void); - disabled?: boolean; size?: string; variant?: string; - className?: string; + isLoading?: boolean; } /** @@ -26,17 +25,33 @@ const LoadingButton: React.FC = ({ disabled, size, variant, - className, + isLoading, + ...props }) => { - const [state, setState] = useState(''); - // This is used to prevent setting the isLoading state after the component has been unmounted. + // Button state depends on isLoading and disabled flags + const getState = () => { + if (isLoading) { return 'pending'; } + if (disabled) { return 'disabled'; } + return ''; + }; + const [state, setState] = useState(getState); + + useEffect(() => { + setState(getState); + }, [isLoading, disabled]); + const componentMounted = useRef(true); + // This is used to prevent setting the isLoading state after the component has been unmounted. useEffect(() => () => { componentMounted.current = false; }, []); const loadingOnClick = useCallback(async (e: any) => { + if (disabled) { + return; + } + if (!onClick) { return; } @@ -55,13 +70,13 @@ const LoadingButton: React.FC = ({ return ( ); }; diff --git a/src/library-authoring/create-collection/CreateCollectionModal.tsx b/src/library-authoring/create-collection/CreateCollectionModal.tsx index fccd59885..175b02ea8 100644 --- a/src/library-authoring/create-collection/CreateCollectionModal.tsx +++ b/src/library-authoring/create-collection/CreateCollectionModal.tsx @@ -1,7 +1,6 @@ import React from 'react'; import { ActionRow, - Button, Form, ModalDialog, } from '@openedx/paragon'; @@ -10,6 +9,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Formik } from 'formik'; import * as Yup from 'yup'; import FormikControl from '../../generic/FormikControl'; +import LoadingButton from '../../generic/loading-button'; import { useLibraryContext } from '../common/context/LibraryContext'; import messages from './messages'; import { useCreateLibraryCollection } from '../data/apiHooks'; @@ -67,55 +67,54 @@ const CreateCollectionModal = () => { onSubmit={handleCreate} > {(formikProps) => ( - <> +
- - - {intl.formatMessage(messages.createCollectionModalNameLabel)} - - )} - value={formikProps.values.title} - placeholder={intl.formatMessage(messages.createCollectionModalNamePlaceholder)} - controlClasses="pb-2" - /> - - {intl.formatMessage(messages.createCollectionModalDescriptionLabel)} - - )} - value={formikProps.values.description} - placeholder={intl.formatMessage(messages.createCollectionModalDescriptionPlaceholder)} - help={( - - {intl.formatMessage(messages.createCollectionModalDescriptionDetails)} - - )} - controlClasses="pb-2" - rows="5" - /> - + + {intl.formatMessage(messages.createCollectionModalNameLabel)} + + )} + value={formikProps.values.title} + placeholder={intl.formatMessage(messages.createCollectionModalNamePlaceholder)} + controlClasses="pb-2" + /> + + {intl.formatMessage(messages.createCollectionModalDescriptionLabel)} + + )} + value={formikProps.values.description} + placeholder={intl.formatMessage(messages.createCollectionModalDescriptionPlaceholder)} + help={( + + {intl.formatMessage(messages.createCollectionModalDescriptionDetails)} + + )} + controlClasses="pb-2" + rows="5" + />
{intl.formatMessage(messages.createCollectionModalCancel)} - + disabled={!formikProps.isValid || !formikProps.dirty} + isLoading={formikProps.isSubmitting} + label={intl.formatMessage(messages.createCollectionModalCreate)} + type="submit" + /> - + )} diff --git a/src/library-authoring/create-unit/CreateUnitModal.tsx b/src/library-authoring/create-unit/CreateUnitModal.tsx index 33f491460..30ae48546 100644 --- a/src/library-authoring/create-unit/CreateUnitModal.tsx +++ b/src/library-authoring/create-unit/CreateUnitModal.tsx @@ -68,21 +68,19 @@ const CreateUnitModal = () => { onSubmit={handleCreate} > {(formikProps) => ( - <> +
- - - {intl.formatMessage(messages.createUnitModalNameLabel)} - - )} - value={formikProps.values.displayName} - placeholder={intl.formatMessage(messages.createUnitModalNamePlaceholder)} - controlClasses="pb-2" - /> - + + {intl.formatMessage(messages.createUnitModalNameLabel)} + + )} + value={formikProps.values.displayName} + placeholder={intl.formatMessage(messages.createUnitModalNamePlaceholder)} + controlClasses="pb-2" + />
@@ -93,11 +91,13 @@ const CreateUnitModal = () => { variant="primary" onClick={formikProps.submitForm} disabled={!formikProps.isValid || !formikProps.dirty} + isLoading={formikProps.isSubmitting} label={intl.formatMessage(messages.createUnitModalCreate)} + type="submit" /> - + )}