Respond to code review feedback

analytics_dashboard/static/apps/learners/app/views/root.js

@@ -15,31 +15,42 @@ define(function (require) {
 
     LearnersRootView = Marionette.LayoutView.extend({
         template: _.template(rootTemplate),
+
         regions: {
             error: '.learners-error-region',
             header: '.learners-header-region',
             main: '.learners-main-region'
         },
+
         childEvents: {
             appError: 'onAppError',
-            clearError: 'onClearError'
+            clearError: 'onClearError',
+            setFocusToTop: 'onSetFocusToTop'
         },
+
         initialize: function (options) {
             this.options = options || {};
         },
+
         onRender: function () {
             this.showChildView('header', new HeaderView({
                 model: this.options.pageModel
             }));
         },
+
         onAppError: function (childView, errorMessage) {
             this.showChildView('error', new AlertView({
                 alertType: 'error',
                 title: errorMessage
             }));
         },
+
         onClearError: function () {
             this.getRegion('error').empty();
+        },
+
+        onSetFocusToTop: function () {
+            this.$('#learner-app-focusable').focus();
         }
     });
 

analytics_dashboard/static/apps/learners/app/views/spec/root-spec.js

@@ -40,5 +40,12 @@ define(function (require) {
             this.rootView.triggerMethod('clearError', 'This is the error copy');
             expect(this.rootView.$('.learners-error-region')).not.toHaveText('This is the error copy');
         });
+
+        it('sets focus on setFocusToTop events', function () {
+            var childView = new Marionette.View();
+            this.rootView.showChildView('main', childView);
+            childView.triggerMethod('setFocusToTop');
+            expect(this.rootView.$('#learner-app-focusable')).toBeFocused();
+        });
     });
 });

analytics_dashboard/static/apps/learners/roster/views/filter.js

@@ -51,10 +51,11 @@ define(function (require) {
 
         templateHelpers: function () {
             // 'filterValues' is an array of objects, each having a 'name' key
-            // and a 'displayName' key.  'name' is the canonical name for the
-            // filter, while 'displayName' is the user-facing representation of
-            // the filter which combines the filter with the number of users
-            // belonging to it.
+            // and a 'displayName' key.  'name' is the name of the filter value
+            // (e.g. the cohort name when filtering by cohort), while
+            // 'displayName' is the user-facing representation of the filter
+            // which combines the filter with the number of users belonging to
+            // it.
             var catchAllFilterValue,
                 filterValues,
                 selectedFilterValue;
@@ -65,12 +66,18 @@ define(function (require) {
                         numLearners = filterPair[1];
                     return {
                         name: name,
-                        displayName: _.template('<%= name %> (<%= numLearners %>)')({
+                        displayName: _.template(
+                            // jshint ignore:start
+                            // Translators: 'name' here refers to the name of the filter, while 'numLearners' refers to the number of learners belonging to that filter.
+                            gettext('<%= name %> (<%= numLearners %>)')
+                            // jshint ignore:end
+                        )({
                             name: name,
                             numLearners: Utils.localizeNumber(numLearners, 0)
                         })
                     };
                 })
+                .sortBy('name')
                 .value();
 
             if (filterValues.length) {
@@ -109,7 +116,7 @@ define(function (require) {
                 $(event.currentTarget).find('option:selected').val()
             );
             this.collection.refresh();
-            $('#learner-app-focusable').focus();
+            this.triggerMethod('setFocusToTop');
         }
     });
 

analytics_dashboard/static/apps/learners/roster/views/spec/roster-spec.js

@@ -507,6 +507,34 @@ define(function (require) {
                 expect($('option[value="' + filterValue + '"]')).toBeSelected();
             };
 
+            it('renders filters in alphabetical order', function () {
+                var options,
+                    rosterView;
+                rosterView = getRosterView({courseMetadata: {
+                    cohorts: {
+                        zebra: 1,
+                        antelope: 2
+                    }
+                }});
+                options = rosterView.$('.learners-filter option');
+                expect(options[1]).toHaveText('antelope (2)');
+                expect(options[2]).toHaveText('zebra (1)');
+            });
+
+            it('sets focus after executing a filter', function () {
+                var rosterView,
+                    spy,
+                    cohortFilterView;
+                rosterView = getRosterView({courseMetadata: {cohorts: {
+                    mudskipper: 1
+                }}});
+                spy = {onFocusTriggered: function () {}};
+                cohortFilterView = rosterView.controls.currentView.cohortFilter.currentView;
+                spyOn(cohortFilterView, 'triggerMethod');
+                expectCanFilterBy('cohort', 'mudskipper');
+                expect(cohortFilterView.triggerMethod).toHaveBeenCalledWith('setFocusToTop');
+            });
+
             SpecHelpers.withConfiguration({
                 'by cohort': [
                     'cohort', // filter field name
@@ -560,6 +588,7 @@ define(function (require) {
 
                     _.chain(this.filterOptions)
                         .pairs()
+                        .sortBy(0) // we expect the filter options to appear in alphabetical order
                         .zip(_.rest(selectOptions))
                         .each(function (filterAndSelectOption) {
                             var filterOption = filterAndSelectOption[0],