Tbain/253 add tags count#2957
Conversation
|
Let me know once you'd like this to me reviewed :) Ideally after someone from your team has done an initial review. |
…oad to force data refresh
…sing include_counts param
…logic on page load to force data refresh
… issues causing UT failiures
…oring into tbain/253_add_tags_count # Conflicts: # package-lock.json # package.json # src/taxonomy/data/api.ts # src/taxonomy/data/apiHooks.ts # src/taxonomy/tag-list/TagListTable.test.jsx # src/taxonomy/tag-list/TagListTable.tsx # src/taxonomy/tag-list/tagColumns.tsx
a19347f to
9dabba6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2957 +/- ##
=======================================
Coverage 95.56% 95.56%
=======================================
Files 1349 1349
Lines 31155 31159 +4
Branches 7064 7067 +3
=======================================
+ Hits 29773 29777 +4
Misses 1320 1320
Partials 62 62 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| queryClient.invalidateQueries({ queryKey: taxonomyQueryKeys.taxonomyMetadata(taxonomyId) }); | ||
| queryClient.invalidateQueries({ | ||
| queryKey: taxonomyQueryKeys.taxonomyMetadata(taxonomyId), | ||
| refetchType: 'none', |
There was a problem hiding this comment.
What's the reason for adding refetchType: 'none', here, and refetchOnMount: 'always', above?
There was a problem hiding this comment.
We had observed behavior where the taxonomy data wouldn't refresh when you navigated to the page, you'd have to do a manual browser refresh in order to get updated data. We had explored adding a small method to force it to refresh on mount, but refetchOnMount performs that same functionality without needing a somewhat hacky implementation. The refetchType: 'none' on the invalidate queries will prevent it from trying to refresh immediately upon invalidation and allow it to be refreshed naturally with the mount action.
There was a problem hiding this comment.
We had observed behavior where the taxonomy data wouldn't refresh when you navigated to the page, you'd have to do a manual browser refresh in order to get updated data.
Isn't that how all web pages normally work though? Or you mean somehow it was displaying stale data after previously displaying "new" data? If the latter, I think the "modes" code is affecting something.
The refetchType: 'none' on the invalidate queries will prevent it from trying to refresh immediately upon invalidation
Normally, we do want things to refresh immediately upon completion of some mutation, and that's how everything else in this MFE works. So this seems a bit unusual to me.
|
@tbain @jesperhodge I think this PR is ready to go, as long as you remove |
…oring into tbain/253_add_tags_count
Description
This implements openedx/modular-learning#253 , the task to add tag usage counts to the tags table under the taxonomies table. The corresponding backend piece is where the aggregation work is done will be in a seperate PR to openedx-core. There are some UX changes that differ from the figmas; namely the figmas show a Cyan/Teal bubble body with black text. The Paragon element we're using does not support this color per se without having to customize it, so were going ahead with using the standard "primary" variant of the bubble tag to keep with Paragon standards, and also this provides better color contrast in support of visual accessibility.
Screenshots of UI changes
Supporting information
Github issue with AC: openedx/modular-learning#253
Paragon Bubble Tag element used to implement this feature: https://paragon-openedx.netlify.app/components/bubble
Figma Mockup(s): https://www.figma.com/design/yE6wmSFnokNM4FG1naasly/Content-Tagging-MVP?node-id=40000158-73422&t=mL0Gi7JcDX2CL2NM-0
Testing instructions
Refer to the AC in the Github Issue as well as reference the Figma mockups for look and feel guidance.
Other information
Include anything else that will help reviewers and consumers understand the change.