From 9483bac6055c258622feca054ef917c9ac465ad0 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 18 Apr 2024 11:27:59 -0500 Subject: [PATCH] fix: Not close drawer with Scape when container is blocked --- src/content-tags-drawer/ContentTagsDrawer.jsx | 9 +-- .../ContentTagsDrawer.test.jsx | 64 +++++++++++++++++-- .../ContentTagsDrawerSheet.jsx | 2 +- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsDrawer.jsx b/src/content-tags-drawer/ContentTagsDrawer.jsx index a1d15810de..a3ee37e900 100644 --- 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 index 03de825cc6..1016f68e79 100644 --- 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) => ( - - - - - + + + + + + + ); describe('', () => { 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('', () => { postMessageSpy.mockRestore(); }); + it('should call `onClose` when Escape key is pressed and no selectable box is active', () => { + const { container } = render(); + + 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('', () => { postMessageSpy.mockRestore(); }); + it('should not call `onClose` when Escape key is pressed and a selectable box is active', () => { + const { container } = render(); + + // 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(); + 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(); + fireEvent.keyDown(container, { + key: 'Escape', + }); + + expect(mockOnClose).not.toHaveBeenCalled(); + }); + it('should call `updateTags` mutation on save', async () => { setupMockDataForStagedTagsTesting(); render(); diff --git a/src/content-tags-drawer/ContentTagsDrawerSheet.jsx b/src/content-tags-drawer/ContentTagsDrawerSheet.jsx index dd0d60bbfa..c37b7581f3 100644 --- a/src/content-tags-drawer/ContentTagsDrawerSheet.jsx +++ b/src/content-tags-drawer/ContentTagsDrawerSheet.jsx @@ -10,7 +10,7 @@ const ContentTagsDrawerSheet = ({ id, onClose, showSheet }) => { const context = useMemo(() => ({ blockingSheet, setBlockingSheet, - }), []); + }), [blockingSheet, setBlockingSheet]); return (