From 5838d68efc1bd29d9dda593d1d0a1306654e58ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Thu, 25 Jan 2024 13:33:47 -0500 Subject: [PATCH] feat: Tagging UX refinements - refresh tag count on edit (#34059) * 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 --- .../contentstore/tests/test_contentstore.py | 6 +- cms/djangoapps/contentstore/views/preview.py | 1 + .../contentstore/views/tests/test_block.py | 8 +- .../xblock_storage_handlers/view_handlers.py | 1 + cms/static/js/factories/tag_count.js | 13 ++++ cms/static/js/models/tag_count.js | 13 ++++ cms/static/js/views/course_outline.js | 27 +++++-- cms/static/js/views/pages/container.js | 1 + .../js/views/pages/container_subviews.js | 77 +++++++++++++++++++ cms/static/js/views/tag_count.js | 54 +++++++++++++ cms/static/sass/elements/_drawer.scss | 4 + cms/templates/container.html | 2 +- cms/templates/course_outline.html | 6 +- cms/templates/js/course-outline.underscore | 13 +--- cms/templates/js/tag-count.underscore | 7 ++ cms/templates/studio_xblock_wrapper.html | 23 ++++-- .../content_tagging/rest_api/v1/urls.py | 2 + webpack.common.config.js | 1 + 18 files changed, 221 insertions(+), 38 deletions(-) create mode 100644 cms/static/js/factories/tag_count.js create mode 100644 cms/static/js/models/tag_count.js create mode 100644 cms/static/js/views/tag_count.js create mode 100644 cms/templates/js/tag-count.underscore diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b82605f8933f..9f90840156ca 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -49,7 +49,6 @@ delete_course, reverse_course_url, reverse_url, - get_taxonomy_tags_widget_url, ) from cms.djangoapps.contentstore.views.component import ADVANCED_COMPONENT_TYPES from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError @@ -1416,15 +1415,12 @@ def test_course_overview_view_with_course(self): course.location.course_key ) - taxonomy_tags_widget_url = get_taxonomy_tags_widget_url(course.id) - self.assertContains( resp, - '
'.format( # lint-amnesty, pylint: disable=line-too-long + '
'.format( # lint-amnesty, pylint: disable=line-too-long locator=str(course.location), course_key=str(course.id), assets_url=assets_url, - taxonomy_tags_widget_url=taxonomy_tags_widget_url, ), status_code=200, html=True diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index a897a38ad21a..9c9926a5b25d 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -315,6 +315,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_reorderable': is_reorderable, 'can_edit': can_edit, 'can_edit_visibility': context.get('can_edit_visibility', is_course), + 'course_authoring_url': settings.COURSE_AUTHORING_MICROFRONTEND_URL, 'is_loading': context.get('is_loading', False), 'is_selected': context.get('is_selected', False), 'selectable': context.get('selectable', False), diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index ac86961b2293..c8ac9b89dc2d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -288,15 +288,9 @@ def test_tag_count_in_container_fragment(self, mock_get_object_tag_counts): self.assertEqual(resp.status_code, 200) usage_key = self.response_usage_key(resp) - # Get the preview HTML without tags - mock_get_object_tag_counts.return_value = {} - html, __ = self._get_container_preview(root_usage_key) - self.assertIn("wrapper-xblock", html) - self.assertNotIn('data-testid="tag-count-button"', html) - # Get the preview HTML with tags mock_get_object_tag_counts.return_value = { - str(usage_key): 13 + str(usage_key): 13, } html, __ = self._get_container_preview(root_usage_key) self.assertIn("wrapper-xblock", html) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index eb79181c4659..8b122d8c8da0 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -1206,6 +1206,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements xblock_info["tags"] = tags if use_tagging_taxonomy_list_page(): xblock_info["taxonomy_tags_widget_url"] = get_taxonomy_tags_widget_url() + xblock_info["course_authoring_url"] = settings.COURSE_AUTHORING_MICROFRONTEND_URL if course_outline: if xblock_info["has_explicit_staff_lock"]: diff --git a/cms/static/js/factories/tag_count.js b/cms/static/js/factories/tag_count.js new file mode 100644 index 000000000000..cadcfa220f1a --- /dev/null +++ b/cms/static/js/factories/tag_count.js @@ -0,0 +1,13 @@ +import * as TagCountView from 'js/views/tag_count'; +import * as TagCountModel from 'js/models/tag_count'; + +// eslint-disable-next-line no-unused-expressions +'use strict'; +export default function TagCountFactory(TagCountJson, el) { + var model = new TagCountModel(TagCountJson, {parse: true}); + var tagCountView = new TagCountView({el, model}); + tagCountView.setupMessageListener(); + tagCountView.render(); +} + +export {TagCountFactory}; diff --git a/cms/static/js/models/tag_count.js b/cms/static/js/models/tag_count.js new file mode 100644 index 000000000000..7007dfc9dc30 --- /dev/null +++ b/cms/static/js/models/tag_count.js @@ -0,0 +1,13 @@ +define(['backbone', 'underscore'], function(Backbone, _) { + /** + * Model for Tag count view + */ + var TagCountModel = Backbone.Model.extend({ + defaults: { + content_id: null, + tags_count: 0, + course_authoring_url: null, + }, + }); + return TagCountModel; +}); diff --git a/cms/static/js/views/course_outline.js b/cms/static/js/views/course_outline.js index 4b7107cc4ddc..04dc98513002 100644 --- a/cms/static/js/views/course_outline.js +++ b/cms/static/js/views/course_outline.js @@ -12,11 +12,11 @@ define(['jquery', 'underscore', 'js/views/xblock_outline', 'edx-ui-toolkit/js/ut 'common/js/components/utils/view_utils', 'js/views/utils/xblock_utils', 'js/models/xblock_outline_info', 'js/views/modals/course_outline_modals', 'js/utils/drag_and_drop', 'common/js/components/views/feedback_notification', 'common/js/components/views/feedback_prompt', - 'js/views/utils/tagging_drawer_utils',], + 'js/views/utils/tagging_drawer_utils', 'js/views/tag_count', 'js/models/tag_count'], function( $, _, XBlockOutlineView, StringUtils, ViewUtils, XBlockViewUtils, XBlockOutlineInfo, CourseOutlineModalsFactory, ContentDragger, NotificationView, PromptView, - TaggingDrawerUtils + TaggingDrawerUtils, TagCountView, TagCountModel ) { var CourseOutlineView = XBlockOutlineView.extend({ // takes XBlockOutlineInfo as a model @@ -28,9 +28,28 @@ function( this.makeContentDraggable(this.el); // Show/hide the paste button this.initializePasteButton(this.el); + this.renderTagCount(); return renderResult; }, + renderTagCount: function() { + const contentId = this.model.get('id'); + const tagCountsByUnit = this.model.get('tag_counts_by_unit') + const tagsCount = tagCountsByUnit !== undefined ? tagCountsByUnit[contentId] : 0 + var countModel = new TagCountModel({ + content_id: contentId, + tags_count: tagsCount, + course_authoring_url: this.model.get('course_authoring_url'), + }, {parse: true}); + var tagCountView = new TagCountView({el: this.$('.tag-count'), model: countModel}); + tagCountView.setupMessageListener(); + tagCountView.render(); + this.$('.tag-count').click((event) => { + event.preventDefault(); + this.openManageTagsDrawer(); + }); + }, + shouldExpandChildren: function() { return this.expandedLocators.contains(this.model.get('id')); }, @@ -461,10 +480,8 @@ function( }, openManageTagsDrawer() { - const article = document.querySelector('[data-taxonomy-tags-widget-url]'); - const taxonomyTagsWidgetUrl = $(article).attr('data-taxonomy-tags-widget-url'); + const taxonomyTagsWidgetUrl = this.model.get('taxonomy_tags_widget_url'); const contentId = this.model.get('id'); - TaggingDrawerUtils.openDrawer(taxonomyTagsWidgetUrl, contentId); }, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 783133af21be..3268b60e416a 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -111,6 +111,7 @@ function($, _, Backbone, gettext, BasePage, el: this.$('.unit-tags'), model: this.model }); + this.tagListView.setupMessageListener(); this.tagListView.render(); this.unitOutlineView = new UnitOutlineView({ diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 8848abc246e2..7ea09eff086e 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -370,6 +370,83 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils, H } }, + setupMessageListener: function () { + window.addEventListener( + "message", (event) => { + // Listen any message from Manage tags drawer. + var data = event.data; + var courseAuthoringUrl = this.model.get("course_authoring_url") + if (event.origin == courseAuthoringUrl + && data.includes('[Manage tags drawer] Tags updated:')) { + // This message arrives when there is a change in the tag list. + // The message contains the new list of tags. + let jsonData = data.replace(/\[Manage tags drawer\] Tags updated: /g, ""); + jsonData = JSON.parse(jsonData); + if (jsonData.contentId == this.model.id) { + this.model.set('tags', this.buildTaxonomyTree(jsonData)); + this.render(); + } + } + }, + ); + }, + + buildTaxonomyTree: function(data) { + // TODO We can use this function for the initial request of tags + // and avoid to use two functions (see get_unit_tags on contentstore/views/component.py) + + var taxonomyList = []; + var totalCount = 0; + var actualId = 0; + data.taxonomies.forEach((taxonomy) => { + // Build a tag tree for each taxonomy + var rootTagsValues = []; + var tags = {}; + taxonomy.tags.forEach((tag) => { + // Creates the tags for all the lineage of this tag + for (let i = tag.lineage.length - 1; i >= 0; i--){ + var tagValue = tag.lineage[i] + var tagProcessedBefore = tags.hasOwnProperty(tagValue); + if (!tagProcessedBefore) { + tags[tagValue] = { + id: actualId, + value: tagValue, + children: [], + } + actualId++; + if (i == 0) { + rootTagsValues.push(tagValue); + } + } + if (i !== tag.lineage.length - 1) { + // Add a child into the children list + tags[tagValue].children.push(tags[tag.lineage[i + 1]]) + } + if (tagProcessedBefore) { + // Break this loop if the tag has been processed before, + // we don't need to process lineage again to avoid duplicates. + break; + } + } + }) + + var tagCount = Object.keys(tags).length; + // Add the tree to the taxonomy list + taxonomyList.push({ + id: taxonomy.taxonomyId, + value: taxonomy.name, + tags: rootTagsValues.map(rootValue => tags[rootValue]), + count: tagCount, + }); + totalCount += tagCount; + }); + + return { + count: totalCount, + taxonomies: taxonomyList, + }; + }, + handleKeyDownOnHeader: function(event) { if (event.key === 'Enter' || event.key === ' ') { event.preventDefault(); diff --git a/cms/static/js/views/tag_count.js b/cms/static/js/views/tag_count.js new file mode 100644 index 000000000000..c7ba4d79e5ed --- /dev/null +++ b/cms/static/js/views/tag_count.js @@ -0,0 +1,54 @@ +define(['jquery', 'underscore', 'js/views/baseview', 'edx-ui-toolkit/js/utils/html-utils'], +function($, _, BaseView, HtmlUtils) { + 'use strict'; + + /** + * TagCountView displays the tag count of a unit/component + * + * This component is being rendered in this way to allow receiving + * messages from the Manage tags drawer and being able to update the count. + */ + var TagCountView = BaseView.extend({ + // takes TagCountModel as a model + + initialize: function() { + BaseView.prototype.initialize.call(this); + this.template = this.loadTemplate('tag-count'); + }, + + setupMessageListener: function () { + window.addEventListener( + 'message', (event) => { + // Listen any message from Manage tags drawer. + var data = event.data; + var courseAuthoringUrl = this.model.get("course_authoring_url") + if (event.origin == courseAuthoringUrl + && data.includes('[Manage tags drawer] Count updated:')) { + // This message arrives when there is a change in the tag list. + // The message contains the new count of tags. + let jsonData = data.replace(/\[Manage tags drawer\] Count updated: /g, ""); + jsonData = JSON.parse(jsonData); + if (jsonData.contentId == this.model.get("content_id")) { + this.model.set('tags_count', jsonData.count); + this.render(); + } + } + } + ); + }, + + render: function() { + HtmlUtils.setHtml( + this.$el, + HtmlUtils.HTML( + this.template({ + tags_count: this.model.get("tags_count"), + }) + ) + ); + return this; + } + }); + + return TagCountView; +}); diff --git a/cms/static/sass/elements/_drawer.scss b/cms/static/sass/elements/_drawer.scss index c18073be9864..96edfe1983f1 100644 --- a/cms/static/sass/elements/_drawer.scss +++ b/cms/static/sass/elements/_drawer.scss @@ -13,6 +13,10 @@ background: rgba(0, 0, 0, 0.8); } +.drawer-cover.gray-cover { + background: rgba(112, 112, 112, 0.8); +} + .drawer { @extend %ui-depth4; diff --git a/cms/templates/container.html b/cms/templates/container.html index cd9530ed5120..d61ce60e9189 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -281,5 +281,5 @@
${_("Location ID")}
-
+
diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index e5959867be86..fbcb2941b24f 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -29,7 +29,7 @@ <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor', 'tag-count']: @@ -281,7 +281,7 @@

${_("Page Actions")}

assets_url = reverse('assets_handler', kwargs={'course_key_string': str(course_locator.course_key)}) %>

${_("Course Outline")}

-
+
@@ -323,5 +323,5 @@

${_("Changing the content learners see")}

-
+
diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index be5e1477549b..c62979bced1f 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -7,7 +7,8 @@ var hasPartitionGroups = xblockInfo.get('has_partition_group_components'); var userPartitionInfo = xblockInfo.get('user_partition_info'); var selectedGroupsLabel = userPartitionInfo['selected_groups_label']; var selectedPartitionIndex = userPartitionInfo['selected_partition_index']; -var tagsCount = (xblockInfo.get('tag_counts_by_unit') || {})[xblockInfo.get('id')] || 0; +var xblockId = xblockInfo.get('id') +var tagsCount = (xblockInfo.get('tag_counts_by_unit') || {})[xblockId] || 0; var statusMessages = []; var messageType; @@ -171,14 +172,8 @@ if (is_proctored_exam) { <% } %> - <% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage && tagsCount > 0) { %> -
  • - - - <%- tagsCount %> - <%- gettext('Manage Tags') %> - -
    • + <% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage) { %> +
  • <% } %> <% if (typeof enableCopyPasteUnits !== "undefined" && enableCopyPasteUnits) { %> diff --git a/cms/templates/js/tag-count.underscore b/cms/templates/js/tag-count.underscore new file mode 100644 index 000000000000..253323109f3c --- /dev/null +++ b/cms/templates/js/tag-count.underscore @@ -0,0 +1,7 @@ +<% if (tags_count && tags_count > 0) { %> + +<% } %> diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 0d6cafc95d66..9ae3a3a5dd21 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -29,6 +29,9 @@ + +<%static:webpack entry="js/factories/tag_count"> + TagCountFactory({ + tags_count: "${tags_count | n, js_escaped_string}", + content_id: "${xblock.location | n, js_escaped_string}", + course_authoring_url: "${course_authoring_url | n, js_escaped_string}", + }, + $('li.tag-count[data-locator="${xblock.location | n, js_escaped_string}"]') + ); + + % if not is_root: % if is_reorderable:
  • @@ -99,14 +112,8 @@
      % if not is_root: % if can_edit: - % if use_tagging and tags_count: -
    • - -
      • + % if use_tagging: +
    • % endif % if not show_inline:
    • diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py index 53addf281ce8..ad7fd8005c71 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py @@ -3,6 +3,7 @@ """ from rest_framework.routers import DefaultRouter +from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagCountsView from django.urls.conf import path, include @@ -16,6 +17,7 @@ router = DefaultRouter() router.register("taxonomies", views.TaxonomyOrgView, basename="taxonomy") router.register("object_tags", views.ObjectTagOrgView, basename="object_tag") +router.register("object_tag_counts", ObjectTagCountsView, basename="object_tag_counts") urlpatterns = [ path( diff --git a/webpack.common.config.js b/webpack.common.config.js index 239cfb4f2e39..075bfcf82783 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -84,6 +84,7 @@ module.exports = Merge.smart({ 'js/factories/xblock_validation': './cms/static/js/factories/xblock_validation.js', 'js/factories/edit_tabs': './cms/static/js/factories/edit_tabs.js', 'js/sock': './cms/static/js/sock.js', + 'js/factories/tag_count': './cms/static/js/factories/tag_count.js', // LMS SingleSupportForm: './lms/static/support/jsx/single_support_form.jsx',