Tagging UX refinement v2 [FC-0036] #34059

ChrisChV · 2024-01-15T21:03:23Z

Description

This PR fixes the following issues:

the layer over the course outline is too dark when the tag drawer is open.
Refresh the count of tags on the unit/outline page(s) after the user makes edits in the tag drawer
- TagList component updated to allow receiving for the Manage tags drawer and refresh all the tag tree and counts
- TagCount refactored to use javascript to render the component and allow receiving for the Manage tags drawer and refresh the count.
- Update new TagCount in units and components

Supporting information

Internal ticket: FAL-3601
Github issue: openedx/modular-learning#167
Related PR: openedx/frontend-app-authoring#800

Testing instructions

Enable contentstore.enable_copy_paste_units and new_studio_mfe.use_tagging_taxonomy_list_page flags.
Before to all you need to run the taxonomy-sample-data.

1. the layer over the course outline is too dark when the tag drawer is open.

Go to a sample course and go to a unit. Click on Manage tags to open the drawer. Verify the color.

2. Refresh the count of tags on the unit/outline page(s) after the user makes edits in the tag drawer

Use this branch of frontend-app-course-authoring-
Go to a sample course and go to a unit. Click on Manage tags to open the drawer.
Add and remove tags.
Close the drawer and verify that the TagList is updated.
Open the manage tags drawer for a component of the unit.
Add and remove tags.
Verify that the count of tags is updated.
Go to a sample course and open the outline.
Click on the tag count of any unit and open the manage tags drawer.
Add and remove tags
Verify that the count of tags is updated.

openedx-webhooks · 2024-01-15T21:03:28Z

Thanks for the pull request, @ChrisChV! 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.

yusuf-musleh

👍 The updated implementation works quite well, just had a suggestion about the messaging format in the related (openedx/frontend-app-authoring#800) for the close message, which would need a small change here as well.

Great work @ChrisChV ! It's great seeing how all of this is coming together.

I tested this: I confirmed that the dark background color has changed to the lighter one. I also tested that when you open the manage tags drawer for a unit from the course outline, add/remove tags updates automatically on the course outline page, including hiding/showing the tags count when 0 or more that 0 tags applied. I also tested the same for opening the tags drawer for a unit from inside the unit page, and the updated tags are reflected automatically in the TagList on the side of the page. And finally I tested that accessing the manage tags drawer from a component in a unit also reflects the changes on the page when adding/removing tags (including hiding and showing the tags count)
I read through the code
I checked for accessibility issues
Includes documentation
~~I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.~~

bradenmacdonald · 2024-01-22T20:27:46Z

@ChrisChV There is some issue with the JS in this PR. When I have it checked out, I can't build the tutor image - it fails with some error in the r.js optimizer. Perhaps you have included some new JS syntax that breaks the (super old) bundling code we have.

Related, I'm seeing this error on the Unit page, even if I run the rebuilt assets command.

bradenmacdonald · 2024-01-22T20:29:05Z

cms/static/js/factories/tag_count.js

+    tagCountView.render();
+}
+
+export {TagCountFactory};


Ah, these import and export statements are probably the problem - I don't think the build process used in edx-platform supports them. But it could be something else - not sure.

There are other factories that has the same syntax. My tutor is unconfigured, I'm going to try to configure it to try another syntax

That error happened to me, but it was fixed with this line. Maybe clearing the cache?

Nevermind, I found the r.js optimizer error

Thx, let me know when you've found a fix.

@bradenmacdonald I fixed the bug. It was an extrra , here You can test it? I still need to configure my tutor with the MFE

…rawer

chore: Test to fix javascript error chore: Test to fix javascript error chore: Test to fix javascript error

bradenmacdonald · 2024-01-24T23:29:50Z

Hmm, I can see the lighter color, and now the JS build error is resolved, but the auto-refresh isn't working for me. I see that Course-Authoring sends the signals, but the CMS is not refreshing anything.

@yusuf-musleh could you please confirm it's working for you in the latest version?

If so, I'm good to merge this as-is. We're actually going to shift focus to the new MFE versions of these pages very soon, so I don't want to spend too much more time on the "legacy" views.

yusuf-musleh · 2024-01-25T11:20:04Z

@bradenmacdonald @ChrisChV I tried it out with the latest changes on this PR and the mfe PR, and ran through all the cases and can confirm that they're all working as expected on my devstack.

openedx-webhooks · 2024-01-25T18:33:50Z

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

