From f19b21b5e56c4f34ae045a6f2911801da30935e8 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Tue, 13 Feb 2024 20:48:18 +0300 Subject: [PATCH 01/26] 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. --- package-lock.json | 196 +++++++++++++++++- package.json | 1 + .../ContentTagsCollapsible.jsx | 108 +++++----- .../ContentTagsCollapsibleHelper.jsx | 30 +-- 4 files changed, 267 insertions(+), 68 deletions(-) diff --git a/package-lock.json b/package-lock.json index fa514c9f7a..0e5e3a5d75 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", diff --git a/package.json b/package.json index 2c91d5ae82..7020949cbb 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.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 0f5e12d916..8486e30d5f 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -1,5 +1,6 @@ // @ts-check import React from 'react'; +import Select, { components, MenuProps } from 'react-select'; import { Badge, Collapsible, @@ -22,6 +23,34 @@ import ContentTagsTree from './ContentTagsTree'; import useContentTagsCollapsibleHelper from './ContentTagsCollapsibleHelper'; +const CustomMenu = (props) => { + const { intl, handleSelectableBoxChange, checkedTags, taxonomyId, tagsTree, searchTerm } = props.selectProps; + return ( + +
+ + + + +
+ + ); +}; + /** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ /** @typedef {import("./data/types.mjs").Tag} ContentTagData */ @@ -102,15 +131,12 @@ import useContentTagsCollapsibleHelper from './ContentTagsCollapsibleHelper'; */ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData }) => { const intl = useIntl(); - const { id, name, canTagObject } = taxonomyAndTagsData; + const { id: taxonomyId, name, canTagObject } = taxonomyAndTagsData; const { tagChangeHandler, tagsTree, contentTagsCount, checkedTags, } = useContentTagsCollapsibleHelper(contentId, taxonomyAndTagsData); - const [isOpen, open, close] = useToggle(false); - const [addTagsButtonRef, setAddTagsButtonRef] = React.useState(null); - const [searchTerm, setSearchTerm] = React.useState(''); const handleSelectableBoxChange = React.useCallback((e) => { @@ -121,7 +147,7 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData }) => { setSearchTerm(term.trim()); }, 500); // Perform search after 500ms - const handleSearchChange = React.useCallback((value) => { + const handleSearchChange = React.useCallback((value, { action, prevInputValue }) => { if (value === '') { // No need to debounce when search term cleared. Clear debounce function handleSearch.cancel(); @@ -131,66 +157,40 @@ const ContentTagsCollapsible = ({ contentId, taxonomyAndTagsData }) => { } }, []); - const modalPopupOnCloseHandler = React.useCallback((event) => { - close(event); - // Clear search term - setSearchTerm(''); - }, []); - return (
-
+
{canTagObject && ( - + Date: Tue, 27 Feb 2024 19:03:50 +0300 Subject: [PATCH 09/26] feat: Implement cancel button + add proptypes --- .../ContentTagsCollapsible.jsx | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 0810250ee9..07d866c03b 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -1,6 +1,6 @@ // @ts-check import React from 'react'; -import Select, { components, MenuProps } from 'react-select'; +import Select, { components } from 'react-select'; import { Badge, Collapsible, @@ -12,8 +12,7 @@ import { import PropTypes from 'prop-types'; import classNames from 'classnames'; import { SelectableBox } from '@edx/frontend-lib-content-components'; -import { cloneDeep } from 'lodash'; -import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; +import { useIntl, intlShape } from '@edx/frontend-platform/i18n'; import { debounce } from 'lodash'; import messages from './messages'; import './ContentTagsCollapsible.scss'; @@ -32,6 +31,8 @@ const CustomMenu = (props) => { taxonomyId, appliedContentTagsTree, stagedContentTagsTree, + handleStagedTagsMenuChange, + selectRef, searchTerm, value, } = props.selectProps; @@ -61,7 +62,7 @@ const CustomMenu = (props) => {
@@ -79,7 +80,34 @@ const CustomMenu = (props) => { ); }; -// TODO: Add props validation for CustomMenu +CustomMenu.propTypes = { + selectProps: PropTypes.shape({ + intl: intlShape.isRequired, + handleSelectableBoxChange: PropTypes.func.isRequired, + checkedTags: PropTypes.arrayOf(PropTypes.string).isRequired, + taxonomyId: PropTypes.number.isRequired, + 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, + }).isRequired, + ).isRequired, + handleStagedTagsMenuChange: PropTypes.func.isRequired, + // eslint-disable-next-line react/forbid-prop-types + selectRef: PropTypes.shape({ current: PropTypes.object }), + searchTerm: PropTypes.string.isRequired, + value: PropTypes.arrayOf(PropTypes.shape({ + value: PropTypes.string.isRequired, + label: PropTypes.string.isRequired, + })).isRequired, + }).isRequired, +}; /** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ /** @typedef {import("./data/types.mjs").Tag} ContentTagData */ @@ -168,6 +196,7 @@ const ContentTagsCollapsible = ({ }) => { const intl = useIntl(); const { id: taxonomyId, name, canTagObject } = taxonomyAndTagsData; + const selectRef = React.useRef(null); const { tagChangeHandler, appliedContentTagsTree, stagedContentTagsTree, contentTagsCount, checkedTags, @@ -227,6 +256,7 @@ const ContentTagsCollapsible = ({ {canTagObject && ( )}
diff --git a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx index 363e9eabc7..2519adba92 100644 --- a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx @@ -83,13 +83,14 @@ const useContentTagsCollapsibleHelper = ( 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) @@ -100,35 +101,41 @@ const useContentTagsCollapsibleHelper = ( 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 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 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))); + const staged = stagedContentTags.map(t => t.label); + const remainingAppliedTags = tags.filter(t => !staged.includes(t)); + + updateTags.mutate({ tags: remainingAppliedTags }); + } + }, [contentId, id, canTagObject, checkedTags, stagedContentTags]); - // ================================================================= + // Handles making requests to the update endpoint when the staged tags need to be committed + const commitStagedTags = React.useCallback(() => { + const staged = stagedContentTags.map(t => t.label); - // TODO: Properly implement this based on feature/requirements + const stagedLineages = stagedContentTags.map(st => decodeURIComponent(st.value).split(',').slice(0, -1)).flat(); - // // Handles making requests to the update endpoint whenever the checked tags change - // 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); - // const tags = checkedTags.map(t => decodeURIComponent(t.split(',').slice(-1))); - // updateTags.mutate({ tags }); - // } - // }, [contentId, id, canTagObject, checkedTags]); + // Filter out applied tags that should become implicit because a child tag was committed + const applied = contentTags.map((t) => t.value).filter(t => !stagedLineages.includes(t)); - // ================================================================== + updateTags.mutate({ tags: [...applied, ...staged] }); + }, [contentTags, stagedContentTags, updateTags]); // This converts the contentTags prop to the tree structure mentioned above 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 - setStagedContentTagsTree({}); - 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) { @@ -172,6 +179,10 @@ const useContentTagsCollapsibleHelper = ( 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, @@ -181,19 +192,11 @@ const useContentTagsCollapsibleHelper = ( }; } else { traversal[tag].explicit = isExplicit; - } - - // Clear out the ancestor tags leading to newly selected tag - // as they automatically become implicit - value.push(encodeURIComponent(tag)); - - if (isExplicit) { - add(value.join(',')); - } else { removeStagedContentTag(id, value.join(',')); - remove(value.join(',')); } + // eslint-disable-next-line no-unused-expressions + isExplicit ? add(value.join(',')) : remove(value.join(',')); traversal = traversal[tag].children; }); }; @@ -222,6 +225,9 @@ const useContentTagsCollapsibleHelper = ( // Remove tag from the SelectableBox.Set remove(tagSelectableBoxValue); + // Remove tags from applied tags + removeTags(appliedContentTagsTree, 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 => { @@ -233,19 +239,32 @@ const useContentTagsCollapsibleHelper = ( // Remove content tag from taxonomy's staged tags select menu removeStagedContentTag(id, tagSelectableBoxValue); } - - // setUpdatingTags(true); }, [ - stagedContentTagsTree, setStagedContentTagsTree, addTags, removeTags, removeTags, + stagedContentTagsTree, setStagedContentTagsTree, addTags, removeTags, id, addStagedContentTag, removeStagedContentTag, ]); + 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 + removeTags(appliedContentTagsTree, tagLineage); + + setRemoveAppliedTag(true); + }, [checkedTags]); + return { tagChangeHandler, + removeAppliedTagHandler, appliedContentTagsTree: sortKeysAlphabetically(appliedContentTagsTree), stagedContentTagsTree: sortKeysAlphabetically(stagedContentTagsTree), contentTagsCount, checkedTags, + commitStagedTags, + updateTags, }; }; diff --git a/src/content-tags-drawer/TagBubble.jsx b/src/content-tags-drawer/TagBubble.jsx index 50c2b3560e..b1b0c9b0ba 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]); From b21e1c80da89333f89500afd37e3d73d1a4368b8 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Thu, 29 Feb 2024 14:39:41 +0300 Subject: [PATCH 11/26] feat: Keep all staged tags only commit explicit --- .../ContentTagsCollapsibleHelper.jsx | 116 +++++++++--------- 1 file changed, 59 insertions(+), 57 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx index 2519adba92..be4e708472 100644 --- a/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsibleHelper.jsx @@ -9,7 +9,7 @@ import { useContentTaxonomyTagsUpdater } from './data/apiHooks'; * Util function that sorts the keys of a tree in alphabetical order. * * @param {object} tree - tree that needs it's keys sorted - * @returns {object} merged tree containing both tree1 and tree2 + * @returns {object} sorted tree */ const sortKeysAlphabetically = (tree) => { const sortedObj = {}; @@ -25,54 +25,28 @@ const sortKeysAlphabetically = (tree) => { }; /** - * Util function that consolidates two tag trees into one, sorting the keys in - * alphabetical order. + * Util function that returns the leafs of a tree. Mainly used to extract the explicit + * tags selected in the staged tags tree * - * @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 to extract the leaf tags from + * @returns {Array} array of leaf (explicit) tags of provided tree */ -const mergeTrees = (tree1, tree2) => { - const mergedTree = cloneDeep(tree1); - - 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); +const getLeafTags = (tree) => { + const leafKeys = []; + + function traverse(node) { + Object.keys(node).forEach(key => { + const child = node[key]; + if (Object.keys(child.children).length === 0) { + leafKeys.push(key); } else { - // eslint-disable-next-line no-param-reassign - destination[key] = cloneDeep(sourceValue); + traverse(child.children); } }); - }; - - mergeRecursively(mergedTree, tree2); - return sortKeysAlphabetically(mergedTree); -}; - -/** - * Util function that removes the tag along with its ancestors if it was - * the only explicit child tag. - * - * @param {object} tree - tag tree to remove the tag from - * @param {string[]} tagsToRemove - full lineage of tag to remove. - * eg: ['grand parent', 'parent', 'tag'] - */ -const removeTags = (tree, tagsToRemove) => { - if (!tree || !tagsToRemove.length) { - return; } - const key = tagsToRemove[0]; - if (tree[key]) { - removeTags(tree[key].children, tagsToRemove.slice(1)); - 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]; - } - } + traverse(tree); + return leafKeys; }; /* @@ -122,15 +96,15 @@ const useContentTagsCollapsibleHelper = ( // Handles making requests to the update endpoint when the staged tags need to be committed const commitStagedTags = React.useCallback(() => { - const staged = stagedContentTags.map(t => t.label); - - const stagedLineages = stagedContentTags.map(st => decodeURIComponent(st.value).split(',').slice(0, -1)).flat(); + // 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, ...staged] }); - }, [contentTags, stagedContentTags, updateTags]); + updateTags.mutate({ tags: [...applied, ...explicitStaged] }); + }, [contentTags, stagedContentTags, stagedContentTagsTree, updateTags]); // This converts the contentTags prop to the tree structure mentioned above const appliedContentTagsTree = React.useMemo(() => { @@ -171,6 +145,41 @@ const useContentTagsCollapsibleHelper = ( return resultTree; }, [contentTags, updateTags.isError]); + /** + * Util function that removes the tag along with its ancestors if it was + * the only explicit child tag. It also unstages tags (and ancestors) as they are 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 + * + */ + const removeTags = React.useCallback((tree, tagsToRemove, staged, fullLineage) => { + if (!tree || !tagsToRemove.length) { + return; + } + const key = tagsToRemove[0]; + if (tree[key]) { + removeTags(tree[key].children, tagsToRemove.slice(1), staged, fullLineage); + + 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]; + + // 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) { + removeStagedContentTag(id, toRemove.join(',')); + } + } + } + } + }, [id, removeStagedContentTag]); + // Add tag to the tree, and while traversing remove any selected ancestor tags // as they should become implicit const addTags = (tree, tagLineage, selectedTag) => { @@ -192,7 +201,6 @@ const useContentTagsCollapsibleHelper = ( }; } else { traversal[tag].explicit = isExplicit; - removeStagedContentTag(id, value.join(',')); } // eslint-disable-next-line no-unused-expressions @@ -225,19 +233,13 @@ const useContentTagsCollapsibleHelper = ( // Remove tag from the SelectableBox.Set remove(tagSelectableBoxValue); - // Remove tags from applied tags - removeTags(appliedContentTagsTree, 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); - removeTags(updatedStagedContentTagsTree, tagLineage); + removeTags(updatedStagedContentTagsTree, tagLineage, true, tagLineage); return updatedStagedContentTagsTree; }); - - // Remove content tag from taxonomy's staged tags select menu - removeStagedContentTag(id, tagSelectableBoxValue); } }, [ stagedContentTagsTree, setStagedContentTagsTree, addTags, removeTags, @@ -251,10 +253,10 @@ const useContentTagsCollapsibleHelper = ( remove(tagSelectableBoxValue); // Remove tags from applied tags - removeTags(appliedContentTagsTree, tagLineage); + removeTags(appliedContentTagsTree, tagLineage, false, tagLineage); setRemoveAppliedTag(true); - }, [checkedTags]); + }, [appliedContentTagsTree, id, removeStagedContentTag]); return { tagChangeHandler, From 42f606a9b3f95c6073d5f26edb4c8856b5744744 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Thu, 29 Feb 2024 15:48:33 +0300 Subject: [PATCH 12/26] feat: Change style of add/cancel/load more buttons --- src/content-tags-drawer/ContentTagsCollapsible.jsx | 4 +++- src/content-tags-drawer/ContentTagsCollapsible.scss | 10 ++++++++++ .../ContentTagsDropDownSelector.jsx | 9 +++++---- .../ContentTagsDropDownSelector.scss | 5 +++++ src/index.scss | 1 + 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 100d7d6f4a..9017d26ae6 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -13,7 +13,6 @@ import { SelectableBox } from '@edx/frontend-lib-content-components'; import { useIntl, intlShape } from '@edx/frontend-platform/i18n'; import { debounce } from 'lodash'; import messages from './messages'; -import './ContentTagsCollapsible.scss'; import ContentTagsDropDownSelector from './ContentTagsDropDownSelector'; @@ -57,16 +56,19 @@ const CustomMenu = (props) => { searchTerm={searchTerm} /> +
diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.scss b/src/content-tags-drawer/ContentTagsDropDownSelector.scss index ee10e3365a..4c32ddb4dd 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.scss +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.scss @@ -4,6 +4,11 @@ .taxonomy-tags-load-more-button { flex: 1; + + &:hover { + background-color: transparent; + color: $info-900 !important; + } } .pgn__selectable_box.taxonomy-tags-selectable-box { diff --git a/src/index.scss b/src/index.scss index 4ba8b57199..79f44950cc 100755 --- a/src/index.scss +++ b/src/index.scss @@ -24,3 +24,4 @@ @import "course-unit/CourseUnit"; @import "course-checklist/CourseChecklist"; @import "content-tags-drawer/ContentTagsDropDownSelector"; +@import "content-tags-drawer/ContentTagsCollapsible"; From 972a01d9bf12bc58831ec645060dfd8484dd99ec Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Sat, 2 Mar 2024 12:15:42 +0300 Subject: [PATCH 13/26] 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. --- .../ContentTagsCollapsible.jsx | 92 +++++++++++++++---- 1 file changed, 72 insertions(+), 20 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 9017d26ae6..0e9127b22b 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -28,11 +28,10 @@ const CustomMenu = (props) => { taxonomyId, appliedContentTagsTree, stagedContentTagsTree, - handleStagedTagsMenuChange, - selectRef, + handleCommitStagedTags, + handleCancelStagedTags, searchTerm, value, - commitStagedTags, } = props.selectProps; return ( @@ -62,7 +61,7 @@ const CustomMenu = (props) => { @@ -70,7 +69,7 @@ const CustomMenu = (props) => { variant="tertiary" className="text-info-500 add-tags-button" disabled={!(value && value.length)} - onClick={() => { commitStagedTags(); handleStagedTagsMenuChange([]); selectRef.current?.blur(); }} + onClick={handleCommitStagedTags} > { intl.formatMessage(messages.collapsibleAddStagedTagsButtonText) } @@ -99,15 +98,13 @@ CustomMenu.propTypes = { children: PropTypes.shape({}).isRequired, }).isRequired, ).isRequired, - handleStagedTagsMenuChange: PropTypes.func.isRequired, - // eslint-disable-next-line react/forbid-prop-types - selectRef: PropTypes.shape({ current: PropTypes.object }), + handleCommitStagedTags: PropTypes.func.isRequired, + handleCancelStagedTags: PropTypes.func.isRequired, searchTerm: PropTypes.string.isRequired, value: PropTypes.arrayOf(PropTypes.shape({ value: PropTypes.string.isRequired, label: PropTypes.string.isRequired, })).isRequired, - commitStagedTags: PropTypes.func.isRequired, }).isRequired, }; @@ -128,6 +125,42 @@ CustomLoadingIndicator.propTypes = { }).isRequired, }; +const CustomIndicatorsContainer = (props) => { + const { + value, + handleCommitStagedTags, + } = props.selectProps; + return ( + + { + (value && value.length && ( + + )) || null + } + {props.children} + + ); +}; + +CustomIndicatorsContainer.propTypes = { + selectProps: PropTypes.shape({ + value: PropTypes.arrayOf(PropTypes.shape({ + value: PropTypes.string.isRequired, + label: PropTypes.string.isRequired, + })).isRequired, + handleCommitStagedTags: PropTypes.func.isRequired, + }).isRequired, + children: PropTypes.node.isRequired, +}; + /** @typedef {import("../taxonomy/data/types.mjs").TaxonomyData} TaxonomyData */ /** @typedef {import("./data/types.mjs").Tag} ContentTagData */ @@ -244,13 +277,15 @@ const ContentTagsCollapsible = ({ setSearchTerm(term.trim()); }, 500); // Perform search after 500ms - const handleSearchChange = React.useCallback((value, { action, prevInputValue }) => { - if (value === '') { - // No need to debounce when search term cleared. Clear debounce function - handleSearch.cancel(); - setSearchTerm(''); - } else { - handleSearch(value); + const handleSearchChange = React.useCallback((value, { action }) => { + if (action === 'input-change') { + if (value === '') { + // No need to debounce when search term cleared. Clear debounce function + handleSearch.cancel(); + setSearchTerm(''); + } else { + handleSearch(value); + } } }, []); @@ -272,6 +307,17 @@ const ContentTagsCollapsible = ({ setStagedTags(taxonomyId, stagedTags); }, [taxonomyId, setStagedTags, stagedContentTags, tagChangeHandler]); + const handleCommitStagedTags = React.useCallback(() => { + commitStagedTags(); + handleStagedTagsMenuChange([]); + selectRef.current?.blur(); + }, [commitStagedTags, handleStagedTagsMenuChange, selectRef]); + + const handleCancelStagedTags = React.useCallback(() => { + handleStagedTagsMenuChange([]); + selectRef.current?.blur(); + }, [handleStagedTagsMenuChange, selectRef]); + return (
@@ -294,7 +340,14 @@ const ContentTagsCollapsible = ({ classNamePrefix="react-select" onInputChange={handleSearchChange} onChange={handleStagedTagsMenuChange} - components={{ Menu: CustomMenu, LoadingIndicator: CustomLoadingIndicator }} + components={{ + // @ts-ignore TODO: Properly fix this + Menu: CustomMenu, + // @ts-ignore TODO: Properly fix this + LoadingIndicator: CustomLoadingIndicator, + // @ts-ignore TODO: Properly fix this + IndicatorsContainer: CustomIndicatorsContainer, + }} closeMenuOnSelect={false} blurInputOnSelect={false} intl={intl} @@ -303,11 +356,10 @@ const ContentTagsCollapsible = ({ taxonomyId={taxonomyId} appliedContentTagsTree={appliedContentTagsTree} stagedContentTagsTree={stagedContentTagsTree} - handleStagedTagsMenuChange={handleStagedTagsMenuChange} - selectRef={selectRef} + handleCommitStagedTags={handleCommitStagedTags} + handleCancelStagedTags={handleCancelStagedTags} searchTerm={searchTerm} value={stagedContentTags} - commitStagedTags={commitStagedTags} /> )}
From 00e8a19bb34f93e0a59f1666208fe2510e199b28 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Sun, 3 Mar 2024 16:07:04 +0300 Subject: [PATCH 14/26] fix: Keep applied tag checked when only staged child unchecked --- src/content-tags-drawer/ContentTagsDropDownSelector.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx index 1bc527ac3b..1cb6229966 100644 --- a/src/content-tags-drawer/ContentTagsDropDownSelector.jsx +++ b/src/content-tags-drawer/ContentTagsDropDownSelector.jsx @@ -147,7 +147,7 @@ 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)} + isIndeterminate={isApplied(tagData) || isImplicit(tagData)} disabled={isApplied(tagData) || isImplicit(tagData)} > From 6ca73d1bae082647fffbcfcddec80bf6ba5d99e8 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Sun, 3 Mar 2024 17:46:49 +0300 Subject: [PATCH 15/26] feat: Style add tags widget + staged tags Also clear search term whenever tags are staged/cancelled --- .../ContentTagsCollapsible.jsx | 14 +++++++++---- .../ContentTagsCollapsible.scss | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 0e9127b22b..617826f239 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -278,7 +278,11 @@ const ContentTagsCollapsible = ({ }, 500); // Perform search after 500ms const handleSearchChange = React.useCallback((value, { action }) => { - if (action === 'input-change') { + if (action === 'input-blur') { + // Cancel/clear search if focused away from select input + handleSearch.cancel(); + setSearchTerm(''); + } else if (action === 'input-change') { if (value === '') { // No need to debounce when search term cleared. Clear debounce function handleSearch.cancel(); @@ -311,12 +315,14 @@ const ContentTagsCollapsible = ({ commitStagedTags(); handleStagedTagsMenuChange([]); selectRef.current?.blur(); - }, [commitStagedTags, handleStagedTagsMenuChange, selectRef]); + setSearchTerm(''); + }, [commitStagedTags, handleStagedTagsMenuChange, selectRef, setSearchTerm]); const handleCancelStagedTags = React.useCallback(() => { handleStagedTagsMenuChange([]); selectRef.current?.blur(); - }, [handleStagedTagsMenuChange, selectRef]); + setSearchTerm(''); + }, [handleStagedTagsMenuChange, selectRef, setSearchTerm]); return (
@@ -337,7 +343,7 @@ const ContentTagsCollapsible = ({ placeholder={intl.formatMessage(messages.collapsibleAddTagsPlaceholderText)} isSearchable className="d-flex flex-column flex-fill" - classNamePrefix="react-select" + classNamePrefix="react-select-add-tags" onInputChange={handleSearchChange} onChange={handleStagedTagsMenuChange} components={{ diff --git a/src/content-tags-drawer/ContentTagsCollapsible.scss b/src/content-tags-drawer/ContentTagsCollapsible.scss index 4f1df4763d..67a51a77e3 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.scss +++ b/src/content-tags-drawer/ContentTagsCollapsible.scss @@ -37,3 +37,23 @@ 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; + } +} From e68d08acf9e506b457ab3f5acf33e30dd5ea9139 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Sun, 3 Mar 2024 18:48:33 +0300 Subject: [PATCH 16/26] feat: Fixed some typing errors --- src/content-tags-drawer/ContentTagsCollapsible.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/content-tags-drawer/ContentTagsCollapsible.jsx b/src/content-tags-drawer/ContentTagsCollapsible.jsx index 617826f239..d80bfe4e3f 100644 --- a/src/content-tags-drawer/ContentTagsCollapsible.jsx +++ b/src/content-tags-drawer/ContentTagsCollapsible.jsx @@ -248,7 +248,7 @@ const ContentTagsCollapsible = ({ }) => { const intl = useIntl(); const { id: taxonomyId, name, canTagObject } = taxonomyAndTagsData; - const selectRef = React.useRef(null); + const selectRef = React.useRef(/** @type {HTMLSelectElement | null} */(null)); const { tagChangeHandler, @@ -335,7 +335,7 @@ const ContentTagsCollapsible = ({ {canTagObject && ( Props (selectProps). +declare module 'react-select/base' { + export interface Props< + Option, + IsMulti extends boolean, + Group extends GroupBase