Skip to content

Commit

Permalink
feat: prevent losing work when users click outside tag drawer after m…
Browse files Browse the repository at this point in the history
…aking edits [FC-0036]
  • Loading branch information
ChrisChV authored May 8, 2024
1 parent 23fb68f commit dd9202f
Show file tree
Hide file tree
Showing 9 changed files with 235 additions and 45 deletions.
9 changes: 5 additions & 4 deletions src/content-tags-drawer/ContentTagsDrawer.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-check
import React, { useEffect } from 'react';
import React, { useContext, useEffect } from 'react';
import PropTypes from 'prop-types';
import {
Container,
Expand All @@ -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.
Expand All @@ -32,6 +32,7 @@ const ContentTagsDrawer = ({ id, onClose }) => {
const contentId = id ?? params.contentId;

const context = useContentTagsDrawerContext(contentId);
const { blockingSheet } = useContext(ContentTagsDrawerSheetContext);

const {
showToastAfterSave,
Expand Down Expand Up @@ -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();
}
};
Expand All @@ -73,7 +74,7 @@ const ContentTagsDrawer = ({ id, onClose }) => {
return () => {
document.removeEventListener('keydown', handleEsc);
};
}, []);
}, [blockingSheet]);

useEffect(() => {
/* istanbul ignore next */
Expand Down
117 changes: 112 additions & 5 deletions src/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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');

Expand All @@ -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 />);
Expand Down
47 changes: 44 additions & 3 deletions src/content-tags-drawer/ContentTagsDrawerHelper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -273,7 +289,7 @@ const useContentTagsDrawerContext = (contentId) => {
);
}
setToastMessage(message);
}, [globalStagedContentTags, globalStagedRemovedContentTags, setToastMessage]);
}, [setToastMessage, countTags]);

// Close the toast
const closeToast = React.useCallback(() => setToastMessage(undefined), [setToastMessage]);
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions src/content-tags-drawer/ContentTagsDrawerSheet.jsx
Original file line number Diff line number Diff line change
@@ -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;
11 changes: 11 additions & 0 deletions src/content-tags-drawer/common/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} */ (() => {}),
});
2 changes: 2 additions & 0 deletions src/content-tags-drawer/index.js
Original file line number Diff line number Diff line change
@@ -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';
17 changes: 6 additions & 11 deletions src/content-tags-drawer/tags-sidebar-controls/TagsSidebarBody.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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}
/>
</>
);
};
Expand Down
Loading

0 comments on commit dd9202f

Please sign in to comment.