diff --git a/common/test/acceptance/pages/lms/edxnotes.py b/common/test/acceptance/pages/lms/edxnotes.py index 91accb00e0..552e9a06ed 100644 --- a/common/test/acceptance/pages/lms/edxnotes.py +++ b/common/test/acceptance/pages/lms/edxnotes.py @@ -34,9 +34,9 @@ class NoteChild(PageObject): return None -class EdxNotesPageGroup(NoteChild): +class EdxNotesChapterGroup(NoteChild): """ - Helper class that works with note groups on Note page of the course. + Helper class that works with chapter (section) grouping of notes in the Course Structure view on the Note page. """ BODY_SELECTOR = ".note-group" @@ -51,18 +51,16 @@ class EdxNotesPageGroup(NoteChild): @property def children(self): children = self.q(css=self._bounded_selector('.note-section')) - return [EdxNotesPageSection(self.browser, child.get_attribute("id")) for child in children] + return [EdxNotesSubsectionGroup(self.browser, child.get_attribute("id")) for child in children] -class EdxNotesPageSection(NoteChild): +class EdxNotesGroupMixin(object): """ - Helper class that works with note sections on Note page of the course. + Helper mixin that works with note groups (used for subsection and tag groupings). """ - BODY_SELECTOR = ".note-section" - @property def title(self): - return self._get_element_text(".course-subtitle") + return self._get_element_text(self.TITLE_SELECTOR) @property def children(self): @@ -74,12 +72,49 @@ class EdxNotesPageSection(NoteChild): return [section.text for section in self.children] +class EdxNotesSubsectionGroup(NoteChild, EdxNotesGroupMixin): + """ + Helper class that works with subsection grouping of notes in the Course Structure view on the Note page. + """ + BODY_SELECTOR = ".note-section" + TITLE_SELECTOR = ".course-subtitle" + + +class EdxNotesTagsGroup(NoteChild, EdxNotesGroupMixin): + """ + Helper class that works with tags grouping of notes in the Tags view on the Note page. + """ + BODY_SELECTOR = ".note-group" + TITLE_SELECTOR = ".tags-title" + + def scrolled_to_top(self, group_index): + """ + Returns True if the group with supplied group)index is scrolled near the top of the page + (expects 10 px padding). + + The group_index must be supplied because JQuery must be used to get this information, and it + does not have access to the bounded selector. + """ + title_selector = "$('" + self.TITLE_SELECTOR + "')[" + str(group_index) + "]" + top_script = "return " + title_selector + ".getBoundingClientRect().top;" + EmptyPromise( + lambda: 8 < self.browser.execute_script(top_script) < 12, + "Expected tag title '{}' to scroll to top, but was at location {}".format( + self.title, self.browser.execute_script(top_script) + ) + ).fulfill() + # Now also verify that focus has moved to this title (for screen readers): + active_script = "return " + title_selector + " === document.activeElement;" + return self.browser.execute_script(active_script) + + class EdxNotesPageItem(NoteChild): """ Helper class that works with note items on Note page of the course. """ BODY_SELECTOR = ".note" UNIT_LINK_SELECTOR = "a.reference-unit-link" + TAG_SELECTOR = "a.reference-tags" def go_to_unit(self, unit_page=None): self.q(css=self._bounded_selector(self.UNIT_LINK_SELECTOR)).click() @@ -102,6 +137,18 @@ class EdxNotesPageItem(NoteChild): def time_updated(self): return self._get_element_text(".reference-updated-date") + @property + def tags(self): + """ The tags associated with this note. """ + tag_links = self.q(css=self._bounded_selector(self.TAG_SELECTOR)) + if len(tag_links) == 0: + return None + return[tag_link.text for tag_link in tag_links] + + def go_to_tag(self, tag_name): + """ Clicks a tag associated with the note to change to the tags view (and scroll to the tag group). """ + self.q(css=self._bounded_selector(self.TAG_SELECTOR)).filter(lambda el: tag_name in el.text).click() + class EdxNotesPageView(PageObject): """ @@ -174,7 +221,17 @@ class CourseStructureView(EdxNotesPageView): BODY_SELECTOR = "#structure-panel" TAB_SELECTOR = ".tab#view-course-structure" CHILD_SELECTOR = ".note-group" - CHILD_CLASS = EdxNotesPageGroup + CHILD_CLASS = EdxNotesChapterGroup + + +class TagsView(EdxNotesPageView): + """ + Helper class for Tags view. + """ + BODY_SELECTOR = "#tags-panel" + TAB_SELECTOR = ".tab#view-tags" + CHILD_SELECTOR = ".note-group" + CHILD_CLASS = EdxNotesTagsGroup class SearchResultsView(EdxNotesPageView): @@ -193,6 +250,7 @@ class EdxNotesPage(CoursePage): MAPPING = { "recent": RecentActivityView, "structure": CourseStructureView, + "tags": TagsView, "search": SearchResultsView, } @@ -210,9 +268,9 @@ class EdxNotesPage(CoursePage): self.current_view = self.MAPPING[tab_name](self.browser) self.current_view.visit() - def close_tab(self, tab_name): + def close_tab(self): """ - Closes the tab `tab_name(str)`. + Closes the current view. """ self.current_view.close() self.current_view = self.MAPPING["recent"](self.browser) @@ -267,20 +325,28 @@ class EdxNotesPage(CoursePage): return [EdxNotesPageItem(self.browser, child.get_attribute("id")) for child in children] @property - def groups(self): + def chapter_groups(self): """ - Returns all groups on the page. + Returns all chapter groups on the page. """ children = self.q(css='.note-group') - return [EdxNotesPageGroup(self.browser, child.get_attribute("id")) for child in children] + return [EdxNotesChapterGroup(self.browser, child.get_attribute("id")) for child in children] @property - def sections(self): + def subsection_groups(self): """ - Returns all sections on the page. + Returns all subsection groups on the page. """ children = self.q(css='.note-section') - return [EdxNotesPageSection(self.browser, child.get_attribute("id")) for child in children] + return [EdxNotesSubsectionGroup(self.browser, child.get_attribute("id")) for child in children] + + @property + def tag_groups(self): + """ + Returns all tag groups on the page. + """ + children = self.q(css='.note-group') + return [EdxNotesTagsGroup(self.browser, child.get_attribute("id")) for child in children] class EdxNotesPageNoContent(CoursePage): diff --git a/common/test/acceptance/tests/lms/test_lms_edxnotes.py b/common/test/acceptance/tests/lms/test_lms_edxnotes.py index 019219a27f..aa4a0956fd 100644 --- a/common/test/acceptance/tests/lms/test_lms_edxnotes.py +++ b/common/test/acceptance/tests/lms/test_lms_edxnotes.py @@ -340,7 +340,11 @@ class EdxNotesPageTest(EdxNotesTestMixin): self.edxnotes_fixture.create_notes(notes_list) self.edxnotes_fixture.install() - def _add_default_notes(self): + def _add_default_notes(self, tags=None): + """ + Creates 5 test notes. If tags are not specified, will populate the notes with some test tag data. + If tags are specified, they will be used for each of the 3 notes that have tags. + """ xblocks = self.course_fixture.get_nested_xblocks(category="html") self._add_notes([ Note( @@ -358,6 +362,7 @@ class EdxNotesPageTest(EdxNotesTestMixin): text="", quote=u"Annotate this text", updated=datetime(2012, 1, 1, 1, 1, 1, 1).isoformat(), + tags=["Review", "cool"] if tags is None else tags ), Note( usage_id=xblocks[0].locator, @@ -367,6 +372,7 @@ class EdxNotesPageTest(EdxNotesTestMixin): quote="Annotate this text", updated=datetime(2013, 1, 1, 1, 1, 1, 1).isoformat(), ranges=[Range(startOffset=0, endOffset=18)], + tags=["Cool", "TODO"] if tags is None else tags ), Note( usage_id=xblocks[3].locator, @@ -375,6 +381,7 @@ class EdxNotesPageTest(EdxNotesTestMixin): text="Fourth note", quote="", updated=datetime(2014, 1, 1, 1, 1, 1, 1).isoformat(), + tags=["review"] if tags is None else tags ), Note( usage_id=xblocks[1].locator, @@ -386,23 +393,28 @@ class EdxNotesPageTest(EdxNotesTestMixin): ), ]) - def assertNoteContent(self, item, text=None, quote=None, unit_name=None, time_updated=None): - if item.text is not None: - self.assertEqual(text, item.text) - else: - self.assertIsNone(text) + def assertNoteContent(self, item, text=None, quote=None, unit_name=None, time_updated=None, tags=None): + """ Verifies the expected properties of the note. """ + self.assertEqual(text, item.text) if item.quote is not None: self.assertIn(quote, item.quote) else: self.assertIsNone(quote) self.assertEqual(unit_name, item.unit_name) self.assertEqual(time_updated, item.time_updated) + self.assertEqual(tags, item.tags) - def assertGroupContent(self, item, title=None, subtitles=None): + def assertChapterContent(self, item, title=None, subtitles=None): + """ + Verifies the expected title and subsection titles (subtitles) for the given chapter. + """ self.assertEqual(item.title, title) self.assertEqual(item.subtitles, subtitles) - def assertSectionContent(self, item, title=None, notes=None): + def assertGroupContent(self, item, title=None, notes=None): + """ + Verifies the expected title and child notes for the given group. + """ self.assertEqual(item.title, title) self.assertEqual(item.notes, notes) @@ -444,7 +456,8 @@ class EdxNotesPageTest(EdxNotesTestMixin): notes[1], text=u"Fourth note", unit_name="Test Unit 3", - time_updated="Jan 01, 2014 at 01:01 UTC" + time_updated="Jan 01, 2014 at 01:01 UTC", + tags=["review"] ) self.assertNoteContent( @@ -452,14 +465,16 @@ class EdxNotesPageTest(EdxNotesTestMixin): quote="Annotate this text", text=u"Third note", unit_name="Test Unit 1", - time_updated="Jan 01, 2013 at 01:01 UTC" + time_updated="Jan 01, 2013 at 01:01 UTC", + tags=["Cool", "TODO"] ) self.assertNoteContent( notes[3], quote=u"Annotate this text", unit_name="Test Unit 2", - time_updated="Jan 01, 2012 at 01:01 UTC" + time_updated="Jan 01, 2012 at 01:01 UTC", + tags=["Review", "cool"] ) self.assertNoteContent( @@ -483,19 +498,19 @@ class EdxNotesPageTest(EdxNotesTestMixin): self.notes_page.visit().switch_to_tab("structure") notes = self.notes_page.notes - groups = self.notes_page.groups - sections = self.notes_page.sections + groups = self.notes_page.chapter_groups + sections = self.notes_page.subsection_groups self.assertEqual(len(notes), 5) self.assertEqual(len(groups), 2) self.assertEqual(len(sections), 3) - self.assertGroupContent( + self.assertChapterContent( groups[0], title=u"Test Section 1", subtitles=[u"Test Subsection 1", u"Test Subsection 2"] ) - self.assertSectionContent( + self.assertGroupContent( sections[0], title=u"Test Subsection 1", notes=[u"Fifth note", u"Third note", None] @@ -514,17 +529,19 @@ class EdxNotesPageTest(EdxNotesTestMixin): quote=u"Annotate this text", text=u"Third note", unit_name="Test Unit 1", - time_updated="Jan 01, 2013 at 01:01 UTC" + time_updated="Jan 01, 2013 at 01:01 UTC", + tags=["Cool", "TODO"] ) self.assertNoteContent( notes[2], quote=u"Annotate this text", unit_name="Test Unit 2", - time_updated="Jan 01, 2012 at 01:01 UTC" + time_updated="Jan 01, 2012 at 01:01 UTC", + tags=["Review", "cool"] ) - self.assertSectionContent( + self.assertGroupContent( sections[1], title=u"Test Subsection 2", notes=[u"Fourth note"] @@ -534,16 +551,17 @@ class EdxNotesPageTest(EdxNotesTestMixin): notes[3], text=u"Fourth note", unit_name="Test Unit 3", - time_updated="Jan 01, 2014 at 01:01 UTC" + time_updated="Jan 01, 2014 at 01:01 UTC", + tags=["review"] ) - self.assertGroupContent( + self.assertChapterContent( groups[1], title=u"Test Section 2", subtitles=[u"Test Subsection 3"], ) - self.assertSectionContent( + self.assertGroupContent( sections[2], title=u"Test Subsection 3", notes=[u"First note"] @@ -557,6 +575,108 @@ class EdxNotesPageTest(EdxNotesTestMixin): time_updated="Jan 01, 2011 at 01:01 UTC" ) + def test_tags_view(self): + """ + Scenario: User can view all notes by associated tags. + Given I have a course with 5 notes and I am viewing the Notes page + When I switch to the "Tags" view + Then I see 4 tag groups + And I see correct content in the notes and groups + """ + self._add_default_notes() + self.notes_page.visit().switch_to_tab("tags") + + notes = self.notes_page.notes + groups = self.notes_page.tag_groups + self.assertEqual(len(notes), 7) + self.assertEqual(len(groups), 4) + + # Tag group "cool" + self.assertGroupContent( + groups[0], + title=u"cool (2)", + notes=[u"Third note", None] + ) + + self.assertNoteContent( + notes[0], + quote=u"Annotate this text", + text=u"Third note", + unit_name="Test Unit 1", + time_updated="Jan 01, 2013 at 01:01 UTC", + tags=["Cool", "TODO"] + ) + + self.assertNoteContent( + notes[1], + quote=u"Annotate this text", + unit_name="Test Unit 2", + time_updated="Jan 01, 2012 at 01:01 UTC", + tags=["Review", "cool"] + ) + + # Tag group "review" + self.assertGroupContent( + groups[1], + title=u"review (2)", + notes=[u"Fourth note", None] + ) + + self.assertNoteContent( + notes[2], + text=u"Fourth note", + unit_name="Test Unit 3", + time_updated="Jan 01, 2014 at 01:01 UTC", + tags=["review"] + ) + + self.assertNoteContent( + notes[3], + quote=u"Annotate this text", + unit_name="Test Unit 2", + time_updated="Jan 01, 2012 at 01:01 UTC", + tags=["Review", "cool"] + ) + + # Tag group "todo" + self.assertGroupContent( + groups[2], + title=u"todo (1)", + notes=["Third note"] + ) + + self.assertNoteContent( + notes[4], + quote=u"Annotate this text", + text=u"Third note", + unit_name="Test Unit 1", + time_updated="Jan 01, 2013 at 01:01 UTC", + tags=["Cool", "TODO"] + ) + + # Notes with no tags + self.assertGroupContent( + groups[3], + title=u"[no tags] (2)", + notes=["Fifth note", "First note"] + ) + + self.assertNoteContent( + notes[5], + quote=u"Annotate this text", + text=u"Fifth note", + unit_name="Test Unit 1", + time_updated="Jan 01, 2015 at 01:01 UTC" + ) + + self.assertNoteContent( + notes[6], + quote=u"Annotate this text", + text=u"First note", + unit_name="Test Unit 4", + time_updated="Jan 01, 2011 at 01:01 UTC" + ) + def test_easy_access_from_notes_page(self): """ Scenario: Ensure that the link to the Unit works correctly. @@ -618,14 +738,102 @@ class EdxNotesPageTest(EdxNotesTestMixin): # Error message disappears self.assertFalse(self.notes_page.is_error_visible) self.assertIn(u"Search Results", self.notes_page.tabs) - self.assertEqual(len(self.notes_page.notes), 4) + notes = self.notes_page.notes + self.assertEqual(len(notes), 4) + + self.assertNoteContent( + notes[0], + quote=u"Annotate this text", + text=u"Fifth note", + unit_name="Test Unit 1", + time_updated="Jan 01, 2015 at 01:01 UTC" + ) + + self.assertNoteContent( + notes[1], + text=u"Fourth note", + unit_name="Test Unit 3", + time_updated="Jan 01, 2014 at 01:01 UTC", + tags=["review"] + ) + + self.assertNoteContent( + notes[2], + quote="Annotate this text", + text=u"Third note", + unit_name="Test Unit 1", + time_updated="Jan 01, 2013 at 01:01 UTC", + tags=["Cool", "TODO"] + ) + + self.assertNoteContent( + notes[3], + quote=u"Annotate this text", + text=u"First note", + unit_name="Test Unit 4", + time_updated="Jan 01, 2011 at 01:01 UTC" + ) + + def test_scroll_to_tag_recent_activity(self): + """ + Scenario: Can scroll to a tag group from the Recent Activity view (default view) + Given I have a course with 5 notes and I open the Notes page + When I click on a tag associated with a note + Then the Tags view tab gets focus and I scroll to the section of notes associated with that tag + """ + self._add_default_notes(["apple", "banana", "kiwi", "pear", "pumpkin", "squash", "zucchini"]) + self.notes_page.visit() + self._scroll_to_tag_and_verify("pear", 3) + + def test_scroll_to_tag_course_structure(self): + """ + Scenario: Can scroll to a tag group from the Course Structure view + Given I have a course with 5 notes and I open the Notes page and select the Course Structure view + When I click on a tag associated with a note + Then the Tags view tab gets focus and I scroll to the section of notes associated with that tag + """ + self._add_default_notes(["apple", "banana", "kiwi", "pear", "pumpkin", "squash", "zucchini"]) + self.notes_page.visit().switch_to_tab("structure") + self._scroll_to_tag_and_verify("squash", 5) + + def test_scroll_to_tag_search(self): + """ + Scenario: Can scroll to a tag group from the Search Results view + Given I have a course with 5 notes and I open the Notes page and perform a search + Then the Search view tab opens and gets focus + And when I click on a tag associated with a note + Then the Tags view tab gets focus and I scroll to the section of notes associated with that tag + """ + self._add_default_notes(["apple", "banana", "kiwi", "pear", "pumpkin", "squash", "zucchini"]) + self.notes_page.visit().search("note") + self._scroll_to_tag_and_verify("pumpkin", 4) + + def test_scroll_to_tag_from_tag_view(self): + """ + Scenario: Can scroll to a tag group from the Tags view + Given I have a course with 5 notes and I open the Notes page and select the Tag view + When I click on a tag associated with a note + Then I scroll to the section of notes associated with that tag + """ + self._add_default_notes(["apple", "banana", "kiwi", "pear", "pumpkin", "squash", "zucchini"]) + self.notes_page.visit().switch_to_tab("tags") + self._scroll_to_tag_and_verify("kiwi", 2) + + def _scroll_to_tag_and_verify(self, tag_name, group_index): + """ Helper method for all scroll to tag tests """ + self.notes_page.notes[1].go_to_tag(tag_name) + + # Because all the notes (with tags) have the same tags, they will end up ordered alphabetically. + pear_group = self.notes_page.tag_groups[group_index] + self.assertEqual(tag_name + " (3)", pear_group.title) + self.assertTrue(pear_group.scrolled_to_top(group_index)) def test_tabs_behaves_correctly(self): """ Scenario: Tabs behaves correctly. Given I have a course with 5 notes When I open Notes page - Then I see only "Recent Activity" and "Location in Course" tabs + Then I see only "Recent Activity", "Location in Course", and "Tags" tabs When I run the search with "note" query And I see that "Search Results" tab appears with 4 notes found Then I switch to "Recent Activity" tab @@ -643,24 +851,24 @@ class EdxNotesPageTest(EdxNotesTestMixin): self.notes_page.visit() # We're on Recent Activity tab. - self.assertEqual(len(self.notes_page.tabs), 2) - self.assertEqual([u"Recent Activity", u"Location in Course"], self.notes_page.tabs) + self.assertEqual(len(self.notes_page.tabs), 3) + self.assertEqual([u"Recent Activity", u"Location in Course", u"Tags"], self.notes_page.tabs) self.notes_page.search("note") # We're on Search Results tab - self.assertEqual(len(self.notes_page.tabs), 3) + self.assertEqual(len(self.notes_page.tabs), 4) self.assertIn(u"Search Results", self.notes_page.tabs) self.assertEqual(len(self.notes_page.notes), 4) # We can switch on Recent Activity tab and back. self.notes_page.switch_to_tab("recent") self.assertEqual(len(self.notes_page.notes), 5) self.notes_page.switch_to_tab("structure") - self.assertEqual(len(self.notes_page.groups), 2) + self.assertEqual(len(self.notes_page.chapter_groups), 2) self.assertEqual(len(self.notes_page.notes), 5) self.notes_page.switch_to_tab("search") self.assertEqual(len(self.notes_page.notes), 4) # Can close search results page - self.notes_page.close_tab("search") - self.assertEqual(len(self.notes_page.tabs), 2) + self.notes_page.close_tab() + self.assertEqual(len(self.notes_page.tabs), 3) self.assertNotIn(u"Search Results", self.notes_page.tabs) self.assertEqual(len(self.notes_page.notes), 5) diff --git a/lms/static/js/edxnotes/models/tab.js b/lms/static/js/edxnotes/models/tab.js index 07882cd702..308eaef108 100644 --- a/lms/static/js/edxnotes/models/tab.js +++ b/lms/static/js/edxnotes/models/tab.js @@ -1,6 +1,6 @@ ;(function (define, undefined) { 'use strict'; -define(['backbone'], function (Backbone) { +define(['underscore', 'backbone'], function (_, Backbone) { var TabModel = Backbone.Model.extend({ defaults: { 'identifier': '', diff --git a/lms/static/js/edxnotes/views/note_group.js b/lms/static/js/edxnotes/views/note_group.js index ca79ae134d..5e31c01180 100644 --- a/lms/static/js/edxnotes/views/note_group.js +++ b/lms/static/js/edxnotes/views/note_group.js @@ -3,19 +3,22 @@ define([ 'gettext', 'underscore', 'backbone' ], function (gettext, _, Backbone) { - var NoteSectionView, NoteGroupView; + var GroupView, ChapterView; - NoteSectionView = Backbone.View.extend({ + GroupView = Backbone.View.extend({ tagName: 'section', - className: 'note-section', id: function () { return 'note-section-' + _.uniqueId(); }, - template: _.template('

