Merge pull request #7923 from edx/christina/show-notes

Tags Tab in Student Notes

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):

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)
 

lms/static/js/edxnotes/models/tab.js

 ;(function (define, undefined) {
 'use strict';
-define(['backbone'], function (Backbone) {
+define(['underscore', 'backbone'], function (_, Backbone) {
     var TabModel = Backbone.Model.extend({
         defaults: {
             'identifier': '',

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('<h4 class="course-subtitle"><%- sectionName %></h4>'),
+
+        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: '<h4 class="course-subtitle"><%- displayName %></h4>',
+                    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);

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;
         },

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.

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();
 
-            this.recentActivityView = new RecentActivityView({
+            // 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,
+                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});

lms/static/js/edxnotes/views/tab_item.js

 ;(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',

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;
                 });

lms/static/js/edxnotes/views/tab_view.js

 ;(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 () {

lms/static/js/edxnotes/views/tabs/course_structure.js

 ;(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
                 });

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()));

lms/static/js/edxnotes/views/tabs/search_results.js

 ;(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({

lms/static/js/edxnotes/views/tabs/tags.js

+;(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: '<h3 class="tags-title sr-is-focusable" tabindex="-1"><%- displayName %></h3>',
+                    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);

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']
             }
         ];
     };

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 () {

lms/static/js/spec/edxnotes/views/tabs/tags_spec.js

+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']);
+        });
+    });
+});

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',

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;
           }
+
         }
       }
 

lms/templates/edxnotes/edxnotes.html

@@ -56,6 +56,7 @@
           </span>
           <span class="copy">${_("Loading")}</span>
         </div>
+        <div class="sr-is-focusable sr-tab-panel" tabindex="-1"></div>
 
         % else:
         <section class="placeholder is-empty">

lms/templates/edxnotes/note-item.underscore

@@ -16,28 +16,30 @@
     <% if (text) { %>
     <ol class="note-comments" role="region" aria-label="<%- gettext('Note') %>">
         <li class="note-comment">
-            <h3 class="note-comment-title"><%- gettext("You commented...") %></h3>
+            <p class="note-comment-title"><%- gettext("You commented...") %></p>
             <p class="note-comment-p"><%= text %></p>
         </li>
     </ol>
     <% } %>
 </div>
 
-<footer class="reference" role="complementary" aria-label="<%- gettext('References') %>">
+<footer class="reference">
     <div class="wrapper-reference-content">
-        <h3 class="reference-title"><%- gettext("Noted in:") %></h3>
+        <p class="reference-title"><%- gettext("Noted in:") %></p>
         <% if (unit.url) { %>
           <a class="reference-meta reference-unit-link" href="<%= unit.url %>#<%= id %>"><%- unit.display_name %></a>
         <% } else { %>
           <span class="reference-meta"><%- unit.display_name %></span>
         <% } %>
 
-        <h3 class="reference-title"><%- gettext("Last Edited:") %></h3>
+        <p class="reference-title"><%- gettext("Last Edited:") %></p>
         <span class="reference-meta reference-updated-date"><%- updated %></span>
 
         <% if (tags.length > 0) { %>
-            <h3 class="reference-title"><%- gettext("Tags:") %></h3>
-            <span class="reference-meta reference-tags"><%- tags.join(", ") %></span>
+            <p class="reference-title"><%- gettext("Tags:") %></p>
+            <% for (var i = 0; i < tags.length; i++) { %>
+                <a class="reference-meta reference-tags" href="#"><%- tags[i] %></a>
+            <% } %>
         <% } %>
     </div>
 </footer>

lms/templates/edxnotes/tab-item.underscore

 <% var hasIcon = icon ? 1 : 0; %>
 
 <a class="tab-label <% if (hasIcon) { print('has-icon') } %>" href="#">
-  <% if (hasIcon) { %><i class="icon <%= icon %>"></i> <% } %><%- gettext(name) %>
+  <% if (hasIcon) { %><i class="icon <%= icon %>" aria-hidden="true"></i> <% } %><%- gettext(name) %>
 </a>
 
 <% if (is_closable) { %>