From 4e70813fa95d4b6f33ab85196b95cc53b118be41 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Wed, 13 Mar 2024 16:27:30 +0300 Subject: [PATCH] [FC-0036] feat: New "Add Tags" widget (#834) * feat: Use react-select for tags selector Replace existing component with react-select component, by passing in our custom component. This retained the existing search functionality. * fix: Fix missing deps causing constant rerender This bug appeared after removing the react-query call to the backend when selecting/unselecting a tag in the dropdown. Since it no longer gets the updated state from the backend, it doesnt mask the bug. The bug is essentially the `ContentTagsCollapsibleHelper` rerendering causing the states to reset overriding the selected (not commited) tags. This is due to missing dependancies in the useCallback. * feat: Add stagedContentTags state in react-select This adds a state and callbacks in the toplevel component of the content tags drawer to be able to add/remove staged content tags and have them showup in the react-select as selected chips. * feat: Split up applied & staged content tags trees Now content tags have seperate tree states for applied ones and staged ones. They are updated seperately and both are used when updating the selectable box UI. This allows for more flexibility with actions that can be performed on the staged content tags with impacting the applied ones. * feat: Change style of implicit checkbox to checks This overrides the indeterminate input checkbox style to match the checked checkbox style, using variables defined in paragon. * feat: Add bottom buttons in tags dropdown selector * refactor: Remove cloneDeep + simplify code * feat: Update placeholder/button texts * feat: Implement cancel button + add proptypes * feat: Implement commit/cancel staged tags This implements the commit functionality for staged tags, taking account for implicit tags. This also handles the case for removing applied tags by clicking on the "x" in the TagBubble. * feat: Keep all staged tags only commit explicit * feat: Change style of add/cancel/load more buttons * feat: Add inline "Add" button to commit tags In the react-select component, an inline "Add" button showsup when some tags are staged, if they are clicked they are commited/applied. * fix: Keep applied tag checked when only staged child unchecked * feat: Style add tags widget + staged tags Also clear search term whenever tags are staged/cancelled * feat: Fixed some typing errors * test: Update tests to fix existing broken cases * test: Add new functionality tests * chore: add types to ContentTagsCollapsible * chore: add types for useContentTagsCollapsibleHelper * fix: Small bug with useIntl * chore: Fix more linter issues * refactor: Separate stagedTags and stagedTagsTree state updates This refactor removed the warning that was caused because the state of a parent component (ContentTagsDrawer) was being updated in the middle of a state update in (ContentTagsCollapsible). This seperated the two state updates to avoid this issue. * chore: Update package-lock.json * fix: Reset applied tags in selectbox when fetching Whenever we get new applied tags from the backend, we reset the applied tags that are checked, and only check the explicit tags. This was causing an issue of duplicate applied tags being added to the selectbox. * chore: Update package.json --------- Co-authored-by: Braden MacDonald --- package-lock.json | 197 +++++++++++- package.json | 1 + .../ContentTagsCollapsible.d.ts | 39 +++ .../ContentTagsCollapsible.jsx | 292 +++++++++++++----- .../ContentTagsCollapsible.scss | 30 ++ .../ContentTagsCollapsible.test.jsx | 248 +++++++++++---- .../ContentTagsCollapsibleHelper.jsx | 290 +++++++++++------ src/content-tags-drawer/ContentTagsDrawer.jsx | 42 ++- .../ContentTagsDrawer.test.jsx | 182 +++++++++++ .../ContentTagsDropDownSelector.jsx | 52 +++- .../ContentTagsDropDownSelector.scss | 13 + .../ContentTagsDropDownSelector.test.jsx | 89 ++---- src/content-tags-drawer/TagBubble.jsx | 2 +- src/content-tags-drawer/messages.js | 16 + src/index.scss | 2 + 15 files changed, 1198 insertions(+), 297 deletions(-) create mode 100644 src/content-tags-drawer/ContentTagsCollapsible.d.ts diff --git a/package-lock.json b/package-lock.json index fa514c9f7..a4d615c22 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,6 +54,7 @@ "react-responsive": "9.0.2", "react-router": "6.16.0", "react-router-dom": "6.16.0", + "react-select": "^5.8.0", "react-textarea-autosize": "^8.4.1", "react-transition-group": "4.4.5", "redux": "4.0.5", @@ -3012,6 +3013,117 @@ "tslib": "^2.4.0" } }, + "node_modules/@emotion/babel-plugin": { + "version": "11.11.0", + "resolved": "https://registry.npmjs.org/@emotion/babel-plugin/-/babel-plugin-11.11.0.tgz", + "integrity": "sha512-m4HEDZleaaCH+XgDDsPF15Ht6wTLsgDTeR3WYj9Q/k76JtWhrJjcP4+/XlG8LGT/Rol9qUfOIztXeA84ATpqPQ==", + "dependencies": { + "@babel/helper-module-imports": "^7.16.7", + "@babel/runtime": "^7.18.3", + "@emotion/hash": "^0.9.1", + "@emotion/memoize": "^0.8.1", + "@emotion/serialize": "^1.1.2", + "babel-plugin-macros": "^3.1.0", + "convert-source-map": "^1.5.0", + "escape-string-regexp": "^4.0.0", + "find-root": "^1.1.0", + "source-map": "^0.5.7", + "stylis": "4.2.0" + } + }, + "node_modules/@emotion/babel-plugin/node_modules/source-map": { + "version": "0.5.7", + "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.5.7.tgz", + "integrity": "sha512-LbrmJOMUSdEVxIKvdcJzQC+nQhe8FUZQTXQy6+I75skNgn3OoQ0DZA8YnFa7gp8tqtL3KPf1kmo0R5DoApeSGQ==", + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/@emotion/cache": { + "version": "11.11.0", + "resolved": "https://registry.npmjs.org/@emotion/cache/-/cache-11.11.0.tgz", + "integrity": "sha512-P34z9ssTCBi3e9EI1ZsWpNHcfY1r09ZO0rZbRO2ob3ZQMnFI35jB536qoXbkdesr5EUhYi22anuEJuyxifaqAQ==", + "dependencies": { + "@emotion/memoize": "^0.8.1", + "@emotion/sheet": "^1.2.2", + "@emotion/utils": "^1.2.1", + "@emotion/weak-memoize": "^0.3.1", + "stylis": "4.2.0" + } + }, + "node_modules/@emotion/hash": { + "version": "0.9.1", + "resolved": "https://registry.npmjs.org/@emotion/hash/-/hash-0.9.1.tgz", + "integrity": "sha512-gJB6HLm5rYwSLI6PQa+X1t5CFGrv1J1TWG+sOyMCeKz2ojaj6Fnl/rZEspogG+cvqbt4AE/2eIyD2QfLKTBNlQ==" + }, + "node_modules/@emotion/memoize": { + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@emotion/memoize/-/memoize-0.8.1.tgz", + "integrity": "sha512-W2P2c/VRW1/1tLox0mVUalvnWXxavmv/Oum2aPsRcoDJuob75FC3Y8FbpfLwUegRcxINtGUMPq0tFCvYNTBXNA==" + }, + "node_modules/@emotion/react": { + "version": "11.11.3", + "resolved": "https://registry.npmjs.org/@emotion/react/-/react-11.11.3.tgz", + "integrity": "sha512-Cnn0kuq4DoONOMcnoVsTOR8E+AdnKFf//6kUWc4LCdnxj31pZWn7rIULd6Y7/Js1PiPHzn7SKCM9vB/jBni8eA==", + "dependencies": { + "@babel/runtime": "^7.18.3", + "@emotion/babel-plugin": "^11.11.0", + "@emotion/cache": "^11.11.0", + "@emotion/serialize": "^1.1.3", + "@emotion/use-insertion-effect-with-fallbacks": "^1.0.1", + "@emotion/utils": "^1.2.1", + "@emotion/weak-memoize": "^0.3.1", + "hoist-non-react-statics": "^3.3.1" + }, + "peerDependencies": { + "react": ">=16.8.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + } + } + }, + "node_modules/@emotion/serialize": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/@emotion/serialize/-/serialize-1.1.3.tgz", + "integrity": "sha512-iD4D6QVZFDhcbH0RAG1uVu1CwVLMWUkCvAqqlewO/rxf8+87yIBAlt4+AxMiiKPLs5hFc0owNk/sLLAOROw3cA==", + "dependencies": { + "@emotion/hash": "^0.9.1", + "@emotion/memoize": "^0.8.1", + "@emotion/unitless": "^0.8.1", + "@emotion/utils": "^1.2.1", + "csstype": "^3.0.2" + } + }, + "node_modules/@emotion/sheet": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/@emotion/sheet/-/sheet-1.2.2.tgz", + "integrity": "sha512-0QBtGvaqtWi+nx6doRwDdBIzhNdZrXUppvTM4dtZZWEGTXL/XE/yJxLMGlDT1Gt+UHH5IX1n+jkXyytE/av7OA==" + }, + "node_modules/@emotion/unitless": { + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@emotion/unitless/-/unitless-0.8.1.tgz", + "integrity": "sha512-KOEGMu6dmJZtpadb476IsZBclKvILjopjUii3V+7MnXIQCYh8W3NgNcgwo21n9LXZX6EDIKvqfjYxXebDwxKmQ==" + }, + "node_modules/@emotion/use-insertion-effect-with-fallbacks": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/@emotion/use-insertion-effect-with-fallbacks/-/use-insertion-effect-with-fallbacks-1.0.1.tgz", + "integrity": "sha512-jT/qyKZ9rzLErtrjGgdkMBn2OP8wl0G3sQlBb3YPryvKHsjvINUhVaPFfP+fpBcOkmrVOVEEHQFJ7nbj2TH2gw==", + "peerDependencies": { + "react": ">=16.8.0" + } + }, + "node_modules/@emotion/utils": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@emotion/utils/-/utils-1.2.1.tgz", + "integrity": "sha512-Y2tGf3I+XVnajdItskUCn6LX+VUDmP6lTL4fcqsXAv43dnlbZiuW4MWQW38rW/BVWSE7Q/7+XQocmpnRYILUmg==" + }, + "node_modules/@emotion/weak-memoize": { + "version": "0.3.1", + "resolved": "https://registry.npmjs.org/@emotion/weak-memoize/-/weak-memoize-0.3.1.tgz", + "integrity": "sha512-EsBwpc7hBUJWAsNPBmJy4hxWx12v6bshQsldrVmjxJoc3isbxhOrF2IcCpaXxfvq03NwkI7sbsOLXbYuqF/8Ww==" + }, "node_modules/@eslint-community/eslint-utils": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/@eslint-community/eslint-utils/-/eslint-utils-4.4.0.tgz", @@ -3116,6 +3228,28 @@ "node": "^12.22.0 || ^14.17.0 || >=16.0.0" } }, + "node_modules/@floating-ui/core": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@floating-ui/core/-/core-1.6.0.tgz", + "integrity": "sha512-PcF++MykgmTj3CIyOQbKA/hDzOAiqI3mhuoN44WRCopIs1sgoDoU4oty4Jtqaj/y3oDU6fnVSm4QG0a3t5i0+g==", + "dependencies": { + "@floating-ui/utils": "^0.2.1" + } + }, + "node_modules/@floating-ui/dom": { + "version": "1.6.3", + "resolved": "https://registry.npmjs.org/@floating-ui/dom/-/dom-1.6.3.tgz", + "integrity": "sha512-RnDthu3mzPlQ31Ss/BTwQ1zjzIhr3lk1gZB1OC56h/1vEtaXkESrOqL5fQVMfXpwGtRwX+YsZBdyHtJMQnkArw==", + "dependencies": { + "@floating-ui/core": "^1.0.0", + "@floating-ui/utils": "^0.2.0" + } + }, + "node_modules/@floating-ui/utils": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/@floating-ui/utils/-/utils-0.2.1.tgz", + "integrity": "sha512-9TANp6GPoMtYzQdt54kfAyMmz1+osLlXdg2ENroU7zzrtflTLrrC/lgrIfaSe+Wu0b89GKccT7vxXA0MoAIO+Q==" + }, "node_modules/@formatjs/cli": { "version": "6.2.7", "resolved": "https://registry.npmjs.org/@formatjs/cli/-/cli-6.2.7.tgz", @@ -7106,6 +7240,35 @@ "node": ">= 10.14.2" } }, + "node_modules/babel-plugin-macros": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/babel-plugin-macros/-/babel-plugin-macros-3.1.0.tgz", + "integrity": "sha512-Cg7TFGpIr01vOQNODXOOaGz2NpCU5gl8x1qJFbb6hbZxR7XrcE2vtbAsTAbJ7/xwJtUuJEw8K8Zr/AE0LHlesg==", + "dependencies": { + "@babel/runtime": "^7.12.5", + "cosmiconfig": "^7.0.0", + "resolve": "^1.19.0" + }, + "engines": { + "node": ">=10", + "npm": ">=6" + } + }, + "node_modules/babel-plugin-macros/node_modules/cosmiconfig": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-7.1.0.tgz", + "integrity": "sha512-AdmX6xUzdNASswsFtmwSt7Vj8po9IuqXm0UXz7QKPuEUmPB4XyjGfaAr2PSuELMwkRMVH1EpIkX5bTZGRB3eCA==", + "dependencies": { + "@types/parse-json": "^4.0.0", + "import-fresh": "^3.2.1", + "parse-json": "^5.0.0", + "path-type": "^4.0.0", + "yaml": "^1.10.0" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/babel-plugin-polyfill-corejs2": { "version": "0.4.8", "resolved": "https://registry.npmjs.org/babel-plugin-polyfill-corejs2/-/babel-plugin-polyfill-corejs2-0.4.8.tgz", @@ -10528,8 +10691,7 @@ "node_modules/find-root": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/find-root/-/find-root-1.1.0.tgz", - "integrity": "sha512-NKfW6bec6GfKc0SGx1e07QZY9PE99u0Bft/0rzSD5k3sO/vwkVUpDUKVm5Gpp5Ue3YfShPFTX2070tDs5kB9Ng==", - "dev": true + "integrity": "sha512-NKfW6bec6GfKc0SGx1e07QZY9PE99u0Bft/0rzSD5k3sO/vwkVUpDUKVm5Gpp5Ue3YfShPFTX2070tDs5kB9Ng==" }, "node_modules/find-up": { "version": "5.0.0", @@ -14686,6 +14848,11 @@ "node": ">= 4.0.0" } }, + "node_modules/memoize-one": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-6.0.0.tgz", + "integrity": "sha512-rkpe71W0N0c0Xz6QD0eJETuWAJGnJ9afsl1srmwPrI+yBCkge5EycXXbYRyvL29zZVUWQCY7InPRCv3GDXuZNw==" + }, "node_modules/memory-fs": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/memory-fs/-/memory-fs-0.2.0.tgz", @@ -17518,6 +17685,26 @@ "react-dom": ">=16.8" } }, + "node_modules/react-select": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/react-select/-/react-select-5.8.0.tgz", + "integrity": "sha512-TfjLDo58XrhP6VG5M/Mi56Us0Yt8X7xD6cDybC7yoRMUNm7BGO7qk8J0TLQOua/prb8vUOtsfnXZwfm30HGsAA==", + "dependencies": { + "@babel/runtime": "^7.12.0", + "@emotion/cache": "^11.4.0", + "@emotion/react": "^11.8.1", + "@floating-ui/dom": "^1.0.1", + "@types/react-transition-group": "^4.4.0", + "memoize-one": "^6.0.0", + "prop-types": "^15.6.0", + "react-transition-group": "^4.3.0", + "use-isomorphic-layout-effect": "^1.1.2" + }, + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0 || ^18.0.0", + "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" + } + }, "node_modules/react-shallow-renderer": { "version": "16.15.0", "resolved": "https://registry.npmjs.org/react-shallow-renderer/-/react-shallow-renderer-16.15.0.tgz", @@ -19907,6 +20094,11 @@ "node": "^12.13.0 || ^14.15.0 || >=16.0.0" } }, + "node_modules/stylis": { + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/stylis/-/stylis-4.2.0.tgz", + "integrity": "sha512-Orov6g6BB1sDfYgzWfTHDOxamtX1bE/zo104Dh9e6fqJ3PooipYyfJ0pUmrZO2wAvO8YbEyeFrkV91XTsGMSrw==" + }, "node_modules/superagent": { "version": "3.8.3", "resolved": "https://registry.npmjs.org/superagent/-/superagent-3.8.3.tgz", @@ -21914,6 +22106,7 @@ "version": "0.1.0", "peerDependencies": { "@edx/frontend-app-course-authoring": "*", + "@edx/frontend-lib-content-components": "*", "@edx/frontend-platform": "*", "@openedx/paragon": "*", "@reduxjs/toolkit": "*", diff --git a/package.json b/package.json index 2c91d5ae8..c740331c6 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "react-responsive": "9.0.2", "react-router": "6.16.0", "react-router-dom": "6.16.0", + "react-select": "5.8.0", "react-textarea-autosize": "^8.4.1", "react-transition-group": "4.4.5", "redux": "4.0.5", diff --git a/src/content-tags-drawer/ContentTagsCollapsible.d.ts b/src/content-tags-drawer/ContentTagsCollapsible.d.ts new file mode 100644 index 000000000..55759439e --- /dev/null +++ b/src/content-tags-drawer/ContentTagsCollapsible.d.ts @@ -0,0 +1,39 @@ +import type {} from 'react-select/base'; +// This import is necessary for module augmentation. +// It allows us to extend the 'Props' interface in the 'react-select/base' module +// and add our custom property 'myCustomProp' to it. + +export interface TagTreeEntry { + explicit: boolean; + children: Record; + canChangeObjecttag: boolean; + canDeleteObjecttag: boolean; +} + +export interface TaxonomySelectProps { + taxonomyId: number; + searchTerm: string; + appliedContentTagsTree: Record; + stagedContentTagsTree: Record; + checkedTags: string[]; + handleCommitStagedTags: () => void; + handleCancelStagedTags: () => void; + handleSelectableBoxChange: React.ChangeEventHandler; +} + +// Unfortunately the only way to specify the custom props we pass into React Select +// is with this global type augmentation. +// https://react-select.com/typescript#custom-select-props +// If in the future other parts of this MFE need to use React Select for different things, +// we should change to using a 'react context' to share this data within , +// rather than using the custom )} - -
- - - {}} - onChange={handleSearchChange} - className="mb-2" - /> - - - -
- -
{ ); }; -ContentTagsCollapsible.propTypes = { - contentId: PropTypes.string.isRequired, - taxonomyAndTagsData: PropTypes.shape({ - id: PropTypes.number, - name: PropTypes.string, - contentTags: PropTypes.arrayOf(PropTypes.shape({ - value: PropTypes.string, - lineage: PropTypes.arrayOf(PropTypes.string), - })), - canTagObject: PropTypes.bool.isRequired, - }).isRequired, -}; - export default ContentTagsCollapsible; diff --git a/src/content-tags-drawer/ContentTagsCollapsible.scss b/src/content-tags-drawer/ContentTagsCollapsible.scss index 3123eebbf..67a51a77e 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.scss +++ b/src/content-tags-drawer/ContentTagsCollapsible.scss @@ -27,3 +27,33 @@ .pgn__modal-popup__arrow { visibility: hidden; } + +.add-tags-button:not([disabled]):hover { + background-color: transparent; + color: $info-900 !important; +} + +.cancel-add-tags-button:hover { + background-color: transparent; + color: $gray-300 !important; +} + +.react-select-add-tags__control { + border-radius: 0 !important; +} + +.react-select-add-tags__control--is-focused { + border-color: black !important; + box-shadow: 0 0 0 1px black !important; +} + +.react-select-add-tags__multi-value__remove { + padding-right: 7px !important; + padding-left: 7px !important; + border-radius: 0 3px 3px 0; + + &:hover { + background-color: black !important; + color: white !important; + } +} diff --git a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx index 772087c18..1b0ed8602 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.test.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.test.jsx @@ -51,11 +51,29 @@ const data = { }, ], }, + stagedContentTags: [], + addStagedContentTag: jest.fn(), + removeStagedContentTag: jest.fn(), + setStagedTags: jest.fn(), }; -const ContentTagsCollapsibleComponent = ({ contentId, taxonomyAndTagsData }) => ( +const ContentTagsCollapsibleComponent = ({ + contentId, + taxonomyAndTagsData, + stagedContentTags, + addStagedContentTag, + removeStagedContentTag, + setStagedTags, +}) => ( - + ); @@ -70,6 +88,10 @@ describe('', () => { jest.useRealTimers(); // Restore real timers after the tests }); + afterEach(() => { + jest.clearAllMocks(); // Reset all mock function call counts after each test case + }); + async function getComponent(updatedData) { const componentData = (!updatedData ? data : updatedData); @@ -77,6 +99,10 @@ describe('', () => { , ); } @@ -130,59 +156,157 @@ describe('', () => { expect(getByText('3')).toBeInTheDocument(); }); - it('should render new tags as they are checked in the dropdown', async () => { + it('should call `addStagedContentTag` when tag checked in the dropdown', async () => { setupTaxonomyMock(); const { container, getByText, getAllByText } = await getComponent(); - // Expand the Taxonomy to view applied tags and "Add tags" button - const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; - fireEvent.click(expandToggle); - - // Click on "Add tags" button to open dropdown to select new tags - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - fireEvent.click(addTagsButton); - - // Wait for the dropdown selector for tags to open, - // Tag 3 should only appear there - expect(getByText('Tag 3')).toBeInTheDocument(); - expect(getAllByText('Tag 3').length === 1); - - const tag3 = getByText('Tag 3'); - - fireEvent.click(tag3); - - // After clicking on Tag 3, it should also appear in amongst - // the tag bubbles in the tree - expect(getAllByText('Tag 3').length === 2); - }); - - it('should remove tag when they are unchecked in the dropdown', async () => { - setupTaxonomyMock(); - const { container, getByText, getAllByText } = await getComponent(); - - // Expand the Taxonomy to view applied tags and "Add tags" button + // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; fireEvent.click(expandToggle); - // Check that Tag 2 appears in tag bubbles - expect(getByText('Tag 2')).toBeInTheDocument(); - - // Click on "Add tags" button to open dropdown to select new tags - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - fireEvent.click(addTagsButton); + // Click on "Add a tag" button to open dropdown to select new tags + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown/mouseUp` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); + fireEvent.mouseUp(addTagsButton); // Wait for the dropdown selector for tags to open, // Tag 3 should only appear there, (i.e. the dropdown is open, since Tag 3 is not applied) - expect(getByText('Tag 3')).toBeInTheDocument(); + expect(getAllByText('Tag 3').length).toBe(1); - // Get the Tag 2 checkbox and click on it - const tag2 = getAllByText('Tag 2')[1]; - fireEvent.click(tag2); + // Click to check Tag 3 and check the `addStagedContentTag` was called with the correct params + const tag3 = getByText('Tag 3'); + fireEvent.click(tag3); - // After clicking on Tag 2, it should be removed from - // the tag bubbles in so only the one in the dropdown appears - expect(getAllByText('Tag 2').length === 1); + const taxonomyId = 123; + const addedStagedTag = { + value: 'Tag%203', + label: 'Tag 3', + }; + expect(data.addStagedContentTag).toHaveBeenCalledTimes(1); + expect(data.addStagedContentTag).toHaveBeenCalledWith(taxonomyId, addedStagedTag); + }); + + it('should call `removeStagedContentTag` when tag staged tag unchecked in the dropdown', async () => { + setupTaxonomyMock(); + const { container, getByText, getAllByText } = await getComponent(); + + // Expand the Taxonomy to view applied tags and "Add a tag" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on "Add a tag" button to open dropdown to select new tags + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown/mouseup` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); + fireEvent.mouseUp(addTagsButton); + + // Wait for the dropdown selector for tags to open, + // Tag 3 should only appear there, (i.e. the dropdown is open, since Tag 3 is not applied) + expect(getAllByText('Tag 3').length).toBe(1); + + // Click to check Tag 3 + const tag3 = getByText('Tag 3'); + fireEvent.click(tag3); + + // Click to uncheck Tag 3 and check the `removeStagedContentTag` was called with the correct params + fireEvent.click(tag3); + const taxonomyId = 123; + const tagValue = 'Tag%203'; + expect(data.removeStagedContentTag).toHaveBeenCalledTimes(1); + expect(data.removeStagedContentTag).toHaveBeenCalledWith(taxonomyId, tagValue); + }); + + it('should call `setStagedTags` to clear staged tags when clicking inline "Add" button', async () => { + setupTaxonomyMock(); + // Setup component to have staged tags + const { container, getByText } = await getComponent({ + ...data, + stagedContentTags: [{ + value: 'Tag%203', + label: 'Tag 3', + }], + }); + + // Expand the Taxonomy to view applied tags and staged tags + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on inline "Add" button and check that the appropriate methods are called + const inlineAdd = getByText(messages.collapsibleInlineAddStagedTagsButtonText.defaultMessage); + fireEvent.click(inlineAdd); + + // Check that `setStagedTags` called with empty tags list to clear staged tags + const taxonomyId = 123; + expect(data.setStagedTags).toHaveBeenCalledTimes(1); + expect(data.setStagedTags).toHaveBeenCalledWith(taxonomyId, []); + }); + + it('should call `setStagedTags` to clear staged tags when clicking "Add tags" button in dropdown', async () => { + setupTaxonomyMock(); + // Setup component to have staged tags + const { container, getByText } = await getComponent({ + ...data, + stagedContentTags: [{ + value: 'Tag%203', + label: 'Tag 3', + }], + }); + + // Expand the Taxonomy to view applied tags and staged tags + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on dropdown with staged tags to expand it + const selectTagsDropdown = container.getElementsByClassName('react-select-add-tags__control')[0]; + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(selectTagsDropdown); + + // Click on "Add tags" button and check that the appropriate methods are called + const dropdownAdd = getByText(messages.collapsibleAddStagedTagsButtonText.defaultMessage); + fireEvent.click(dropdownAdd); + + // Check that `setStagedTags` called with empty tags list to clear staged tags + const taxonomyId = 123; + expect(data.setStagedTags).toHaveBeenCalledTimes(1); + expect(data.setStagedTags).toHaveBeenCalledWith(taxonomyId, []); + }); + + it('should close dropdown and clear staged tags when clicking "Cancel" inside dropdown', async () => { + // Setup component to have staged tags + const { container, getByText } = await getComponent({ + ...data, + stagedContentTags: [{ + value: 'Tag%203', + label: 'Tag 3', + }], + }); + + // Expand the Taxonomy to view applied tags and staged tags + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on dropdown with staged tags to expand it + const selectTagsDropdown = container.getElementsByClassName('react-select-add-tags__control')[0]; + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(selectTagsDropdown); + + // Click on inline "Add" button and check that the appropriate methods are called + const dropdownCancel = getByText(messages.collapsibleCancelStagedTagsButtonText.defaultMessage); + fireEvent.click(dropdownCancel); + + // Check that `setStagedTags` called with empty tags list to clear staged tags + const taxonomyId = 123; + expect(data.setStagedTags).toHaveBeenCalledTimes(1); + expect(data.setStagedTags).toHaveBeenCalledWith(taxonomyId, []); + + // Check that the dropdown is closed + expect(dropdownCancel).not.toBeInTheDocument(); }); it('should handle search term change', async () => { @@ -190,16 +314,17 @@ describe('', () => { container, getByText, getByRole, getByDisplayValue, } = await getComponent(); - // Expand the Taxonomy to view applied tags and "Add tags" button + // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; fireEvent.click(expandToggle); - // Click on "Add tags" button to open dropdown - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - fireEvent.click(addTagsButton); + // Click on "Add a tag" button to open dropdown + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown` instead of `click` since the react-select didn't respond to click + fireEvent.mouseDown(addTagsButton); // Get the search field - const searchField = getByRole('searchbox'); + const searchField = getByRole('combobox'); const searchTerm = 'memo'; @@ -226,14 +351,15 @@ describe('', () => { setupTaxonomyMock(); const { container, getByText, queryByText } = await getComponent(); - // Expand the Taxonomy to view applied tags and "Add tags" button + // Expand the Taxonomy to view applied tags and "Add a tag" button const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; fireEvent.click(expandToggle); - // Click on "Add tags" button to open dropdown - const addTagsButton = getByText(messages.addTagsButtonText.defaultMessage); - fireEvent.click(addTagsButton); + // Click on "Add a tag" button to open dropdown + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); // Wait for the dropdown selector for tags to open, Tag 3 should appear // since it is not applied @@ -250,6 +376,24 @@ describe('', () => { expect(queryByText('Tag 3')).not.toBeInTheDocument(); }); + it('should remove applied tags when clicking on `x` of tag bubble', async () => { + setupTaxonomyMock(); + const { container, getByText } = await getComponent(); + + // Expand the Taxonomy to view applied tags + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on 'x' of applied tag to remove it + const appliedTag = getByText('Tag 2'); + const xButtonAppliedTag = appliedTag.nextSibling; + xButtonAppliedTag.click(); + + // Check that the applied tag has been removed + expect(appliedTag).not.toBeInTheDocument(); + }); + it('should render taxonomy tags data without tags number badge', async () => { const updatedData = { ...data }; updatedData.taxonomyAndTagsData = { ...updatedData.taxonomyAndTagsData }; diff --git a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx index aed895c45..6ded9481d 100644 --- a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx @@ -5,80 +5,85 @@ import { cloneDeep } from 'lodash'; import { useContentTaxonomyTagsUpdater } from './data/apiHooks'; +/** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ +/** @typedef {import("./data/types.mjs").Tag} ContentTagData */ +/** @typedef {import("./ContentTagsCollapsible").TagTreeEntry} TagTreeEntry */ + /** - * Util function that consolidates two tag trees into one, sorting the keys in - * alphabetical order. + * Util function that sorts the keys of a tree in alphabetical order. * - * @param {object} tree1 - first tag tree - * @param {object} tree2 - second tag tree - * @returns {object} merged tree containing both tree1 and tree2 + * @param {object} tree - tree that needs it's keys sorted + * @returns {object} sorted tree */ -const mergeTrees = (tree1, tree2) => { - const mergedTree = cloneDeep(tree1); - - const sortKeysAlphabetically = (obj) => { - const sortedObj = {}; - Object.keys(obj) - .sort() - .forEach((key) => { - sortedObj[key] = obj[key]; - if (obj[key] && typeof obj[key] === 'object') { - sortedObj[key].children = sortKeysAlphabetically(obj[key].children); - } - }); - return sortedObj; - }; - - const mergeRecursively = (destination, source) => { - Object.entries(source).forEach(([key, sourceValue]) => { - const destinationValue = destination[key]; - - if (destinationValue && sourceValue && typeof destinationValue === 'object' && typeof sourceValue === 'object') { - mergeRecursively(destinationValue, sourceValue); - } else { - // eslint-disable-next-line no-param-reassign - destination[key] = cloneDeep(sourceValue); +const sortKeysAlphabetically = (tree) => { + const sortedObj = {}; + Object.keys(tree) + .sort() + .forEach((key) => { + sortedObj[key] = tree[key]; + if (tree[key] && typeof tree[key] === 'object') { + sortedObj[key].children = sortKeysAlphabetically(tree[key].children); } }); - }; - - mergeRecursively(mergedTree, tree2); - return sortKeysAlphabetically(mergedTree); + return sortedObj; }; /** - * Util function that removes the tag along with its ancestors if it was - * the only explicit child tag. + * Util function that returns the leafs of a tree. Mainly used to extract the explicit + * tags selected in the staged tags tree * - * @param {object} tree - tag tree to remove the tag from - * @param {string[]} tagsToRemove - full lineage of tag to remove. - * eg: ['grand parent', 'parent', 'tag'] + * @param {object} tree - tree to extract the leaf tags from + * @returns {Array} array of leaf (explicit) tags of provided tree */ -const removeTags = (tree, tagsToRemove) => { - if (!tree || !tagsToRemove.length) { - return; - } - const key = tagsToRemove[0]; - if (tree[key]) { - removeTags(tree[key].children, tagsToRemove.slice(1)); +const getLeafTags = (tree) => { + const leafKeys = []; - if (Object.keys(tree[key].children).length === 0 && (tree[key].explicit === false || tagsToRemove.length === 1)) { - // eslint-disable-next-line no-param-reassign - delete tree[key]; - } + function traverse(node) { + Object.keys(node).forEach(key => { + const child = node[key]; + if (Object.keys(child.children).length === 0) { + leafKeys.push(key); + } else { + traverse(child.children); + } + }); } + + traverse(tree); + return leafKeys; }; -/* +/** * Handles all the underlying logic for the ContentTagsCollapsible component + * @param {string} contentId The ID of the content we're tagging (e.g. usage key) + * @param {TaxonomyData & {contentTags: ContentTagData[]}} taxonomyAndTagsData + * @param {(taxonomyId: number, tag: {value: string, label: string}) => void} addStagedContentTag + * @param {(taxonomyId: number, tagValue: string) => void} removeStagedContentTag + * @param {{value: string, label: string}[]} stagedContentTags + * @returns {{ + * tagChangeHandler: (tagSelectableBoxValue: string, checked: boolean) => void, + * removeAppliedTagHandler: (tagSelectableBoxValue: string) => void, + * appliedContentTagsTree: Record, + * stagedContentTagsTree: Record, + * contentTagsCount: number, + * checkedTags: any, + * commitStagedTags: () => void, + * updateTags: import('@tanstack/react-query').UseMutationResult + * }} */ -const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { +const useContentTagsCollapsibleHelper = ( + contentId, + taxonomyAndTagsData, + addStagedContentTag, + removeStagedContentTag, + stagedContentTags, +) => { const { id, contentTags, canTagObject, } = taxonomyAndTagsData; - // State to determine whether the tags are being updating so we can make a call + // State to determine whether an applied tag was removed so we make a call // to the update endpoint to the reflect those changes - const [updatingTags, setUpdatingTags] = React.useState(false); + const [removingAppliedTag, setRemoveAppliedTag] = React.useState(false); const updateTags = useContentTaxonomyTagsUpdater(contentId, id); // Keeps track of the content objects tags count (both implicit and explicit) @@ -86,32 +91,55 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { // Keeps track of the tree structure for tags that are add by selecting/unselecting // tags in the dropdowns. - const [addedContentTags, setAddedContentTags] = React.useState({}); + const [stagedContentTagsTree, setStagedContentTagsTree] = React.useState({}); // To handle checking/unchecking tags in the SelectableBox - const [checkedTags, { add, remove, clear }] = useCheckboxSetValues(); + const [checkedTags, { add, remove }] = useCheckboxSetValues(); - // Handles making requests to the update endpoint whenever the checked tags change + // State to keep track of the staged tags (and along with ancestors) that should be removed + const [stagedTagsToRemove, setStagedTagsToRemove] = React.useState(/** @type string[] */([])); + + // Handles making requests to the backend when applied tags are removed React.useEffect(() => { // We have this check because this hook is fired when the component first loads // and reloads (on refocus). We only want to make a request to the update endpoint when - // the user is updating the tags. - if (updatingTags) { - setUpdatingTags(false); + // the user removes an applied tag + if (removingAppliedTag) { + setRemoveAppliedTag(false); + + // Filter out staged tags from the checktags so they do not get committed const tags = checkedTags.map(t => decodeURIComponent(t.split(',').slice(-1))); - updateTags.mutate({ tags }); + const staged = stagedContentTags.map(t => t.label); + const remainingAppliedTags = tags.filter(t => !staged.includes(t)); + + updateTags.mutate({ tags: remainingAppliedTags }); } - }, [contentId, id, canTagObject, checkedTags]); + }, [contentId, id, canTagObject, checkedTags, stagedContentTags]); + + // Handles the removal of staged content tags based on what was removed + // from the staged tags tree. We are doing it in a useEffect since the removeTag + // method is being called inside a setState of the parent component, which + // was causing warnings + React.useEffect(() => { + stagedTagsToRemove.forEach(tag => removeStagedContentTag(id, tag)); + }, [stagedTagsToRemove, removeStagedContentTag, id]); + + // Handles making requests to the update endpoint when the staged tags need to be committed + const commitStagedTags = React.useCallback(() => { + // Filter out only leaf nodes of staging tree to commit + const explicitStaged = getLeafTags(stagedContentTagsTree); + + // Filter out applied tags that should become implicit because a child tag was committed + const stagedLineages = stagedContentTags.map(st => decodeURIComponent(st.value).split(',').slice(0, -1)).flat(); + const applied = contentTags.map((t) => t.value).filter(t => !stagedLineages.includes(t)); + + updateTags.mutate({ tags: [...applied, ...explicitStaged] }); + }, [contentTags, stagedContentTags, stagedContentTagsTree, updateTags]); // This converts the contentTags prop to the tree structure mentioned above - const appliedContentTags = React.useMemo(() => { + const appliedContentTagsTree = React.useMemo(() => { let contentTagsCounter = 0; - // Clear all the tags that have not been commited and the checked boxes when - // fresh contentTags passed in so the latest state from the backend is rendered - setAddedContentTags({}); - clear(); - // When an error occurs while updating, the contentTags query is invalidated, // hence they will be recalculated, and the updateTags mutation should be reset. if (updateTags.isError) { @@ -134,8 +162,12 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { // Populating the SelectableBox with "selected" (explicit) tags const value = item.lineage.map(l => encodeURIComponent(l)).join(','); - // eslint-disable-next-line no-unused-expressions - isExplicit ? add(value) : remove(value); + // Clear all the existing applied tags + remove(value); + // Add only the explicitly applied tags + if (isExplicit) { + add(value); + } contentTagsCounter += 1; } @@ -147,13 +179,53 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { return resultTree; }, [contentTags, updateTags.isError]); - // This is the source of truth that represents the current state of tags in - // this Taxonomy as a tree. Whenever either the `appliedContentTags` (i.e. tags passed in - // the prop from the backed) change, or when the `addedContentTags` (i.e. tags added by - // selecting/unselecting them in the dropdown) change, the tree is recomputed. - const tagsTree = React.useMemo(() => ( - mergeTrees(appliedContentTags, addedContentTags) - ), [appliedContentTags, addedContentTags]); + /** + * Util function that removes the tag along with its ancestors if it was + * the only explicit child tag. It returns a list of staged tags (and ancestors) that + * were unstaged and should be removed + * + * @param {object} tree - tag tree to remove the tag from + * @param {string[]} tagsToRemove - remaining lineage of tag to remove at each recursive level. + * eg: ['grand parent', 'parent', 'tag'] + * @param {boolean} staged - whether we are removing staged tags or not + * @param {string[]} fullLineage - Full lineage of tag being removed + * @returns {string[]} array of staged tag values (with ancestors) that should be removed from staged tree + * + */ + const removeTags = React.useCallback((tree, tagsToRemove, staged, fullLineage) => { + const removedTags = []; + + const traverseAndRemoveTags = (subTree, innerTagsToRemove) => { + if (!subTree || !innerTagsToRemove.length) { + return; + } + const key = innerTagsToRemove[0]; + if (subTree[key]) { + traverseAndRemoveTags(subTree[key].children, innerTagsToRemove.slice(1)); + + if ( + Object.keys(subTree[key].children).length === 0 + && (subTree[key].explicit === false || innerTagsToRemove.length === 1) + ) { + // eslint-disable-next-line no-param-reassign + delete subTree[key]; + + // Remove tags (including ancestors) from staged tags select menu + if (staged) { + // Build value from lineage by traversing beginning till key, then encoding them + const toRemove = fullLineage.slice(0, fullLineage.indexOf(key) + 1).map(item => encodeURIComponent(item)); + if (toRemove.length > 0) { + removedTags.push(toRemove.join(',')); + } + } + } + } + }; + + traverseAndRemoveTags(tree, tagsToRemove); + + return removedTags; + }, []); // Add tag to the tree, and while traversing remove any selected ancestor tags // as they should become implicit @@ -163,6 +235,10 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { tagLineage.forEach(tag => { const isExplicit = selectedTag === tag; + // Clear out the ancestor tags leading to newly selected tag + // as they automatically become implicit + value.push(encodeURIComponent(tag)); + if (!traversal[tag]) { traversal[tag] = { explicit: isExplicit, @@ -174,12 +250,8 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { traversal[tag].explicit = isExplicit; } - // Clear out the ancestor tags leading to newly selected tag - // as they automatically become implicit - value.push(encodeURIComponent(tag)); // eslint-disable-next-line no-unused-expressions isExplicit ? add(value.join(',')) : remove(value.join(',')); - traversal = traversal[tag].children; }); }; @@ -188,26 +260,62 @@ const useContentTagsCollapsibleHelper = (contentId, taxonomyAndTagsData) => { const tagLineage = tagSelectableBoxValue.split(',').map(t => decodeURIComponent(t)); const selectedTag = tagLineage.slice(-1)[0]; - const addedTree = { ...addedContentTags }; if (checked) { + const stagedTree = cloneDeep(stagedContentTagsTree); // We "add" the tag to the SelectableBox.Set inside the addTags method - addTags(addedTree, tagLineage, selectedTag); + addTags(stagedTree, tagLineage, selectedTag); + + // Update the staged content tags tree + setStagedContentTagsTree(stagedTree); + + // Add content tag to taxonomy's staged tags select menu + addStagedContentTag( + id, + { + value: tagSelectableBoxValue, + label: selectedTag, + }, + ); } else { // Remove tag from the SelectableBox.Set remove(tagSelectableBoxValue); - // We remove them from both incase we are unselecting from an - // existing applied Tag or a newly added one - removeTags(addedTree, tagLineage); - removeTags(appliedContentTags, tagLineage); + // Remove tag along with it's from ancestors if it's the only child tag + // from the staged tags tree and update the staged content tags tree + setStagedContentTagsTree(prevStagedContentTagsTree => { + const updatedStagedContentTagsTree = cloneDeep(prevStagedContentTagsTree); + const tagsToRemove = removeTags(updatedStagedContentTagsTree, tagLineage, true, tagLineage); + setStagedTagsToRemove(tagsToRemove); + return updatedStagedContentTagsTree; + }); } + }, [ + stagedContentTagsTree, setStagedContentTagsTree, addTags, removeTags, + id, addStagedContentTag, removeStagedContentTag, + ]); - setAddedContentTags(addedTree); - setUpdatingTags(true); - }, []); + const removeAppliedTagHandler = React.useCallback((tagSelectableBoxValue) => { + const tagLineage = tagSelectableBoxValue.split(',').map(t => decodeURIComponent(t)); + + // Remove tag from the SelectableBox.Set + remove(tagSelectableBoxValue); + + // Remove tags from applied tags + const tagsToRemove = removeTags(appliedContentTagsTree, tagLineage, false, tagLineage); + setStagedTagsToRemove(tagsToRemove); + + setRemoveAppliedTag(true); + }, [appliedContentTagsTree, id, removeStagedContentTag]); return { - tagChangeHandler, tagsTree, contentTagsCount, checkedTags, + tagChangeHandler, + removeAppliedTagHandler, + appliedContentTagsTree: sortKeysAlphabetically(appliedContentTagsTree), + stagedContentTagsTree: sortKeysAlphabetically(stagedContentTagsTree), + contentTagsCount, + checkedTags, + commitStagedTags, + updateTags, }; }; diff --git a/src/content-tags-drawer/ContentTagsDrawer.jsx b/src/content-tags-drawer/ContentTagsDrawer.jsx index c117f6fd2..853930b68 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.jsx @@ -1,5 +1,10 @@ // @ts-check -import React, { useMemo, useEffect } from 'react'; +import React, { + useMemo, + useEffect, + useState, + useCallback, +} from 'react'; import PropTypes from 'prop-types'; import { Container, @@ -40,6 +45,32 @@ const ContentTagsDrawer = ({ id, onClose }) => { const org = extractOrgFromContentId(contentId); + const [stagedContentTags, setStagedContentTags] = useState({}); + + // Add a content tags to the staged tags for a taxonomy + const addStagedContentTag = useCallback((taxonomyId, addedTag) => { + setStagedContentTags(prevStagedContentTags => { + const updatedStagedContentTags = { + ...prevStagedContentTags, + [taxonomyId]: [...(prevStagedContentTags[taxonomyId] ?? []), addedTag], + }; + return updatedStagedContentTags; + }); + }, [setStagedContentTags]); + + // Remove a content tag from the staged tags for a taxonomy + const removeStagedContentTag = useCallback((taxonomyId, tagValue) => { + setStagedContentTags(prevStagedContentTags => ({ + ...prevStagedContentTags, + [taxonomyId]: prevStagedContentTags[taxonomyId].filter((t) => t.value !== tagValue), + })); + }, [setStagedContentTags]); + + // Sets the staged content tags for taxonomy to the provided list of tags + const setStagedTags = useCallback((taxonomyId, tagsList) => { + setStagedContentTags(prevStagedContentTags => ({ ...prevStagedContentTags, [taxonomyId]: tagsList })); + }, [setStagedContentTags]); + const useTaxonomyListData = () => { const taxonomyListData = useTaxonomyListDataResponse(org); const isTaxonomyListLoaded = useIsTaxonomyListDataLoaded(org); @@ -122,7 +153,14 @@ const ContentTagsDrawer = ({ id, onClose }) => { { isTaxonomyListLoaded && isContentTaxonomyTagsLoaded ? taxonomies.map((data) => (
- +
)) diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 0f7f1815a..37fd2343f 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -8,8 +8,10 @@ import ContentTagsDrawer from './ContentTagsDrawer'; import { useContentTaxonomyTagsData, useContentData, + useTaxonomyTagsData, } from './data/apiHooks'; import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from '../taxonomy/data/apiHooks'; +import messages from './messages'; const contentId = 'block-v1:SampleTaxonomyOrg1+STC1+2023_1+type@vertical+block@7f47fe2dbcaf47c5a071671c741fe1ab'; const mockOnClose = jest.fn(); @@ -33,6 +35,15 @@ jest.mock('./data/apiHooks', () => ({ useContentTaxonomyTagsUpdater: jest.fn(() => ({ isError: false, })), + useTaxonomyTagsData: jest.fn(() => ({ + hasMorePages: false, + tagPages: { + isLoading: true, + isError: false, + canAddTag: false, + data: [], + }, + })), })); jest.mock('../taxonomy/data/apiHooks', () => ({ @@ -47,6 +58,82 @@ const RootWrapper = (params) => ( ); describe('', () => { + const setupMockDataForStagedTagsTesting = () => { + useIsTaxonomyListDataLoaded.mockReturnValue(true); + useContentTaxonomyTagsData.mockReturnValue({ + isSuccess: true, + data: { + taxonomies: [ + { + name: 'Taxonomy 1', + taxonomyId: 123, + canTagObject: true, + tags: [ + { + value: 'Tag 1', + lineage: ['Tag 1'], + canDeleteObjecttag: true, + }, + { + value: 'Tag 2', + lineage: ['Tag 2'], + canDeleteObjecttag: true, + }, + ], + }, + ], + }, + }); + useTaxonomyListDataResponse.mockReturnValue({ + results: [{ + id: 123, + name: 'Taxonomy 1', + description: 'This is a description 1', + canTagObject: true, + }], + }); + + useTaxonomyTagsData.mockReturnValue({ + hasMorePages: false, + canAddTag: false, + tagPages: { + isLoading: false, + isError: false, + data: [{ + value: 'Tag 1', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12345, + subTagsUrl: null, + canChangeTag: false, + canDeleteTag: false, + }, { + value: 'Tag 2', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12346, + subTagsUrl: null, + canChangeTag: false, + canDeleteTag: false, + }, { + value: 'Tag 3', + externalId: null, + childCount: 0, + depth: 0, + parentValue: null, + id: 12347, + subTagsUrl: null, + canChangeTag: false, + canDeleteTag: false, + }], + }, + }); + }; + it('should render page and page title correctly', () => { const { getByText } = render(); expect(getByText('Manage tags')).toBeInTheDocument(); @@ -154,6 +241,101 @@ describe('', () => { }); }); + it('should test adding a content tag to the staged tags for a taxonomy', () => { + setupMockDataForStagedTagsTesting(); + + const { container, getByText, getAllByText } = render(); + + // Expand the Taxonomy to view applied tags and "Add a tag" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on "Add a tag" button to open dropdown + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); + + // Tag 3 should only appear in dropdown selector, (i.e. the dropdown is open, since Tag 3 is not applied) + expect(getAllByText('Tag 3').length).toBe(1); + + // Click to check Tag 3 + const tag3 = getByText('Tag 3'); + fireEvent.click(tag3); + + // Check that Tag 3 has been staged, i.e. there should be 2 of them on the page + expect(getAllByText('Tag 3').length).toBe(2); + }); + + it('should test removing a staged content from a taxonomy', () => { + setupMockDataForStagedTagsTesting(); + + const { container, getByText, getAllByText } = render(); + + // Expand the Taxonomy to view applied tags and "Add a tag" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on "Add a tag" button to open dropdown + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); + + // Tag 3 should only appear in dropdown selector, (i.e. the dropdown is open, since Tag 3 is not applied) + expect(getAllByText('Tag 3').length).toBe(1); + + // Click to check Tag 3 + const tag3 = getByText('Tag 3'); + fireEvent.click(tag3); + + // Check that Tag 3 has been staged, i.e. there should be 2 of them on the page + expect(getAllByText('Tag 3').length).toBe(2); + + // Click it again to unstage it and confirm that there is only one on the page + fireEvent.click(tag3); + expect(getAllByText('Tag 3').length).toBe(1); + }); + + it('should test clearing staged tags for a taxonomy', () => { + setupMockDataForStagedTagsTesting(); + + const { + container, + getByText, + getAllByText, + queryByText, + } = render(); + + // Expand the Taxonomy to view applied tags and "Add a tag" button + const expandToggle = container.getElementsByClassName('collapsible-trigger')[0]; + + fireEvent.click(expandToggle); + + // Click on "Add a tag" button to open dropdown + const addTagsButton = getByText(messages.collapsibleAddTagsPlaceholderText.defaultMessage); + // Use `mouseDown` instead of `click` since the react-select didn't respond to `click` + fireEvent.mouseDown(addTagsButton); + + // Tag 3 should only appear in dropdown selector, (i.e. the dropdown is open, since Tag 3 is not applied) + expect(getAllByText('Tag 3').length).toBe(1); + + // Click to check Tag 3 + const tag3 = getByText('Tag 3'); + fireEvent.click(tag3); + + // Check that Tag 3 has been staged, i.e. there should be 2 of them on the page + expect(getAllByText('Tag 3').length).toBe(2); + + // Click on the Cancel button in the dropdown to clear the staged tags + const dropdownCancel = getByText(messages.collapsibleCancelStagedTagsButtonText.defaultMessage); + fireEvent.click(dropdownCancel); + + // Check that there are no more Tag 3 on the page, since the staged one is cleared + // and the dropdown has been closed + expect(queryByText('Tag 3')).not.toBeInTheDocument(); + }); + it('should call closeManageTagsDrawer when CloseButton is clicked', async () => { const postMessageSpy = jest.spyOn(window.parent, 'postMessage'); diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx index 801597786..1cb622996 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx @@ -7,10 +7,9 @@ import { } from '@openedx/paragon'; import { SelectableBox } from '@edx/frontend-lib-content-components'; import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; -import { ArrowDropDown, ArrowDropUp } from '@openedx/paragon/icons'; +import { ArrowDropDown, ArrowDropUp, Add } from '@openedx/paragon/icons'; import PropTypes from 'prop-types'; import messages from './messages'; -import './ContentTagsDropDownSelector.scss'; import { useTaxonomyTagsData } from './data/apiHooks'; @@ -42,7 +41,7 @@ HighlightedText.defaultProps = { }; const ContentTagsDropDownSelector = ({ - taxonomyId, level, lineage, tagsTree, searchTerm, + taxonomyId, level, lineage, appliedContentTagsTree, stagedContentTagsTree, searchTerm, }) => { const intl = useIntl(); @@ -89,13 +88,30 @@ const ContentTagsDropDownSelector = ({ }; const isImplicit = (tag) => { - // Traverse the tags tree using the lineage - let traversal = tagsTree; + // Traverse the applied tags tree using the lineage + let appliedTraversal = appliedContentTagsTree; lineage.forEach(t => { - traversal = traversal[t]?.children || {}; + appliedTraversal = appliedTraversal[t]?.children || {}; }); + const isAppliedImplicit = (appliedTraversal[tag.value] && !appliedTraversal[tag.value].explicit); - return (traversal[tag.value] && !traversal[tag.value].explicit) || false; + // Traverse the staged tags tree using the lineage + let stagedTraversal = stagedContentTagsTree; + lineage.forEach(t => { + stagedTraversal = stagedTraversal[t]?.children || {}; + }); + const isStagedImplicit = (stagedTraversal[tag.value] && !stagedTraversal[tag.value].explicit); + + return isAppliedImplicit || isStagedImplicit || false; + }; + + const isApplied = (tag) => { + // Traverse the applied tags tree using the lineage + let appliedTraversal = appliedContentTagsTree; + lineage.forEach(t => { + appliedTraversal = appliedTraversal[t]?.children || {}; + }); + return !!appliedTraversal[tag.value]; }; const loadMoreTags = useCallback(() => { @@ -131,8 +147,8 @@ const ContentTagsDropDownSelector = ({ aria-label={intl.formatMessage(messages.taxonomyTagsCheckboxAriaLabel, { tag: tagData.value })} data-selectable-box="taxonomy-tags" value={[...lineage, tagData.value].map(t => encodeURIComponent(t)).join(',')} - isIndeterminate={isImplicit(tagData)} - disabled={isImplicit(tagData)} + isIndeterminate={isApplied(tagData) || isImplicit(tagData)} + disabled={isApplied(tagData) || isImplicit(tagData)} > @@ -156,7 +172,8 @@ const ContentTagsDropDownSelector = ({ taxonomyId={taxonomyId} level={level + 1} lineage={[...lineage, tagData.value]} - tagsTree={tagsTree} + appliedContentTagsTree={appliedContentTagsTree} + stagedContentTagsTree={stagedContentTagsTree} searchTerm={searchTerm} /> )} @@ -166,11 +183,12 @@ const ContentTagsDropDownSelector = ({ { hasMorePages ? ( -
+
@@ -197,7 +215,13 @@ ContentTagsDropDownSelector.propTypes = { taxonomyId: PropTypes.number.isRequired, level: PropTypes.number.isRequired, lineage: PropTypes.arrayOf(PropTypes.string), - tagsTree: PropTypes.objectOf( + appliedContentTagsTree: PropTypes.objectOf( + PropTypes.shape({ + explicit: PropTypes.bool.isRequired, + children: PropTypes.shape({}).isRequired, + }).isRequired, + ).isRequired, + stagedContentTagsTree: PropTypes.objectOf( PropTypes.shape({ explicit: PropTypes.bool.isRequired, children: PropTypes.shape({}).isRequired, diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.scss b/src/content-tags-drawer/ContentTagsDropDownSelector.scss index 4a3541e10..4c32ddb4d 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.scss +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.scss @@ -4,11 +4,24 @@ .taxonomy-tags-load-more-button { flex: 1; + + &:hover { + background-color: transparent; + color: $info-900 !important; + } } .pgn__selectable_box.taxonomy-tags-selectable-box { box-shadow: none; padding: 0; + + // Override indeterminate [-] (implicit) checkbox styles to match checked checkbox styles + // In the future, this customizability should be implemented in paragon instead + input.pgn__form-checkbox-input { + &:indeterminate { + @extend :checked; /* stylelint-disable-line scss/at-extend-no-missing-placeholder */ + } + } } .pgn__selectable_box.taxonomy-tags-selectable-box:disabled, diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx index 7800e99e7..ee067aa69 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.test.jsx @@ -25,10 +25,12 @@ const data = { taxonomyId: 123, level: 0, tagsTree: {}, + appliedContentTagsTree: {}, + stagedContentTagsTree: {}, }; const ContentTagsDropDownSelectorComponent = ({ - taxonomyId, level, lineage, tagsTree, searchTerm, + taxonomyId, level, lineage, tagsTree, searchTerm, appliedContentTagsTree, stagedContentTagsTree, }) => ( ); @@ -53,15 +57,25 @@ describe('', () => { jest.clearAllMocks(); }); + async function getComponent(updatedData) { + const componentData = (!updatedData ? data : updatedData); + + return render( + , + ); + } + it('should render taxonomy tags drop down selector loading with spinner', async () => { await act(async () => { - const { getByRole } = render( - , - ); + const { getByRole } = await getComponent(); const spinner = getByRole('status'); expect(spinner.textContent).toEqual('Loading tags'); // Uses }); @@ -86,14 +100,8 @@ describe('', () => { }); await act(async () => { - const { container, getByText } = render( - , - ); + const { container, getByText } = await getComponent(); + await waitFor(() => { expect(getByText('Tag 1')).toBeInTheDocument(); expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(0); @@ -120,13 +128,8 @@ describe('', () => { }); await act(async () => { - const { container, getByText } = render( - , - ); + const { container, getByText } = await getComponent(); + await waitFor(() => { expect(getByText('Tag 2')).toBeInTheDocument(); expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(1); @@ -162,13 +165,7 @@ describe('', () => { }, }, }; - const { container, getByText } = render( - , - ); + const { container, getByText } = await getComponent(dataWithTagsTree); await waitFor(() => { expect(getByText('Tag 2')).toBeInTheDocument(); expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(1); @@ -230,13 +227,7 @@ describe('', () => { }, }, }; - const { container, getByText } = render( - , - ); + const { container, getByText } = await getComponent(dataWithTagsTree); await waitFor(() => { expect(getByText('Tag 2')).toBeInTheDocument(); expect(container.getElementsByClassName('taxonomy-tags-arrow-drop-down').length).toBe(1); @@ -291,15 +282,7 @@ describe('', () => { const initalSearchTerm = 'test 1'; await act(async () => { - const { rerender } = render( - , - ); + const { rerender } = await getComponent({ ...data, searchTerm: initalSearchTerm }); await waitFor(() => { expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, initalSearchTerm); @@ -312,6 +295,8 @@ describe('', () => { level={data.level} tagsTree={data.tagsTree} searchTerm={updatedSearchTerm} + appliedContentTagsTree={{}} + stagedContentTagsTree={{}} />); await waitFor(() => { @@ -326,6 +311,8 @@ describe('', () => { level={data.level} tagsTree={data.tagsTree} searchTerm={cleanSearchTerm} + appliedContentTagsTree={{}} + stagedContentTagsTree={{}} />); await waitFor(() => { @@ -347,15 +334,7 @@ describe('', () => { const searchTerm = 'uncommon search term'; await act(async () => { - const { getByText } = render( - , - ); + const { getByText } = await getComponent({ ...data, searchTerm }); await waitFor(() => { expect(useTaxonomyTagsData).toBeCalledWith(data.taxonomyId, null, 1, searchTerm); diff --git a/src/content-tags-drawer/TagBubble.jsx b/src/content-tags-drawer/TagBubble.jsx index 50c2b3560..b1b0c9b0b 100644 --- a/src/content-tags-drawer/TagBubble.jsx +++ b/src/content-tags-drawer/TagBubble.jsx @@ -14,7 +14,7 @@ const TagBubble = ({ const handleClick = React.useCallback(() => { if (!implicit && canRemove) { - removeTagHandler(lineage.join(','), false); + removeTagHandler(lineage.join(',')); } }, [implicit, lineage, canRemove, removeTagHandler]); diff --git a/src/content-tags-drawer/messages.js b/src/content-tags-drawer/messages.js index c54e6b7bc..4d67ccc72 100644 --- a/src/content-tags-drawer/messages.js +++ b/src/content-tags-drawer/messages.js @@ -33,6 +33,22 @@ const messages = defineMessages({ id: 'course-authoring.content-tags-drawer.content-tags-collapsible.selectable-box.selection.aria.label', defaultMessage: 'taxonomy tags selection', }, + collapsibleAddTagsPlaceholderText: { + id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.placeholder-text', + defaultMessage: 'Add a tag', + }, + collapsibleAddStagedTagsButtonText: { + id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.save-staged-tags', + defaultMessage: 'Add tags', + }, + collapsibleCancelStagedTagsButtonText: { + id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.cancel-staged-tags', + defaultMessage: 'Cancel', + }, + collapsibleInlineAddStagedTagsButtonText: { + id: 'course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.inline-save-staged-tags', + defaultMessage: 'Add', + }, }); export default messages; diff --git a/src/index.scss b/src/index.scss index feedc9424..79f44950c 100755 --- a/src/index.scss +++ b/src/index.scss @@ -23,3 +23,5 @@ @import "course-outline/CourseOutline"; @import "course-unit/CourseUnit"; @import "course-checklist/CourseChecklist"; +@import "content-tags-drawer/ContentTagsDropDownSelector"; +@import "content-tags-drawer/ContentTagsCollapsible";