[FC-0036] feat: New "Add Tags" widget #834

yusuf-musleh · 2024-02-13T17:50:46Z

Description

This PR add the new "Add Tags" widget. Which separates out staging tags when checking them and committing (saving) them.

Supporting information

Related Tickets:

Closes [Tagging] New "Add Tags" Widget in Tag Drawer modular-learning#188

Testing instructions

Run you local dev stack.
If you don't already have sample taxonomy/tags data, follow the instructions in this repo to generate sample data: https://github.com/open-craft/taxonomy-sample-data
Navigate to a sample course and open the tags drawer by clicking on "Manage Tags"
Expand some taxonomies and click on the "Add a Tag" select input
Confirm the following:
1. When checking tags in the dropdown, it keeps track of which tags are being newly added (staged). If you presse cancel, no changes are made. If press "Add Tags", the new tags are saved
2. Newly added tags appear as chips in the add tags field, and can be removed by clicking on their X.
3. Existing tags (already applied) that were added before the user clicks "Add Tags" do not appear as chips in the "Add Tags" widget.
4. You can type into the widget to search the tags by keyword
5. The functionality from feat: add search highlight/expand and "no tags" message [FC-0036] #799 still exists. i.e matches are highlighted, parent tags are expanded but only if their child/grandchild matches the search, and a message is displayed if there are no matching tags.
6. You can continue to type to search/filter tags even when there are chips displayed inside the field.
7. Very long tag names wrap to the next line; they are not truncated.
8. "Load More" is only shown if there are more pages of tags and is updated with the new style
9. The styling of checkboxes of implicit tags has changed. Instead of a dash, it is now a checkmark (the transparency remains the same i.e. 40% opacity).
10. The inline "Add" button is only shown if there is at least one newly added tag/chip visible.
11. The second "Add Tags" button at the bottom of the dropdown is disabled until/unless there is at least one newly added tag/chip visible.
12. The dropdown is shown when the "Add Tags" search box or the dropdown itself has focus. If the user clicks off the dropdown onto somewhere else on the tagging drawer, the dropdown will close but the chips will remain in the box and the tags won't be added until/unless the user clicks "Add Tags".
13. If the user selects "cancel" on the dropdown menu, their selections (i.e. chips) are forgotten and the select menu is closed.
14. Existing (previously saved) tags are shown as disabled checkmarks and cannot be removed using the checkboxes in the dropdown "Add Tags" widget. Instead, users click on the (X) next to them to remove them. But newly-added tags (that appear as chips) can be undone by un-checking them in the tag tree dropdown. This is the same as clicking the X on the chip.

Private-ref: FAL-3643

openedx-webhooks · 2024-02-13T17:50:50Z

Thanks for the pull request, @yusuf-musleh! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

src/content-tags-drawer/ContentTagsCollapsible.jsx

src/content-tags-drawer/ContentTagsDrawer.jsx

codecov · 2024-03-04T11:48:33Z

Codecov Report

Attention: Patch coverage is 91.83673% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 91.29%. Comparing base (7f5687f) to head (94581f1).

Files	Patch %	Lines
...ntent-tags-drawer/ContentTagsCollapsibleHelper.jsx	93.33%	5 Missing ⚠️
...ontent-tags-drawer/ContentTagsDropDownSelector.jsx	75.00%	4 Missing ⚠️
src/content-tags-drawer/ContentTagsCollapsible.jsx	93.02%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #834      +/-   ##
==========================================
- Coverage   91.30%   91.29%   -0.01%     
==========================================
  Files         561      561              
  Lines        9768     9849      +81     
  Branches     2081     2099      +18     
==========================================
+ Hits         8919     8992      +73     
- Misses        816      824       +8     
  Partials       33       33

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ChrisChV

@yusuf-musleh Looks good! Very great work! 👍

I tested this: I followed the testing instructions
I read through the code and considered the security, stability and performance implications of the changes.
Includes tests for bugfixes and/or features added.
Includes documentation

yusuf-musleh · 2024-03-05T17:03:27Z

src/content-tags-drawer/ContentTagsCollapsible.jsx