<%- sectionName %>

'), + + initialize: function () { + this.template = _.template(this.options.template); + this.className = this.options.className; + }, render: function () { this.$el.prepend(this.template({ - sectionName: this.options.section.display_name + displayName: this.options.displayName })); return this; @@ -26,7 +29,7 @@ define([ } }); - NoteGroupView = Backbone.View.extend({ + ChapterView = Backbone.View.extend({ tagName: 'section', className: 'note-group', id: function () { @@ -52,7 +55,13 @@ define([ }, addChild: function (sectionInfo) { - var section = new NoteSectionView({section: sectionInfo}); + var section = new GroupView( + { + displayName: sectionInfo.display_name, + template: '

<%- displayName %>

', + className: "note-section" + } + ); this.children.push(section); return section; }, @@ -65,6 +74,6 @@ define([ } }); - return NoteGroupView; + return {GroupView: GroupView, ChapterView: ChapterView}; }); }).call(this, define || RequireJS.define); diff --git a/lms/static/js/edxnotes/views/note_item.js b/lms/static/js/edxnotes/views/note_item.js index 9f83a615c1..6c26391346 100644 --- a/lms/static/js/edxnotes/views/note_item.js +++ b/lms/static/js/edxnotes/views/note_item.js @@ -13,6 +13,7 @@ define([ events: { 'click .note-excerpt-more-link': 'moreHandler', 'click .reference-unit-link': 'unitLinkHandler', + 'click .reference-tags': 'tagHandler' }, initialize: function (options) { @@ -56,6 +57,11 @@ define([ }, this)); }, + tagHandler: function (event) { + event.preventDefault(); + this.options.scrollToTag(event.currentTarget.text); + }, + redirectTo: function (uri) { window.location = uri; }, diff --git a/lms/static/js/edxnotes/views/notes_factory.js b/lms/static/js/edxnotes/views/notes_factory.js index 4bb521c0bc..bae02f1f9d 100644 --- a/lms/static/js/edxnotes/views/notes_factory.js +++ b/lms/static/js/edxnotes/views/notes_factory.js @@ -7,7 +7,7 @@ define([ 'js/edxnotes/plugins/caret_navigation' ], function ($, _, Annotator, NotesLogger) { var plugins = ['Auth', 'Store', 'Scroller', 'Events', 'Accessibility', 'CaretNavigation', 'Tags'], - getOptions, setupPlugins, updateHeaders, getAnnotator; + getOptions, setupPlugins, getAnnotator; /** * Returns options for the annotator. diff --git a/lms/static/js/edxnotes/views/notes_page.js b/lms/static/js/edxnotes/views/notes_page.js index aa39ce513a..4c43dd1542 100644 --- a/lms/static/js/edxnotes/views/notes_page.js +++ b/lms/static/js/edxnotes/views/notes_page.js @@ -3,33 +3,53 @@ define([ 'backbone', 'js/edxnotes/collections/tabs', 'js/edxnotes/views/tabs_list', 'js/edxnotes/views/tabs/recent_activity', 'js/edxnotes/views/tabs/course_structure', - 'js/edxnotes/views/tabs/search_results' + 'js/edxnotes/views/tabs/search_results', 'js/edxnotes/views/tabs/tags' ], function ( Backbone, TabsCollection, TabsListView, RecentActivityView, CourseStructureView, - SearchResultsView + SearchResultsView, TagsView ) { var NotesPageView = Backbone.View.extend({ initialize: function (options) { + var scrollToTag, tagsModel; + this.options = options; this.tabsCollection = new TabsCollection(); + // Must create the Tags view first to get the "scrollToTag" method. + this.tagsView = new TagsView({ + el: this.el, + collection: this.collection, + tabsCollection: this.tabsCollection + }); + + scrollToTag = this.tagsView.scrollToTag; + + // Remove the Tags model from the tabs collection because it should not appear first. + tagsModel = this.tabsCollection.shift(); + this.recentActivityView = new RecentActivityView({ el: this.el, collection: this.collection, - tabsCollection: this.tabsCollection + tabsCollection: this.tabsCollection, + scrollToTag: scrollToTag }); this.courseStructureView = new CourseStructureView({ el: this.el, collection: this.collection, - tabsCollection: this.tabsCollection + tabsCollection: this.tabsCollection, + scrollToTag: scrollToTag }); + // Add the Tags model after the Course Structure model. + this.tabsCollection.push(tagsModel); + this.searchResultsView = new SearchResultsView({ el: this.el, tabsCollection: this.tabsCollection, debug: this.options.debug, - createTabOnInitialization: false + createTabOnInitialization: false, + scrollToTag: scrollToTag }); this.tabsView = new TabsListView({collection: this.tabsCollection}); diff --git a/lms/static/js/edxnotes/views/tab_item.js b/lms/static/js/edxnotes/views/tab_item.js index 4346a8c7c2..a07e228e1c 100644 --- a/lms/static/js/edxnotes/views/tab_item.js +++ b/lms/static/js/edxnotes/views/tab_item.js @@ -1,7 +1,7 @@ ;(function (define, undefined) { 'use strict'; -define(['gettext', 'underscore', 'backbone', 'js/edxnotes/utils/template'], -function (gettext, _, Backbone, templateUtils) { +define(['gettext', 'underscore', 'jquery', 'backbone', 'js/edxnotes/utils/template'], +function (gettext, _, $, Backbone, templateUtils) { var TabItemView = Backbone.View.extend({ tagName: 'li', className: 'tab', diff --git a/lms/static/js/edxnotes/views/tab_panel.js b/lms/static/js/edxnotes/views/tab_panel.js index 8ab9f24937..1639759acf 100644 --- a/lms/static/js/edxnotes/views/tab_panel.js +++ b/lms/static/js/edxnotes/views/tab_panel.js @@ -26,9 +26,9 @@ function (gettext, _, Backbone, NoteItemView) { }, getNotes: function (collection) { - var container = document.createDocumentFragment(), + var container = document.createDocumentFragment(), scrollToTag = this.options.scrollToTag, notes = _.map(collection, function (model) { - var note = new NoteItemView({model: model}); + var note = new NoteItemView({model: model, scrollToTag: scrollToTag}); container.appendChild(note.render().el); return note; }); diff --git a/lms/static/js/edxnotes/views/tab_view.js b/lms/static/js/edxnotes/views/tab_view.js index 1c064ad615..baa8a79801 100644 --- a/lms/static/js/edxnotes/views/tab_view.js +++ b/lms/static/js/edxnotes/views/tab_view.js @@ -1,8 +1,8 @@ ;(function (define, undefined) { 'use strict'; define([ - 'underscore', 'backbone', 'js/edxnotes/models/tab' -], function (_, Backbone, TabModel) { + 'jquery', 'underscore', 'backbone', 'js/edxnotes/models/tab' +], function ($, _, Backbone, TabModel) { var TabView = Backbone.View.extend({ PanelConstructor: null, @@ -52,6 +52,7 @@ define([ // If the view is already rendered, destroy it. this.destroySubView(); this.renderContent().always(this.hideLoadingIndicator); + this.$('.sr-is-focusable.sr-tab-panel').focus(); return this; }, @@ -63,7 +64,7 @@ define([ getSubView: function () { var collection = this.getCollection(); - return new this.PanelConstructor({collection: collection}); + return new this.PanelConstructor({collection: collection, scrollToTag: this.options.scrollToTag}); }, destroySubView: function () { diff --git a/lms/static/js/edxnotes/views/tabs/course_structure.js b/lms/static/js/edxnotes/views/tabs/course_structure.js index 9d1a5918dd..8b5e39f050 100644 --- a/lms/static/js/edxnotes/views/tabs/course_structure.js +++ b/lms/static/js/edxnotes/views/tabs/course_structure.js @@ -1,9 +1,9 @@ ;(function (define, undefined) { 'use strict'; define([ - 'gettext', 'js/edxnotes/views/note_group', 'js/edxnotes/views/tab_panel', + 'gettext', 'underscore', 'js/edxnotes/views/note_group', 'js/edxnotes/views/tab_panel', 'js/edxnotes/views/tab_view' -], function (gettext, NoteGroupView, TabPanelView, TabView) { +], function (gettext, _, NoteGroupView, TabPanelView, TabView) { var CourseStructureView = TabView.extend({ PanelConstructor: TabPanelView.extend({ id: 'structure-panel', @@ -14,28 +14,28 @@ define([ container = document.createDocumentFragment(); _.each(courseStructure.chapters, function (chapterInfo) { - var group = this.getGroup(chapterInfo); + var chapterView = this.getChapterGroupView(chapterInfo); _.each(chapterInfo.children, function (location) { var sectionInfo = courseStructure.sections[location], - section; + sectionView; if (sectionInfo) { - section = group.addChild(sectionInfo); + sectionView = chapterView.addChild(sectionInfo); _.each(sectionInfo.children, function (location) { var notes = courseStructure.units[location]; if (notes) { - section.addChild(this.getNotes(notes)) + sectionView.addChild(this.getNotes(notes)); } }, this); } }, this); - container.appendChild(group.render().el); + container.appendChild(chapterView.render().el); }, this); this.$el.append(container); return this; }, - getGroup: function (chapter, section) { - var group = new NoteGroupView({ + getChapterGroupView: function (chapter, section) { + var group = new NoteGroupView.ChapterView({ chapter: chapter, section: section }); diff --git a/lms/static/js/edxnotes/views/tabs/recent_activity.js b/lms/static/js/edxnotes/views/tabs/recent_activity.js index 58a54c3403..9ab21353d8 100644 --- a/lms/static/js/edxnotes/views/tabs/recent_activity.js +++ b/lms/static/js/edxnotes/views/tabs/recent_activity.js @@ -11,7 +11,7 @@ define([ return [ TabPanelView.prototype.className, 'note-group' - ].join(' ') + ].join(' '); }, renderContent: function () { this.$el.append(this.getNotes(this.collection.toArray())); diff --git a/lms/static/js/edxnotes/views/tabs/search_results.js b/lms/static/js/edxnotes/views/tabs/search_results.js index f9914e32fe..768fc8f708 100644 --- a/lms/static/js/edxnotes/views/tabs/search_results.js +++ b/lms/static/js/edxnotes/views/tabs/search_results.js @@ -1,9 +1,9 @@ ;(function (define, undefined) { 'use strict'; define([ - 'gettext', 'js/edxnotes/views/tab_panel', 'js/edxnotes/views/tab_view', + 'jquery', 'underscore', 'gettext', 'js/edxnotes/views/tab_panel', 'js/edxnotes/views/tab_view', 'js/edxnotes/views/search_box' -], function (gettext, TabPanelView, TabView, SearchBoxView) { +], function ($, _, gettext, TabPanelView, TabView, SearchBoxView) { var SearchResultsView = TabView.extend({ PanelConstructor: TabPanelView.extend({ id: 'search-results-panel', @@ -78,7 +78,8 @@ define([ if (collection.length) { return new this.PanelConstructor({ collection: collection, - searchQuery: this.searchResults.searchQuery + searchQuery: this.searchResults.searchQuery, + scrollToTag: this.options.scrollToTag }); } else { return new this.NoResultsViewConstructor({ diff --git a/lms/static/js/edxnotes/views/tabs/tags.js b/lms/static/js/edxnotes/views/tabs/tags.js new file mode 100644 index 0000000000..909bd26894 --- /dev/null +++ b/lms/static/js/edxnotes/views/tabs/tags.js @@ -0,0 +1,137 @@ +;(function (define, undefined) { +'use strict'; +define([ + 'gettext', 'jquery', 'underscore', 'js/edxnotes/views/note_group', 'js/edxnotes/views/tab_panel', + 'js/edxnotes/views/tab_view' +], function (gettext, $, _, NoteGroupView, TabPanelView, TabView) { + + var TagsView = TabView.extend({ + scrollToTag: function(tagName) { + var titleElement, displayedTitle; + if (!this.tabModel.isActive()) { + this.tabModel.activate(); + } + + displayedTitle = this.contentView.titleMap[tagName.toLowerCase()]; + titleElement = this.$el.find('.tags-title').filter( + function(){ return $(this).text() === displayedTitle; } + ); + $('html,body').animate( + { scrollTop: titleElement.offset().top - 10 }, + 'slow', + function () { titleElement.focus(); } + ); + }, + + initialize: function (options) { + TabView.prototype.initialize.call(this, options); + _.bindAll(this, 'scrollToTag'); + this.options.scrollToTag = this.scrollToTag; + }, + + PanelConstructor: TabPanelView.extend({ + id: 'tags-panel', + title: 'Tags', + // Translators: this is a title shown before all Notes that have no associated tags. It is put within + // brackets to differentiate it from user-defined tags, but it should still be translated. + noTags: gettext('[no tags]'), // User-defined tags cannot have spaces, so no risk of a collision. + + renderContent: function () { + var notesByTag = {}, noTags = this.noTags, addNoteForTag, noteList, tags, i, + sortedTagNames, container, group, noteGroup, tagTitle, titleMap; + + // Iterate through all the notes and build up a dictionary structure by tag. + // Note that the collection will be in most-recently updated order already. + addNoteForTag = function (note, tag) { + noteList = notesByTag[tag.toLowerCase()]; + if (noteList === undefined) { + noteList = []; + notesByTag[tag.toLowerCase()] = noteList; + } + // If a note was tagged with the same tag more than once, don't add again. + // We can assume it would be the last element of the list because we iterate through + // all tags on a given note before moving on to the text note. + if (noteList.length === 0 || noteList[noteList.length - 1] !== note) { + noteList.push(note); + } + }; + + this.collection.each(function(note){ + tags = note.get('tags'); + if (tags.length === 0) { + addNoteForTag(note, noTags); + } + else { + for (i = 0; i < tags.length; i++) { + addNoteForTag(note, tags[i]); + } + } + }); + + sortedTagNames = Object.keys(notesByTag).sort(function (a, b) { + // "no tags" should always appear last + if (a === noTags) { + return 1; + } + else if (b === noTags) { + return -1; + } + else if (notesByTag[a].length > notesByTag[b].length) { + return -1; + } + else if (notesByTag[a].length < notesByTag[b].length) { + return 1; + } + else { + return a.toLowerCase() <= b.toLowerCase() ? -1 : 1; + } + }); + + container = document.createDocumentFragment(); + + // Store map of titles for scrollToTag functionality. + this.titleMap = {}; + titleMap = this.titleMap; + + _.each(sortedTagNames, function (tagName) { + noteGroup = notesByTag[tagName]; + var tagTitle = interpolate_text( + "{tagName} ({numberOfNotesWithTag})", + {tagName: tagName, numberOfNotesWithTag: noteGroup.length} + ); + group = this.getGroup(tagTitle); + titleMap[tagName] = tagTitle; + + group.addChild(this.getNotes(noteGroup)); + container.appendChild(group.render().el); + }, this); + + this.$el.append(container); + return this; + }, + + getGroup: function (tagName) { + var group = new NoteGroupView.GroupView({ + displayName: tagName, + template: '

