feat: prevent losing work when users click outside tag drawer after making edits [FC-0036]
This commit is contained in:
@@ -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 */
|
||||
|
||||
@@ -19,10 +19,12 @@ 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();
|
||||
const mockMutate = jest.fn();
|
||||
const mockSetBlockingSheet = jest.fn();
|
||||
|
||||
jest.mock('react-router-dom', () => ({
|
||||
...jest.requireActual('react-router-dom'),
|
||||
@@ -61,15 +63,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 +754,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 +786,98 @@ 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 `setBlockingSheet` on add a tag', async () => {
|
||||
setupMockDataForStagedTagsTesting();
|
||||
render(<RootWrapper blockingSheet setBlockingSheet={mockSetBlockingSheet} />);
|
||||
expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument();
|
||||
|
||||
expect(mockSetBlockingSheet).toHaveBeenCalledWith(false);
|
||||
|
||||
// To edit mode
|
||||
const editTagsButton = screen.getByRole('button', {
|
||||
name: /edit tags/i,
|
||||
});
|
||||
fireEvent.click(editTagsButton);
|
||||
|
||||
// Click on "Add a tag" button to open dropdown
|
||||
const addTagsButton = screen.getByText(/add a tag/i);
|
||||
// Use `mouseDown` instead of `click` since the react-select didn't respond to `click`
|
||||
fireEvent.mouseDown(addTagsButton);
|
||||
|
||||
// Click to check Tag 3
|
||||
const tag3 = screen.getByText(/tag 3/i);
|
||||
fireEvent.click(tag3);
|
||||
|
||||
// Click "Add tags" to save to global staged tags
|
||||
const addTags = screen.getByRole('button', { name: /add tags/i });
|
||||
fireEvent.click(addTags);
|
||||
|
||||
expect(mockSetBlockingSheet).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it('should call `setBlockingSheet` on delete a tag', async () => {
|
||||
setupMockDataForStagedTagsTesting();
|
||||
render(<RootWrapper blockingSheet setBlockingSheet={mockSetBlockingSheet} />);
|
||||
expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument();
|
||||
|
||||
expect(mockSetBlockingSheet).toHaveBeenCalledWith(false);
|
||||
|
||||
// To edit mode
|
||||
const editTagsButton = screen.getByRole('button', {
|
||||
name: /edit tags/i,
|
||||
});
|
||||
fireEvent.click(editTagsButton);
|
||||
|
||||
// Delete the tag
|
||||
const tag = screen.getByText(/tag 2/i);
|
||||
const deleteButton = within(tag).getByRole('button', {
|
||||
name: /delete/i,
|
||||
});
|
||||
fireEvent.click(deleteButton);
|
||||
|
||||
expect(mockSetBlockingSheet).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it('should call `updateTags` mutation on save', async () => {
|
||||
setupMockDataForStagedTagsTesting();
|
||||
render(<RootWrapper />);
|
||||
|
||||
@@ -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,20 +238,33 @@ const useContentTagsDrawerContext = (contentId) => {
|
||||
setCollapsibleToInitalState,
|
||||
]);
|
||||
|
||||
// Build toast message and show toast after save drawer.
|
||||
// Count added and removed tags
|
||||
/* istanbul ignore next */
|
||||
const showToastAfterSave = React.useCallback(() => {
|
||||
const countTags = React.useCallback(() => {
|
||||
const tagsAddedList = Object.values(globalStagedContentTags);
|
||||
const tagsRemovedList = Object.values(globalStagedRemovedContentTags);
|
||||
|
||||
const tagsAdded = tagsAddedList.length === 1 ? tagsAddedList[0].length : tagsAddedList.reduce(
|
||||
/* istanbul ignore next */
|
||||
(acc, curr) => acc + curr.length,
|
||||
0,
|
||||
);
|
||||
const tagsRemoved = tagsRemovedList.length === 1 ? tagsRemovedList[0].length : tagsRemovedList.reduce(
|
||||
/* istanbul ignore next */
|
||||
(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 +289,7 @@ const useContentTagsDrawerContext = (contentId) => {
|
||||
);
|
||||
}
|
||||
setToastMessage(message);
|
||||
}, [globalStagedContentTags, globalStagedRemovedContentTags, setToastMessage]);
|
||||
}, [setToastMessage, countTags]);
|
||||
|
||||
// Close the toast
|
||||
const closeToast = React.useCallback(() => setToastMessage(undefined), [setToastMessage]);
|
||||
@@ -321,6 +337,31 @@ const useContentTagsDrawerContext = (contentId) => {
|
||||
const mergedTagsArray = fechedTaxonomies.map(obj => mergedTags[obj.id]);
|
||||
|
||||
setTagsByTaxonomy(mergedTagsArray);
|
||||
|
||||
if (setBlockingSheet) {
|
||||
const areChangesInTags = () => {
|
||||
// It is calculated in this way, because there are cases in which
|
||||
// there are keys in the map, but they contain empty lists
|
||||
// (e.g. add a tag, and remove the same tag later).
|
||||
|
||||
const tagsAddedList = Object.values(globalStagedContentTags);
|
||||
const tagsRemovedList = Object.values(globalStagedRemovedContentTags);
|
||||
|
||||
if (tagsAddedList.some(tags => tags.length > 0)) {
|
||||
return true;
|
||||
}
|
||||
if (tagsRemovedList.some(tags => tags.length > 0)) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
if (areChangesInTags()) {
|
||||
setBlockingSheet(true);
|
||||
} else {
|
||||
setBlockingSheet(false);
|
||||
}
|
||||
}
|
||||
}, [
|
||||
fechedTaxonomies,
|
||||
globalStagedContentTags,
|
||||
|
||||
44
src/content-tags-drawer/ContentTagsDrawerSheet.jsx
Normal file
44
src/content-tags-drawer/ContentTagsDrawerSheet.jsx
Normal file
@@ -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;
|
||||
@@ -35,3 +35,14 @@ export const ContentTagsDrawerContext = React.createContext({
|
||||
closeToast: /** @type{() => void} */ (() => {}),
|
||||
setCollapsibleToInitalState: /** @type{() => void} */ (() => {}),
|
||||
});
|
||||
|
||||
// This context has not been added to ContentTagsDrawerContext because it has been
|
||||
// created one level higher to control the behavior of the Sheet that contatins the Drawer.
|
||||
// This logic is not used in legacy edx-platform screens. But it can be separated if we keep
|
||||
// the contexts separate.
|
||||
// TODO We can join both contexts when the Drawer is no longer used on edx-platform
|
||||
/* istanbul ignore next */
|
||||
export const ContentTagsDrawerSheetContext = React.createContext({
|
||||
blockingSheet: /** @type{boolean} */ (false),
|
||||
setBlockingSheet: /** @type{Function} */ (() => {}),
|
||||
});
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -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';
|
||||
@@ -234,16 +233,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}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user