+              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,
+              }}


I couldn't get the types to work properly here, any advice on how to properly fix this?

@yusuf-musleh Sorry for the delay, I'm hoping to check out this PR and answer your question soon :)

@yusuf-musleh I figure out the types for you. I meant to push them to a new branch just now, but accidentally pushed them to this same branch. Sorry about that. Please review and incorporate or not as you wish.

@bradenmacdonald Thanks a lot for fixing the types! It was really tricky getting them to work. There were some more typing issues the linter was complaining about, but I fixed them here: 0200cf7

I also found a react warning in the console caused by setting state of a parent component while updating the state in the child. I spent some time looking into it and refactored it here: cdc2269 seems to be working fine now.

@yusuf-musleh Nice work. This is my first time trying to include a .d.ts file in an Open edX MFE with the same name as the .jsx file, and frankly I'm surprised that it works at all. But it seems to solve the problem :)

yelsayed · 2024-03-06T04:57:22Z

@yusuf-musleh Thanks! I was looking forward to this feature :)

xitij2000

I've noticed a strange thing when using this code.

If I add a tag and then remove tags, the first tag seems to be getting duplicated each time I remove another tag. This seems to happen only if the first tag you added (or the alphabetically first?) is a root tag.

i.e. After adding and removing a few tags when I removed a tag this was the post JSON:

{
	"tags": [
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Abilities",
		"Holland Codes",
		"Foundational Knowledge",
		"Being Responsible",
		"Skills"
	]
}

package.json

Replace existing component with react-select component, by passing in our custom component. This retained the existing search functionality.

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.

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.

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.

This overrides the indeterminate input checkbox style to match the checked checkbox style, using variables defined in paragon.

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.

In the react-select component, an inline "Add" button showsup when some tags are staged, if they are clicked they are commited/applied.

Also clear search term whenever tags are staged/cancelled

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.

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.

yusuf-musleh · 2024-03-13T05:13:32Z

I've noticed a strange thing when using this code.

If I add a tag and then remove tags, the first tag seems to be getting duplicated each time I remove another tag. This seems to happen only if the first tag you added (or the alphabetically first?) is a root tag. ...

@xitij2000 Good catch! I fixed the issue here (234afac). It turns out I forgot to reset the "added" (checked) tags whenever we get the applied tags from the backend, so it would keep adding the remaining existing tags to the selectable box again everytime we delete one (react query would fetch latest tags). In older versions of the code, I would clear all of them, but now since we have staged tags, I only need to clear the applied ones, then only add the latest explicit ones.

openedx-webhooks · 2024-03-13T13:27:33Z

@yusuf-musleh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 13, 2024

yusuf-musleh changed the title ~~[WIP] [FC-0036] feat: Use react-select for tags selector~~ [WIP] [FC-0036] feat: New "Add Tags" widget Feb 13, 2024

yusuf-musleh force-pushed the yusuf-musleh/add-tags-widget branch 3 times, most recently from b3aa084 to 6bfd13b Compare February 21, 2024 06:43

bradenmacdonald reviewed Feb 21, 2024

View reviewed changes

yusuf-musleh force-pushed the yusuf-musleh/add-tags-widget branch 3 times, most recently from da28fb9 to 6fecc42 Compare February 29, 2024 11:40

yusuf-musleh changed the title ~~[WIP] [FC-0036] feat: New "Add Tags" widget~~ [FC-0036] feat: New "Add Tags" widget Mar 4, 2024

ChrisChV approved these changes Mar 4, 2024

View reviewed changes

yusuf-musleh force-pushed the yusuf-musleh/add-tags-widget branch 3 times, most recently from 104b55f to 76de6d5 Compare March 5, 2024 16:46

yusuf-musleh marked this pull request as ready for review March 5, 2024 16:55

yusuf-musleh requested a review from a team as a code owner March 5, 2024 16:55

yusuf-musleh commented Mar 5, 2024

View reviewed changes

yusuf-musleh force-pushed the yusuf-musleh/add-tags-widget branch 2 times, most recently from cdc2269 to 603416c Compare March 12, 2024 03:54