<%- displayName %>

', + className: "note-group" + }); + this.children.push(group); + return group; + } + }), + + tabInfo: { + // Translators: 'Tags' is the name of the view (noun) within the Student Notes page that shows all + // notes organized by the tags the student has associated with them (if any). When defining a + // note in the courseware, the student can choose to associate 1 or more tags with the note + // in order to group similar notes together and help with search. + name: gettext('Tags'), + identifier: 'view-tags', + icon: 'fa fa-tag' + } + }); + + return TagsView; +}); +}).call(this, define || RequireJS.define); diff --git a/lms/static/js/spec/edxnotes/helpers.js b/lms/static/js/spec/edxnotes/helpers.js index 09cf2634b9..81bf7c4daa 100644 --- a/lms/static/js/spec/edxnotes/helpers.js +++ b/lms/static/js/spec/edxnotes/helpers.js @@ -105,6 +105,7 @@ define(['underscore'], function(_) { }; getDefaultNotes = function () { + // Note that the server returns notes in reverse chronological order (newest first). return [ { chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), @@ -113,7 +114,8 @@ define(['underscore'], function(_) { created: 'December 11, 2014 at 11:12AM', updated: 'December 11, 2014 at 11:12AM', text: 'Third added model', - quote: 'Note 4' + quote: 'Note 4', + tags: ['Pumpkin', 'pumpkin', 'yummy'] }, { chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), @@ -131,7 +133,8 @@ define(['underscore'], function(_) { created: 'December 11, 2014 at 11:11AM', updated: 'December 11, 2014 at 11:11AM', text: 'Second added model', - quote: 'Note 3' + quote: 'Note 3', + tags: ['yummy'] }, { chapter: getChapter('Second Chapter', 0, 1, [1, 'w_n', 0]), @@ -140,7 +143,8 @@ define(['underscore'], function(_) { created: 'December 11, 2014 at 11:10AM', updated: 'December 11, 2014 at 11:10AM', text: 'First added model', - quote: 'Note 2' + quote: 'Note 2', + tags: ['PUMPKIN', 'pie'] }, { chapter: getChapter('First Chapter', 1, 0, [2]), @@ -149,7 +153,8 @@ define(['underscore'], function(_) { created: 'December 11, 2014 at 11:10AM', updated: 'December 11, 2014 at 11:10AM', text: 'First added model', - quote: 'Note 1' + quote: 'Note 1', + tags: ['pie', 'pumpkin'] } ]; }; diff --git a/lms/static/js/spec/edxnotes/views/note_item_spec.js b/lms/static/js/spec/edxnotes/views/note_item_spec.js index 617764812c..8fb25b63c0 100644 --- a/lms/static/js/spec/edxnotes/views/note_item_spec.js +++ b/lms/static/js/spec/edxnotes/views/note_item_spec.js @@ -9,7 +9,7 @@ define([ ) { 'use strict'; describe('EdxNotes NoteItemView', function() { - var getView = function (model) { + var getView = function (model, scrollToTag) { model = new NoteModel(_.defaults(model || {}, { id: 'id-123', user: 'user-123', @@ -23,7 +23,7 @@ define([ } })); - return new NoteItemView({model: model}).render(); + return new NoteItemView({model: model, scrollToTag: scrollToTag}).render(); }; beforeEach(function() { @@ -67,7 +67,19 @@ define([ var view = getView({tags: ["First", "Second"]}); expect(view.$('.reference-title').length).toBe(3); expect(view.$('.reference-title')[2]).toContainText('Tags:'); - expect(view.$('.reference-tags').last()).toContainText('First, Second'); + expect(view.$('a.reference-tags').length).toBe(2); + expect(view.$('a.reference-tags')[0]).toContainText('First'); + expect(view.$('a.reference-tags')[1]).toContainText('Second'); + }); + + it('should handle a click event on the tag', function() { + var scrollToTagSpy = { + scrollToTag: function (tagName){} + }; + spyOn(scrollToTagSpy, 'scrollToTag'); + var view = getView({tags: ["only"]}, scrollToTagSpy.scrollToTag); + view.$('a.reference-tags').click(); + expect(scrollToTagSpy.scrollToTag).toHaveBeenCalledWith("only"); }); it('should log the edx.student_notes.used_unit_link event properly', function () { diff --git a/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js b/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js new file mode 100644 index 0000000000..b06a8e831b --- /dev/null +++ b/lms/static/js/spec/edxnotes/views/tabs/tags_spec.js @@ -0,0 +1,80 @@ +define([ + 'jquery', 'underscore', 'js/common_helpers/template_helpers', 'js/spec/edxnotes/helpers', + 'js/edxnotes/collections/notes', 'js/edxnotes/collections/tabs', + 'js/edxnotes/views/tabs/tags', 'js/spec/edxnotes/custom_matchers', + 'jasmine-jquery' +], function( + $, _, TemplateHelpers, Helpers, NotesCollection, TabsCollection, TagsView, + customMatchers +) { + 'use strict'; + describe('EdxNotes TagsView', function() { + var notes = Helpers.getDefaultNotes(), + getView, getText, getNoteText; + + getText = function (selector) { + return $(selector).map(function () { return _.trim($(this).text()); }).toArray(); + }; + + getNoteText = function (groupIndex) { + return $($('.note-group')[groupIndex]).find('.note-excerpt-p').map(function () { + return _.trim($(this).text()); + }).toArray(); + }; + + getView = function (collection, tabsCollection, options) { + var view; + + options = _.defaults(options || {}, { + el: $('.wrapper-student-notes'), + collection: collection, + tabsCollection: tabsCollection + }); + + view = new TagsView(options); + tabsCollection.at(0).activate(); + + return view; + }; + + beforeEach(function () { + customMatchers(this); + loadFixtures('js/fixtures/edxnotes/edxnotes.html'); + TemplateHelpers.installTemplates([ + 'templates/edxnotes/note-item', 'templates/edxnotes/tab-item' + ]); + + this.collection = new NotesCollection(notes); + this.tabsCollection = new TabsCollection(); + }); + + it('displays a tab and content properly ordered by tag', function () { + var view = getView(this.collection, this.tabsCollection), + tags = getText('.tags-title'), + pumpkinNotes = getNoteText(0), + pieNotes = getNoteText(1), + yummyNotes = getNoteText(2), + noTagsNotes = getNoteText(3); + + expect(this.tabsCollection).toHaveLength(1); + expect(this.tabsCollection.at(0).toJSON()).toEqual({ + name: 'Tags', + identifier: 'view-tags', + icon: 'fa fa-tag', + is_active: true, + is_closable: false + }); + expect(view.$('#tags-panel')).toExist(); + + // Pumpkin notes has the greatest number of notes, and therefore should come first. + // Yummy and pie notes have the same number of notes. They should be sorted alphabetically. + // "no tags" should always be last. + expect(tags).toEqual(['pumpkin (3)', 'pie (2)', 'yummy (2)', '[no tags] (1)']); + + expect(pumpkinNotes).toEqual(['Note 4', 'Note 2', 'Note 1']); + expect(yummyNotes).toEqual(['Note 4', 'Note 3']); + expect(pieNotes).toEqual(['Note 2', 'Note 1']); + expect(noTagsNotes).toEqual(['Note 5']); + }); + }); +}); diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 05b9357134..231effc1af 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -616,6 +616,7 @@ 'lms/include/js/spec/edxnotes/views/tabs/search_results_spec.js', 'lms/include/js/spec/edxnotes/views/tabs/recent_activity_spec.js', 'lms/include/js/spec/edxnotes/views/tabs/course_structure_spec.js', + 'lms/include/js/spec/edxnotes/views/tabs/tags_spec.js', 'lms/include/js/spec/edxnotes/views/visibility_decorator_spec.js', 'lms/include/js/spec/edxnotes/views/toggle_notes_factory_spec.js', 'lms/include/js/spec/edxnotes/models/tab_spec.js', diff --git a/lms/static/sass/course/_student-notes.scss b/lms/static/sass/course/_student-notes.scss index ef7f2a2707..5c92c84653 100644 --- a/lms/static/sass/course/_student-notes.scss +++ b/lms/static/sass/course/_student-notes.scss @@ -108,7 +108,7 @@ $divider-visual-tertiary: ($baseline/20) solid $gray-l4; padding-top: ($baseline*1.5); // course structure labels - .course-title { + .course-title, .tags-title { @extend %t-title6; @extend %t-weight4; margin: 0 0 ($baseline/2) 0; @@ -124,6 +124,11 @@ $divider-visual-tertiary: ($baseline/20) solid $gray-l4; color: $gray-d3; } + .tags-title { + border-bottom: $divider-visual-tertiary; + padding-bottom: ($baseline/2); + } + // individual note .note { @include clearfix(); @@ -228,10 +233,22 @@ $divider-visual-tertiary: ($baseline/20) solid $gray-l4; color: $m-gray-d2; } + // Put commas between tags. + a.reference-meta.reference-tags:after { + content: ","; + color: $m-gray-d2; + } + + // But not after the last tag. + a.reference-meta.reference-tags:last-child:after { + content: ""; + } + // needed for poor base LMS styling scope a.reference-meta { @extend %shame-link-text; } + } } diff --git a/lms/templates/edxnotes/edxnotes.html b/lms/templates/edxnotes/edxnotes.html index 7b91b1da33..7b4ec8db1b 100644 --- a/lms/templates/edxnotes/edxnotes.html +++ b/lms/templates/edxnotes/edxnotes.html @@ -56,6 +56,7 @@ ${_("Loading")} +
% else:
diff --git a/lms/templates/edxnotes/note-item.underscore b/lms/templates/edxnotes/note-item.underscore index f1461dff56..1c9281d4f5 100644 --- a/lms/templates/edxnotes/note-item.underscore +++ b/lms/templates/edxnotes/note-item.underscore @@ -16,28 +16,30 @@ <% if (text) { %>
  1. -

    <%- gettext("You commented...") %>

    +

    <%- gettext("You commented...") %>

    <%= text %>

<% } %> -