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

Tagging UX refinement v2 [FC-0036] #34059

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
'<article class="outline outline-complex outline-course" data-locator="{locator}" data-course-key="{course_key}" data-course-assets="{assets_url}" data-taxonomy-tags-widget-url="{taxonomy_tags_widget_url}" >'.format( # lint-amnesty, pylint: disable=line-too-long
'<article class="outline outline-complex outline-course" data-locator="{locator}" data-course-key="{course_key}" data-course-assets="{assets_url}" >'.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
Expand Down
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 1 addition & 7 deletions cms/djangoapps/contentstore/views/tests/test_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"]:
Expand Down
13 changes: 13 additions & 0 deletions cms/static/js/factories/tag_count.js
Original file line number Diff line number Diff line change
@@ -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};
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, I found the r.js optimizer error

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

13 changes: 13 additions & 0 deletions cms/static/js/models/tag_count.js
Original file line number Diff line number Diff line change
@@ -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;
});
27 changes: 22 additions & 5 deletions cms/static/js/views/course_outline.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'));
},
Expand Down Expand Up @@ -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);
},

Expand Down
1 change: 1 addition & 0 deletions cms/static/js/views/pages/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
77 changes: 77 additions & 0 deletions cms/static/js/views/pages/container_subviews.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
54 changes: 54 additions & 0 deletions cms/static/js/views/tag_count.js
Original file line number Diff line number Diff line change
@@ -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;
});
4 changes: 4 additions & 0 deletions cms/static/sass/elements/_drawer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion cms/templates/container.html
Original file line number Diff line number Diff line change
Expand Up @@ -281,5 +281,5 @@ <h5 class="title">${_("Location ID")}</h5>
</div>

<div id="manage-tags-drawer" class="drawer"></div>
<div class="drawer-cover"></div>
<div class="drawer-cover gray-cover"></div>
</%block>
6 changes: 3 additions & 3 deletions cms/templates/course_outline.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

<%block name="header_extras">
<link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css')}" />
% 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']:
<script type="text/template" id="${template_name}-tpl">
<%static:include path="js/${template_name}.underscore" />
</script>
Expand Down Expand Up @@ -281,7 +281,7 @@ <h3 class="sr">${_("Page Actions")}</h3>
assets_url = reverse('assets_handler', kwargs={'course_key_string': str(course_locator.course_key)})
%>
<h2 class="sr">${_("Course Outline")}</h2>
<article class="outline outline-complex outline-course" data-locator="${course_locator}" data-course-key="${course_locator.course_key}" data-course-assets="${assets_url}" data-taxonomy-tags-widget-url="${taxonomy_tags_widget_url}">
<article class="outline outline-complex outline-course" data-locator="${course_locator}" data-course-key="${course_locator.course_key}" data-course-assets="${assets_url}">
</article>
</div>
<div class="ui-loading">
Expand Down Expand Up @@ -323,5 +323,5 @@ <h3 class="title-3">${_("Changing the content learners see")}</h3>
</div>

<div id="manage-tags-drawer" class="drawer"></div>
<div class="drawer-cover"></div>
<div class="drawer-cover gray-cover"></div>
</%block>
13 changes: 4 additions & 9 deletions cms/templates/js/course-outline.underscore
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -171,14 +172,8 @@ if (is_proctored_exam) {
</li>
<% } %>

<% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage && tagsCount > 0) { %>
<li class="action-item">
<a href="#" data-tooltip="<%- gettext('Manage Tags') %>" class="manage-tags-button action-button">
<span class="icon fa fa-tag" aria-hidden="true"></span>
<span><%- tagsCount %></span>
<span class="sr action-button-text"><%- gettext('Manage Tags') %></span>
</a>
</li>
<% if (xblockInfo.isVertical() && typeof useTaggingTaxonomyListPage !== "undefined" && useTaggingTaxonomyListPage) { %>
<li class="action-item tag-count" data-locator="<%- xblockId %>"></li>
<% } %>

<% if (typeof enableCopyPasteUnits !== "undefined" && enableCopyPasteUnits) { %>
Expand Down
7 changes: 7 additions & 0 deletions cms/templates/js/tag-count.underscore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<% if (tags_count && tags_count > 0) { %>
<button data-tooltip="<%- gettext("Manage Tags") %>" class="btn-default action-button manage-tags-button" data-testid="tag-count-button">
<span class="icon fa fa-tag" aria-hidden="true"></span>
<span><%- tags_count %></span>
<span class="sr action-button-text"><%- gettext("Manage Tags") %></span>
</button>
<% } %>
Loading
Loading