xitij2000 reviewed Mar 12, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

yusuf-musleh added 7 commits March 13, 2024 07:55

feat: Use react-select for tags selector

f19b21b

Replace existing component with react-select component, by passing in our custom component. This retained the existing search functionality.

feat: Add stagedContentTags state in react-select

85b5025

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: Change style of implicit checkbox to checks

80d9220

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

f121042

refactor: Remove cloneDeep + simplify code

70aa9c3

yusuf-musleh and others added 19 commits March 13, 2024 07:55

feat: Update placeholder/button texts

bb783c9

feat: Implement cancel button + add proptypes

45d9ae3

feat: Implement commit/cancel staged tags

f18c650

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

b21e1c8

feat: Change style of add/cancel/load more buttons

42f606a

feat: Add inline "Add" button to commit tags

972a01d

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

00e8a19

feat: Style add tags widget + staged tags

6ca73d1

Also clear search term whenever tags are staged/cancelled

feat: Fixed some typing errors

e68d08a

test: Update tests to fix existing broken cases

9227bf8

test: Add new functionality tests

34cc413

chore: add types to ContentTagsCollapsible

0f2a608

chore: add types for useContentTagsCollapsibleHelper

4c2ba51

fix: Small bug with useIntl

183bf2f

chore: Fix more linter issues

35cd452

chore: Update package-lock.json

c58e012

fix: Reset applied tags in selectbox when fetching

234afac

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

94581f1

yusuf-musleh force-pushed the yusuf-musleh/add-tags-widget branch from 603416c to 94581f1 Compare March 13, 2024 04:58

yusuf-musleh requested a review from a team as a code owner March 13, 2024 04:58

xitij2000 approved these changes Mar 13, 2024

View reviewed changes

xitij2000 merged commit 4e70813 into openedx:master Mar 13, 2024
6 checks passed

bradenmacdonald deleted the yusuf-musleh/add-tags-widget branch July 12, 2024 08:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FC-0036] feat: New "Add Tags" widget #834

[FC-0036] feat: New "Add Tags" widget #834

yusuf-musleh commented Feb 13, 2024 •

edited

Loading

openedx-webhooks commented Feb 13, 2024 •

edited

Loading

codecov bot commented Mar 4, 2024 •

edited

Loading

ChrisChV left a comment

yusuf-musleh Mar 5, 2024

bradenmacdonald Mar 8, 2024

bradenmacdonald Mar 9, 2024

yusuf-musleh Mar 10, 2024

bradenmacdonald Mar 11, 2024 •

edited

Loading

yelsayed commented Mar 6, 2024

xitij2000 left a comment

yusuf-musleh commented Mar 13, 2024

openedx-webhooks commented Mar 13, 2024

[FC-0036] feat: New "Add Tags" widget #834

[FC-0036] feat: New "Add Tags" widget #834

Conversation

yusuf-musleh commented Feb 13, 2024 • edited Loading

Description

Supporting information

Testing instructions

openedx-webhooks commented Feb 13, 2024 • edited Loading

codecov bot commented Mar 4, 2024 • edited Loading

Codecov Report

ChrisChV left a comment

Choose a reason for hiding this comment

yusuf-musleh Mar 5, 2024

Choose a reason for hiding this comment

bradenmacdonald Mar 8, 2024

Choose a reason for hiding this comment

bradenmacdonald Mar 9, 2024

Choose a reason for hiding this comment

yusuf-musleh Mar 10, 2024

Choose a reason for hiding this comment

bradenmacdonald Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

yelsayed commented Mar 6, 2024

xitij2000 left a comment

Choose a reason for hiding this comment

yusuf-musleh commented Mar 13, 2024

openedx-webhooks commented Mar 13, 2024

yusuf-musleh commented Feb 13, 2024 •

edited

Loading

openedx-webhooks commented Feb 13, 2024 •

edited

Loading

codecov bot commented Mar 4, 2024 •

edited

Loading

bradenmacdonald Mar 11, 2024 •

edited

Loading