edx-pipeline-bot · 2024-01-25T20:44:45Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2024-01-25T21:04:50Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2024-01-25T21:04:50Z

2U Release Notice: This PR has been deployed to the edX production environment.

* style: drawer-cover color updated for all tagging drawers * feat: Update TagList component when a tag is updated on Manage tags drawer * feat: Refactor TagCount to be able to refresh the count * feat: Sync tag count in units

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

ChrisChV changed the title ~~UX refinement v2~~ Tagging UX refinement v2 Jan 15, 2024

ChrisChV marked this pull request as draft January 15, 2024 21:04

ChrisChV changed the title ~~Tagging UX refinement v2~~ Tagging UX refinement v2 [FC-0036] Jan 17, 2024

ChrisChV mentioned this pull request Jan 17, 2024

feat: Send message after update tags on content [FC-0036] openedx/frontend-app-authoring#800

Closed

itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jan 18, 2024

ChrisChV marked this pull request as ready for review January 18, 2024 16:05

yusuf-musleh approved these changes Jan 19, 2024

View reviewed changes

bradenmacdonald reviewed Jan 22, 2024

View reviewed changes

ChrisChV added 6 commits January 22, 2024 18:20

style: drawer-cover color updated for tagging drawer

a96a942

style: drawer-cover color updated for all tagging drawers

0db5218

feat: Update TagList component when a tag is updated on Manage tags d…

13f4580

…rawer

feat: Reafactor TagCount to be hable to refresh the count

ebcf800

feat: Sync tag count in units

ad2c813

test: Fixing python tests

1544a45

ChrisChV force-pushed the chris/FAL-3601-UX-refinement-v2 branch from 834d7ce to 876f61f Compare January 22, 2024 23:20

chore: Test to fix javascript error

61bb176

chore: Test to fix javascript error chore: Test to fix javascript error chore: Test to fix javascript error

ChrisChV force-pushed the chris/FAL-3601-UX-refinement-v2 branch from 21cdab6 to 61bb176 Compare January 23, 2024 00:27

fix: Bug with javascript

6b78dc9

bradenmacdonald merged commit 5838d68 into openedx:master Jan 25, 2024
64 checks passed

bradenmacdonald deleted the chris/FAL-3601-UX-refinement-v2 branch January 25, 2024 18:33

ChrisChV mentioned this pull request Feb 12, 2024

refactor: Update tags drawer listener for a Json message #34223

Closed

andrey-canon mentioned this pull request May 9, 2024

And/backport tag course nelc/edx-platform#28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tagging UX refinement v2 [FC-0036] #34059

Tagging UX refinement v2 [FC-0036] #34059

ChrisChV commented Jan 15, 2024 •

edited

Loading

openedx-webhooks commented Jan 15, 2024 •

edited

Loading

yusuf-musleh left a comment

bradenmacdonald commented Jan 22, 2024

bradenmacdonald Jan 22, 2024

ChrisChV Jan 23, 2024

ChrisChV Jan 23, 2024

ChrisChV Jan 23, 2024

bradenmacdonald Jan 23, 2024

ChrisChV Jan 24, 2024

bradenmacdonald commented Jan 24, 2024

yusuf-musleh commented Jan 25, 2024

openedx-webhooks commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

Tagging UX refinement v2 [FC-0036] #34059

Tagging UX refinement v2 [FC-0036] #34059

Conversation

ChrisChV commented Jan 15, 2024 • edited Loading

Description

Supporting information

Testing instructions

openedx-webhooks commented Jan 15, 2024 • edited Loading

yusuf-musleh left a comment

Choose a reason for hiding this comment

bradenmacdonald commented Jan 22, 2024

bradenmacdonald Jan 22, 2024

Choose a reason for hiding this comment

ChrisChV Jan 23, 2024

Choose a reason for hiding this comment

ChrisChV Jan 23, 2024

Choose a reason for hiding this comment

ChrisChV Jan 23, 2024

Choose a reason for hiding this comment

bradenmacdonald Jan 23, 2024

Choose a reason for hiding this comment

ChrisChV Jan 24, 2024

Choose a reason for hiding this comment

bradenmacdonald commented Jan 24, 2024

yusuf-musleh commented Jan 25, 2024

openedx-webhooks commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

edx-pipeline-bot commented Jan 25, 2024

ChrisChV commented Jan 15, 2024 •

edited

Loading

openedx-webhooks commented Jan 15, 2024 •

edited

Loading