Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UU-58] Implement tagging & taxonomy feature in outline #855

Merged
merged 10 commits into from
Mar 8, 2024

Conversation

ChrisChV
Copy link
Contributor

@ChrisChV ChrisChV commented Feb 27, 2024

Description

Adds the Manage tags menu item and TagCount button to Units.

Screenshots

image

Supporting information

Testing instructions

  • Enable contentstore.new_studio_mfe.use_new_course_outline_page waffle flag
  • Run Taxonomy Sample Data script to create units with tags.
  • Go to a course to display the course outline.
  • Expand sections and subsections to show units.
  • Verify the tag count component.
  • Click on the tago count component. Verify that it opens the tag drawer.
  • Add or delete tags and verify that the tag count is updated in real time.
  • Close the drawer.
  • Click on the menu dots an on Manage tags. Verify that it opens the tag drawer.

@ChrisChV ChrisChV requested a review from a team as a code owner February 27, 2024 18:28
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 27, 2024
@openedx-webhooks
Copy link

openedx-webhooks commented Feb 27, 2024

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.

@ChrisChV ChrisChV marked this pull request as draft February 27, 2024 18:29
@KristinAoki
Copy link
Member

@navinkarkera Is this also some of the work that you were planning on doing for the outline workstream?

@ChrisChV ChrisChV force-pushed the chris/BB-8633-tagging-on-unit branch from 7c3e176 to 16a62df Compare February 27, 2024 18:31
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.97%. Comparing base (f035391) to head (a6019e5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #855      +/-   ##
==========================================
+ Coverage   90.91%   90.97%   +0.05%     
==========================================
  Files         533      535       +2     
  Lines        9256     9294      +38     
  Branches     1946     1958      +12     
==========================================
+ Hits         8415     8455      +40     
+ Misses        809      807       -2     
  Partials       32       32              

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

@navinkarkera
Copy link
Contributor

@KristinAoki Yes. @ChrisChV has offered to help with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created this component as generic and with extra functionalities (e.g. changing opacity), because it will be used in other interfaces. E.g #852

@ChrisChV ChrisChV marked this pull request as ready for review February 28, 2024 17:21
@ChrisChV ChrisChV force-pushed the chris/BB-8633-tagging-on-unit branch from 4d2c9a2 to 1912890 Compare February 29, 2024 14:52
Comment on lines +169 to +176
{onClickManageTags && (
<Dropdown.Item
data-testid={`${namePrefix}-card-header__menu-manage-tags-button`}
onClick={onClickManageTags}
>
{intl.formatMessage(messages.menuManageTags)}
</Dropdown.Item>
)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the dropdown item and the tag count to both have onClick attributes? Should the tag count be read only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the tag count will be read only in other pages. E.g #852

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so you will update the tag count in the header to be read only and the dropdown item will have to onclick attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the course outline it is necessary that the Manage tags menu (dropdown item) and the tag count component can open the Manage tags drawer, for that reason they both have the onclick attribute (Ref)

Copy link
Contributor

@navinkarkera navinkarkera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisChV Nice work! 👍

  • I tested this: (add/delete tags via new menu option)
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@@ -127,6 +130,7 @@ const CardHeader = ({
{(isVertical || isSequential) && (
<CardStatus status={status} showDiscussionsEnabledBadge={showDiscussionsEnabledBadge} />
)}
{ tagsCount !== undefined && tagsCount !== 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{ tagsCount !== undefined && tagsCount !== 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }
{ tagsCount > 0 && <TagCount count={tagsCount} onClick={onClickManageTags} /> }

@navinkarkera navinkarkera added the jira:2u We want an issue in the 2U Jira instance label Mar 7, 2024
@openedx-webhooks
Copy link

I've created issue TNL-11474 in the private 2U Jira.

@KristinAoki KristinAoki merged commit c39b52a into openedx:master Mar 8, 2024
6 checks passed
@openedx-webhooks
Copy link

@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.

@ChrisChV ChrisChV deleted the chris/BB-8633-tagging-on-unit branch March 11, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira:2u We want an issue in the 2U Jira instance open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants