feat: Change behaviour of Tag Drawer on click outside

* Avoid close Tags drawer on edit * To avoid lose user data * Wrap with ContentTagsDrawerSheet to build drawer on MFE * ContentTagsDrawerSheetContext created * Not close drawer with Escape is pressed when container is blocked
open-craft · Apr 25, 2024 · 73c978b · 73c978b
1 parent 9327948
commit 73c978b
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 46 deletions.
diff --git a/src/content-tags-drawer/ContentTagsDrawer.jsx b/src/content-tags-drawer/ContentTagsDrawer.jsx
@@ -1,5 +1,5 @@
 // @ts-check
-import React, { useEffect } from 'react';
+import React, { useContext, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import {
   Container,
@@ -14,7 +14,7 @@ import messages from './messages';
 import ContentTagsCollapsible from './ContentTagsCollapsible';
 import Loading from '../generic/Loading';
 import useContentTagsDrawerContext from './ContentTagsDrawerHelper';
-import { ContentTagsDrawerContext } from './common/context';
+import { ContentTagsDrawerContext, ContentTagsDrawerSheetContext } from './common/context';
 
 /**
  * Drawer with the functionality to show and manage tags in a certain content.
@@ -32,6 +32,7 @@ const ContentTagsDrawer = ({ id, onClose }) => {
   const contentId = id ?? params.contentId;
 
   const context = useContentTagsDrawerContext(contentId);
+  const { blockingSheet } = useContext(ContentTagsDrawerSheetContext);
 
   const {
     showToastAfterSave,
@@ -64,7 +65,7 @@ const ContentTagsDrawer = ({ id, onClose }) => {
     const handleEsc = (event) => {
       /* Close drawer when ESC-key is pressed and selectable dropdown box not open */
       const selectableBoxOpen = document.querySelector('[data-selectable-box="taxonomy-tags"]');
-      if (event.key === 'Escape' && !selectableBoxOpen) {
+      if (event.key === 'Escape' && !selectableBoxOpen && !blockingSheet) {
         onCloseDrawer();
       }
     };
@@ -73,7 +74,7 @@ const ContentTagsDrawer = ({ id, onClose }) => {
     return () => {
       document.removeEventListener('keydown', handleEsc);
     };
-  }, []);
+  }, [blockingSheet]);
 
   useEffect(() => {
     /* istanbul ignore next */

diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx
@@ -19,6 +19,7 @@ import {
 } from './data/apiHooks';
 import { getTaxonomyListData } from '../taxonomy/data/api';
 import messages from './messages';
+import { ContentTagsDrawerSheetContext } from './common/context';
 
 const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@7f47fe2dbcaf47c5a071671c741fe1ab';
 const mockOnClose = jest.fn();
@@ -61,15 +62,18 @@ jest.mock('../taxonomy/data/api', () => ({
 const queryClient = new QueryClient();
 
 const RootWrapper = (params) => (
-  <IntlProvider locale="en" messages={{}}>
-    <QueryClientProvider client={queryClient}>
-      <ContentTagsDrawer {...params} />
-    </QueryClientProvider>
-  </IntlProvider>
+  <ContentTagsDrawerSheetContext.Provider value={params}>
+    <IntlProvider locale="en" messages={{}}>
+      <QueryClientProvider client={queryClient}>
+        <ContentTagsDrawer {...params} />
+      </QueryClientProvider>
+    </IntlProvider>
+  </ContentTagsDrawerSheetContext.Provider>
 );
 
 describe('<ContentTagsDrawer />', () => {
   beforeEach(async () => {
+    jest.clearAllMocks();
     await queryClient.resetQueries();
     // By default, we mock the API call with a promise that never resolves.
     // You can override this in specific test.
@@ -749,6 +753,16 @@ describe('<ContentTagsDrawer />', () => {
     postMessageSpy.mockRestore();
   });
 
+  it('should call `onClose` when Escape key is pressed and no selectable box is active', () => {
+    const { container } = render(<RootWrapper onClose={mockOnClose} />);
+
+    fireEvent.keyDown(container, {
+      key: 'Escape',
+    });
+
+    expect(mockOnClose).toHaveBeenCalled();
+  });
+
   it('should not call closeManageTagsDrawer when Escape key is pressed and a selectable box is active', () => {
     const postMessageSpy = jest.spyOn(window.parent, 'postMessage');
 
@@ -771,6 +785,46 @@ describe('<ContentTagsDrawer />', () => {
     postMessageSpy.mockRestore();
   });
 
+  it('should not call `onClose` when Escape key is pressed and a selectable box is active', () => {
+    const { container } = render(<RootWrapper onClose={mockOnClose} />);
+
+    // Simulate that the selectable box is open by adding an element with the data attribute
+    const selectableBox = document.createElement('div');
+    selectableBox.setAttribute('data-selectable-box', 'taxonomy-tags');
+    document.body.appendChild(selectableBox);
+
+    fireEvent.keyDown(container, {
+      key: 'Escape',
+    });
+
+    expect(mockOnClose).not.toHaveBeenCalled();
+
+    // Remove the added element
+    document.body.removeChild(selectableBox);
+  });
+
+  it('should not call closeManageTagsDrawer when Escape key is pressed and container is blocked', () => {
+    const postMessageSpy = jest.spyOn(window.parent, 'postMessage');
+
+    const { container } = render(<RootWrapper blockingSheet />);
+    fireEvent.keyDown(container, {
+      key: 'Escape',
+    });
+
+    expect(postMessageSpy).not.toHaveBeenCalled();
+
+    postMessageSpy.mockRestore();
+  });
+
+  it('should not call `onClose` when Escape key is pressed and container is blocked', () => {
+    const { container } = render(<RootWrapper blockingSheet onClose={mockOnClose} />);
+    fireEvent.keyDown(container, {
+      key: 'Escape',
+    });
+
+    expect(mockOnClose).not.toHaveBeenCalled();
+  });
+
   it('should call `updateTags` mutation on save', async () => {
     setupMockDataForStagedTagsTesting();
     render(<RootWrapper />);

diff --git a/src/content-tags-drawer/ContentTagsDrawerHelper.jsx b/src/content-tags-drawer/ContentTagsDrawerHelper.jsx
@@ -6,6 +6,7 @@ import { useContentData, useContentTaxonomyTagsData, useContentTaxonomyTagsUpdat
 import { useTaxonomyList } from '../taxonomy/data/apiHooks';
 import { extractOrgFromContentId } from './utils';
 import messages from './messages';
+import { ContentTagsDrawerSheetContext } from './common/context';
 
 /** @typedef {import("./data/types.mjs").Tag} ContentTagData */
 /** @typedef {import("./data/types.mjs").StagedTagData} StagedTagData */
@@ -48,6 +49,8 @@ const useContentTagsDrawerContext = (contentId) => {
   const intl = useIntl();
   const org = extractOrgFromContentId(contentId);
 
+  const { setBlockingSheet } = React.useContext(ContentTagsDrawerSheetContext);
+
   // True if the drawer is on edit mode.
   const [isEditMode, setIsEditMode] = React.useState(false);
   // This stores the tags added on the add tags Select in all taxonomies.
@@ -235,9 +238,8 @@ const useContentTagsDrawerContext = (contentId) => {
     setCollapsibleToInitalState,
   ]);
 
-  // Build toast message and show toast after save drawer.
-  /* istanbul ignore next */
-  const showToastAfterSave = React.useCallback(() => {
+  // Count added and removed tags
+  const countTags = React.useCallback(() => {
     const tagsAddedList = Object.values(globalStagedContentTags);
     const tagsRemovedList = Object.values(globalStagedRemovedContentTags);
 
@@ -249,6 +251,17 @@ const useContentTagsDrawerContext = (contentId) => {
       (acc, curr) => acc + curr.length,
       0,
     );
+    return {
+      tagsAdded,
+      tagsRemoved,
+    };
+  }, [globalStagedContentTags, globalStagedRemovedContentTags]);
+
+  // Build toast message and show toast after save drawer.
+  /* istanbul ignore next */
+  const showToastAfterSave = React.useCallback(() => {
+    const { tagsAdded, tagsRemoved } = countTags();
+
     let message;
     if (tagsAdded && tagsRemoved) {
       message = `${intl.formatMessage(
@@ -273,7 +286,7 @@ const useContentTagsDrawerContext = (contentId) => {
       );
     }
     setToastMessage(message);
-  }, [globalStagedContentTags, globalStagedRemovedContentTags, setToastMessage]);
+  }, [setToastMessage, countTags]);
 
   // Close the toast
   const closeToast = React.useCallback(() => setToastMessage(undefined), [setToastMessage]);
@@ -321,6 +334,15 @@ const useContentTagsDrawerContext = (contentId) => {
     const mergedTagsArray = fechedTaxonomies.map(obj => mergedTags[obj.id]);
 
     setTagsByTaxonomy(mergedTagsArray);
+
+    if (setBlockingSheet) {
+      const { tagsAdded, tagsRemoved } = countTags();
+      if (tagsAdded || tagsRemoved) {
+        setBlockingSheet(true);
+      } else {
+        setBlockingSheet(false);
+      }
+    }
   }, [
     fechedTaxonomies,
     globalStagedContentTags,

diff --git a/src/content-tags-drawer/ContentTagsDrawerSheet.jsx b/src/content-tags-drawer/ContentTagsDrawerSheet.jsx
@@ -0,0 +1,44 @@
+// @ts-check
+import React, { useMemo, useState } from 'react';
+import { Sheet } from '@openedx/paragon';
+import PropTypes from 'prop-types';
+import ContentTagsDrawer from './ContentTagsDrawer';
+import { ContentTagsDrawerSheetContext } from './common/context';
+
+const ContentTagsDrawerSheet = ({ id, onClose, showSheet }) => {
+  const [blockingSheet, setBlockingSheet] = useState(false);
+
+  const context = useMemo(() => ({
+    blockingSheet, setBlockingSheet,
+  }), [blockingSheet, setBlockingSheet]);
+
+  return (
+    <ContentTagsDrawerSheetContext.Provider value={context}>
+      <Sheet
+        position="right"
+        show={showSheet}
+        onClose={onClose}
+        blocking={blockingSheet}
+      >
+        <ContentTagsDrawer
+          id={id}
+          onClose={onClose}
+        />
+      </Sheet>
+    </ContentTagsDrawerSheetContext.Provider>
+  );
+};
+
+ContentTagsDrawerSheet.propTypes = {
+  id: PropTypes.string,
+  onClose: PropTypes.func,
+  showSheet: PropTypes.bool,
+};
+
+ContentTagsDrawerSheet.defaultProps = {
+  id: undefined,
+  onClose: undefined,
+  showSheet: false,
+};
+
+export default ContentTagsDrawerSheet;
diff --git a/src/content-tags-drawer/common/context.js b/src/content-tags-drawer/common/context.js
@@ -35,3 +35,8 @@ export const ContentTagsDrawerContext = React.createContext({
   closeToast: /** @type{() => void} */ (() => {}),
   setCollapsibleToInitalState: /** @type{() => void} */ (() => {}),
 });
+
+export const ContentTagsDrawerSheetContext = React.createContext({
+  blockingSheet: /** @type{boolean} */ (false),
+  setBlockingSheet: /** @type{Function} */ (() => {}),
+});
diff --git a/src/content-tags-drawer/index.js b/src/content-tags-drawer/index.js
@@ -1,2 +1,4 @@
 // eslint-disable-next-line import/prefer-default-export
 export { default as ContentTagsDrawer } from './ContentTagsDrawer';
+// eslint-disable-next-line import/prefer-default-export
+export { default as ContentTagsDrawerSheet } from './ContentTagsDrawerSheet';
diff --git a/src/content-tags-drawer/tags-sidebar-controls/TagsSidebarBody.jsx b/src/content-tags-drawer/tags-sidebar-controls/TagsSidebarBody.jsx
@@ -1,12 +1,12 @@
 // @ts-check
 import React, { useState, useMemo } from 'react';
 import {
-  Card, Stack, Button, Sheet, Collapsible, Icon,
+  Card, Stack, Button, Collapsible, Icon,
 } from '@openedx/paragon';
 import { ArrowDropDown, ArrowDropUp } from '@openedx/paragon/icons';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import { useParams } from 'react-router-dom';
-import { ContentTagsDrawer } from '..';
+import { ContentTagsDrawerSheet } from '..';
 
 import messages from '../messages';
 import { useContentTaxonomyTagsData } from '../data/apiHooks';
@@ -93,16 +93,11 @@ const TagsSidebarBody = () => {
           </Button>
         </Stack>
       </Card.Body>
-      <Sheet
-        position="right"
-        show={showManageTags}
+      <ContentTagsDrawerSheet
+        id={contentId}
         onClose={onClose}
-      >
-        <ContentTagsDrawer
-          id={contentId}
-          onClose={onClose}
-        />
-      </Sheet>
+        showSheet={showManageTags}
+      />
     </>
   );
 };

diff --git a/src/course-outline/card-header/CardHeader.jsx b/src/course-outline/card-header/CardHeader.jsx
@@ -10,7 +10,6 @@ import {
   Hyperlink,
   Icon,
   IconButton,
-  Sheet,
   useToggle,
 } from '@openedx/paragon';
 import {
@@ -19,7 +18,7 @@ import {
 } from '@openedx/paragon/icons';
 
 import { useContentTagsCount } from '../../generic/data/apiHooks';
-import { ContentTagsDrawer } from '../../content-tags-drawer';
+import { ContentTagsDrawerSheet } from '../../content-tags-drawer';
 import TagCount from '../../generic/tag-count';
 import { useEscapeClick } from '../../hooks';
 import { ITEM_BADGE_STATUS } from '../constants';
@@ -229,16 +228,11 @@ const CardHeader = ({
           </Dropdown>
         </div>
       </div>
-      <Sheet
-        position="right"
-        show={isManageTagsDrawerOpen}
+      <ContentTagsDrawerSheet
+        id={cardId}
         onClose={/* istanbul ignore next */ () => closeManageTagsDrawer()}
-      >
-        <ContentTagsDrawer
-          id={cardId}
-          onClose={/* istanbul ignore next */ () => closeManageTagsDrawer()}
-        />
-      </Sheet>
+        showSheet={isManageTagsDrawerOpen}
+      />
     </>
   );
 };

diff --git a/src/course-outline/status-bar/StatusBar.jsx b/src/course-outline/status-bar/StatusBar.jsx
@@ -4,11 +4,11 @@ import PropTypes from 'prop-types';
 import { FormattedDate, useIntl } from '@edx/frontend-platform/i18n';
 import { getConfig } from '@edx/frontend-platform/config';
 import {
-  Button, Hyperlink, Form, Sheet, Stack, useToggle,
+  Button, Hyperlink, Form, Stack, useToggle,
 } from '@openedx/paragon';
 import { AppContext } from '@edx/frontend-platform/react';
 
-import { ContentTagsDrawer } from '../../content-tags-drawer';
+import { ContentTagsDrawerSheet } from '../../content-tags-drawer';
 import TagCount from '../../generic/tag-count';
 import { useHelpUrls } from '../../help-urls/hooks';
 import { VIDEO_SHARING_OPTIONS } from '../constants';
@@ -188,16 +188,11 @@ const StatusBar = ({
 
         )}
       </Stack>
-      <Sheet
-        position="right"
-        show={isManageTagsDrawerOpen}
+      <ContentTagsDrawerSheet
+        id={courseId}
         onClose={/* istanbul ignore next */ () => closeManageTagsDrawer()}
-      >
-        <ContentTagsDrawer
-          id={courseId}
-          onClose={/* istanbul ignore next */ () => closeManageTagsDrawer()}
-        />
-      </Sheet>
+        showSheet={isManageTagsDrawerOpen}
+      />
     </>
   );
 };