feat: handle unsaved changes in text & problem editors (openedx#1444)

The text & problem xblock editors will display a confirmation box before cancelling only if user has changed something else it will directly go back. (cherry picked from commit df8a65d)
open-craft · Nov 5, 2024 · a3f7431 · a3f7431
1 parent f10ad9f
commit a3f7431
Show file tree

Hide file tree

Showing 23 changed files with 352 additions and 75 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -93,8 +93,8 @@
     "react-onclickoutside": "^6.13.0",
     "react-redux": "7.2.9",
     "react-responsive": "9.0.2",
-    "react-router": "6.23.1",
-    "react-router-dom": "6.23.1",
+    "react-router": "6.27.0",
+    "react-router-dom": "6.27.0",
     "react-select": "5.8.0",
     "react-textarea-autosize": "^8.5.3",
     "react-transition-group": "4.4.5",

diff --git a/src/course-unit/course-xblock/CourseXBlock.jsx b/src/course-unit/course-xblock/CourseXBlock.jsx
@@ -7,7 +7,7 @@ import {
 } from '@openedx/paragon';
 import { EditOutline as EditIcon, MoreVert as MoveVertIcon } from '@openedx/paragon/icons';
 import { useIntl } from '@edx/frontend-platform/i18n';
-import { useNavigate, useSearchParams } from 'react-router-dom';
+import { useSearchParams } from 'react-router-dom';
 
 import { getCanEdit, getCourseId } from 'CourseAuthoring/course-unit/data/selectors';
 import DeleteModal from '../../generic/delete-modal/DeleteModal';
@@ -19,6 +19,7 @@ import { copyToClipboard } from '../../generic/data/thunks';
 import { COMPONENT_TYPES } from '../../generic/block-type-utils/constants';
 import XBlockMessages from './xblock-messages/XBlockMessages';
 import messages from './messages';
+import { createCorrectInternalRoute } from '../../utils';
 
 const CourseXBlock = ({
   id, title, type, unitXBlockActions, shouldScroll, userPartitionInfo,
@@ -28,7 +29,6 @@ const CourseXBlock = ({
   const [isDeleteModalOpen, openDeleteModal, closeDeleteModal] = useToggle(false);
   const [isConfigureModalOpen, openConfigureModal, closeConfigureModal] = useToggle(false);
   const dispatch = useDispatch();
-  const navigate = useNavigate();
   const canEdit = useSelector(getCanEdit);
   const courseId = useSelector(getCourseId);
   const intl = useIntl();
@@ -58,7 +58,11 @@ const CourseXBlock = ({
       case COMPONENT_TYPES.html:
       case COMPONENT_TYPES.problem:
       case COMPONENT_TYPES.video:
-        navigate(`/course/${courseId}/editor/${type}/${id}`);
+        // Not using useNavigate from react router to use browser navigation
+        // which allows us to block back button if unsaved changes in editor are present.
+        window.location.assign(
+          createCorrectInternalRoute(`/course/${courseId}/editor/${type}/${id}`),
+        );
         break;
       default:
     }

diff --git a/src/course-unit/course-xblock/CourseXBlock.test.jsx b/src/course-unit/course-xblock/CourseXBlock.test.jsx
@@ -29,7 +29,6 @@ const blockId = '567890';
 const handleDeleteMock = jest.fn();
 const handleDuplicateMock = jest.fn();
 const handleConfigureSubmitMock = jest.fn();
-const mockedUsedNavigate = jest.fn();
 const {
   name,
   block_id: id,
@@ -42,11 +41,6 @@ const unitXBlockActionsMock = {
   handleDuplicate: handleDuplicateMock,
 };
 
-jest.mock('react-router-dom', () => ({
-  ...jest.requireActual('react-router-dom'),
-  useNavigate: () => mockedUsedNavigate,
-}));
-
 jest.mock('react-redux', () => ({
   ...jest.requireActual('react-redux'),
   useSelector: jest.fn(),
@@ -78,6 +72,16 @@ useSelector.mockImplementation((selector) => {
 });
 
 describe('<CourseXBlock />', () => {
+  const locationTemp = window.location;
+  beforeAll(() => {
+    delete window.location;
+    window.location = {
+      assign: jest.fn(),
+    };
+  });
+  afterAll(() => {
+    window.location = locationTemp;
+  });
   beforeEach(async () => {
     initializeMockApp({
       authenticatedUser: {
@@ -168,8 +172,8 @@ describe('<CourseXBlock />', () => {
       expect(editButton).toBeInTheDocument();
 
       userEvent.click(editButton);
-      expect(mockedUsedNavigate).toHaveBeenCalled();
-      expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/html/${id}`);
+      expect(window.location.assign).toHaveBeenCalled();
+      expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/html/${id}`);
     });
 
     it('navigates to editor page on edit Video xblock', () => {
@@ -182,8 +186,8 @@ describe('<CourseXBlock />', () => {
       expect(editButton).toBeInTheDocument();
 
       userEvent.click(editButton);
-      expect(mockedUsedNavigate).toHaveBeenCalled();
-      expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/video/${id}`);
+      expect(window.location.assign).toHaveBeenCalled();
+      expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/video/${id}`);
     });
 
     it('navigates to editor page on edit Problem xblock', () => {
@@ -196,8 +200,8 @@ describe('<CourseXBlock />', () => {
       expect(editButton).toBeInTheDocument();
 
       userEvent.click(editButton);
-      expect(mockedUsedNavigate).toHaveBeenCalled();
-      expect(mockedUsedNavigate).toHaveBeenCalledWith(`/course/${courseId}/editor/problem/${id}`);
+      expect(window.location.assign).toHaveBeenCalled();
+      expect(window.location.assign).toHaveBeenCalledWith(`/course/${courseId}/editor/problem/${id}`);
       expect(handleDeleteMock).toHaveBeenCalledWith(id);
     });
   });

diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx
@@ -29,6 +29,12 @@ jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({
   },
 }));
 
+const isDirtyMock = jest.fn();
+jest.mock('../TextEditor/hooks', () => ({
+  ...jest.requireActual('../TextEditor/hooks'),
+  isDirty: () => isDirtyMock,
+}));
+
 const defaultPropsHtml = {
   blockId: 'block-v1:Org+TS100+24+type@html+block@123456html',
   blockType: 'html',
@@ -45,15 +51,27 @@ const fieldsHtml = {
 };
 
 describe('EditorContainer', () => {
+  let mockEvent: Event;
+
   beforeEach(() => {
     initializeMocks();
+    mockEvent = new Event('beforeunload');
+    jest.spyOn(window, 'addEventListener');
+    jest.spyOn(window, 'removeEventListener');
+    jest.spyOn(mockEvent, 'preventDefault');
+    Object.defineProperty(mockEvent, 'returnValue', { writable: true });
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
   });
 
-  test('it displays a confirmation dialog when closing the editor modal', async () => {
+  test('it displays a confirmation dialog when closing the editor modal if data is changed', async () => {
     jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => (
       { status: 200, data: snakeCaseObject(fieldsHtml) }
     ));
 
+    isDirtyMock.mockReturnValue(true);
     render(<EditorPage {...defaultPropsHtml} />);
 
     // Then the editor should open
@@ -68,12 +86,48 @@ describe('EditorContainer', () => {
     fireEvent.click(closeButton);
     // Now we should see the confirmation message:
     expect(await screen.findByText(confirmMessage)).toBeInTheDocument();
+    expect(defaultPropsHtml.onClose).not.toHaveBeenCalled();
 
+    // Should close modal if cancelled
+    const cancelBtn = await screen.findByRole('button', { name: 'Cancel' });
+    fireEvent.click(cancelBtn);
     expect(defaultPropsHtml.onClose).not.toHaveBeenCalled();
+
+    // open modal again
+    fireEvent.click(closeButton);
     // And can confirm the cancelation:
     const confirmButton = await screen.findByRole('button', { name: 'OK' });
     fireEvent.click(confirmButton);
     expect(defaultPropsHtml.onClose).toHaveBeenCalled();
+    window.dispatchEvent(mockEvent);
+    // should not be blocked by beforeunload event as the page was unloaded using close/cancel option
+    expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
+    expect(mockEvent.preventDefault).not.toHaveBeenCalled();
+  });
+
+  test('it does not display any confirmation dialog when closing the editor modal if data is not changed', async () => {
+    jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => (
+      { status: 200, data: snakeCaseObject(fieldsHtml) }
+    ));
+
+    isDirtyMock.mockReturnValue(false);
+    render(<EditorPage {...defaultPropsHtml} />);
+
+    // Then the editor should open
+    expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument();
+
+    // Assert the "are you sure?" message isn't visible yet
+    const confirmMessage = /Are you sure you want to exit the editor/;
+    expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument();
+
+    // Find and click the close button
+    const closeButton = await screen.findByRole('button', { name: 'Exit the editor' });
+    fireEvent.click(closeButton);
+    // Even now we should not see the confirmation message as data is not dirty, i.e. not changed:
+    expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument();
+
+    // And onClose is directly called
+    expect(defaultPropsHtml.onClose).toHaveBeenCalled();
   });
 
   test('it disables the save button until the fields have been loaded', async () => {
@@ -94,4 +148,21 @@ describe('EditorContainer', () => {
     // Now the save button should be active:
     await waitFor(() => expect(saveButton).not.toBeDisabled());
   });
+
+  test('beforeunload event is triggered on page unload if data is changed', async () => {
+    jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => (
+      { status: 200, data: snakeCaseObject(fieldsHtml) }
+    ));
+
+    isDirtyMock.mockReturnValue(true);
+    render(<EditorPage {...defaultPropsHtml} />);
+
+    // Then the editor should open
+    expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument();
+    // on beforeunload event block user
+    window.dispatchEvent(mockEvent);
+    expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
+    expect(mockEvent.preventDefault).toHaveBeenCalled();
+    expect(mockEvent.returnValue).toBe(true);
+  });
 });
diff --git a/src/editors/containers/EditorContainer/index.tsx b/src/editors/containers/EditorContainer/index.tsx
@@ -20,6 +20,7 @@ import TitleHeader from './components/TitleHeader';
 import * as hooks from './hooks';
 import messages from './messages';
 import './index.scss';
+import usePromptIfDirty from '../../../generic/promptIfDirty/usePromptIfDirty';
 
 interface WrapperProps {
   children: React.ReactNode;
@@ -61,32 +62,57 @@ export const FooterWrapper: React.FC<WrapperProps> = ({ children }) => {
 interface Props extends EditorComponent {
   children: React.ReactNode;
   getContent: Function;
+  isDirty: () => boolean;
   validateEntry?: Function | null;
 }
 
 const EditorContainer: React.FC<Props> = ({
   children,
   getContent,
+  isDirty,
   onClose = null,
   validateEntry = null,
   returnFunction = null,
 }) => {
   const intl = useIntl();
   const dispatch = useDispatch();
+  // Required to mark data as not dirty on save
+  const [saved, setSaved] = React.useState(false);
   const isInitialized = hooks.isInitialized();
   const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle();
   const handleCancel = hooks.handleCancel({ onClose, returnFunction });
   const disableSave = !isInitialized;
   const saveFailed = hooks.saveFailed();
   const clearSaveFailed = hooks.clearSaveError({ dispatch });
-  const onSave = hooks.handleSaveClicked({
+  const handleSave = hooks.handleSaveClicked({
     dispatch,
     getContent,
     validateEntry,
     returnFunction,
   });
+
+  const onSave = () => {
+    setSaved(true);
+    handleSave();
+  };
+  // Stops user from navigating away if they have unsaved changes.
+  usePromptIfDirty(() => {
+    // Do not block if cancel modal is used or data is saved.
+    if (isCancelConfirmOpen || saved) {
+      return false;
+    }
+    return isDirty();
+  });
+
+  const confirmCancelIfDirty = () => {
+    if (isDirty()) {
+      openCancelConfirmModal();
+    } else {
+      handleCancel();
+    }
+  };
   return (
-    <EditorModalWrapper onClose={openCancelConfirmModal}>
+    <EditorModalWrapper onClose={confirmCancelIfDirty}>
       {saveFailed && (
         <Toast show onClose={clearSaveFailed}>
           <FormattedMessage {...messages.contentSaveFailed} />
@@ -108,7 +134,9 @@ const EditorContainer: React.FC<Props> = ({
           </Button>
         )}
         isOpen={isCancelConfirmOpen}
-        close={closeCancelConfirmModal}
+        close={() => {
+          closeCancelConfirmModal();
+        }}
         title={intl.formatMessage(messages.cancelConfirmTitle)}
       >
         <FormattedMessage {...messages.cancelConfirmDescription} />
@@ -121,7 +149,7 @@ const EditorContainer: React.FC<Props> = ({
           <IconButton
             src={Close}
             iconAs={Icon}
-            onClick={openCancelConfirmModal}
+            onClick={confirmCancelIfDirty}
             alt={intl.formatMessage(messages.exitButtonAlt)}
           />
         </div>
@@ -135,7 +163,7 @@ const EditorContainer: React.FC<Props> = ({
             <Button
               aria-label={intl.formatMessage(messages.cancelButtonAriaLabel)}
               variant="tertiary"
-              onClick={openCancelConfirmModal}
+              onClick={confirmCancelIfDirty}
             >
               <FormattedMessage {...messages.cancelButtonLabel} />
             </Button>

diff --git a/...ors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap b/...ors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap
@@ -3,6 +3,7 @@
 exports[`EditorProblemView component renders raw editor 1`] = `
 <EditorContainer
   getContent={[Function]}
+  isDirty={[Function]}
   returnFunction={null}
 >
   <AlertModal
@@ -72,6 +73,7 @@ exports[`EditorProblemView component renders raw editor 1`] = `
 exports[`EditorProblemView component renders simple view 1`] = `
 <EditorContainer
   getContent={[Function]}
+  isDirty={[Function]}
   returnFunction={null}
 >
   <AlertModal