Merge pull request #29 from hypothesis/tab-bar-mousedown

Improve tab bar input handling and accessibility

h/static/scripts/app.js

@@ -135,6 +135,7 @@ module.exports = angular.module('h', [
   .directive('helpLink', require('./directive/help-link'))
   .directive('helpPanel', require('./directive/help-panel'))
   .directive('hAutofocus', require('./directive/h-autofocus'))
+  .directive('hOnTouch', require('./directive/h-on-touch'))
   .directive('hTooltip', require('./directive/h-tooltip'))
   .directive('loggedoutMessage', require('./directive/loggedout-message'))
   .directive('loginControl', require('./directive/login-control'))

h/static/scripts/directive/h-on-touch.js

+'use strict';
+
+/**
+ * Install an event handler on an element.
+ *
+ * The event handler follows the same behavior as the ng-<event name>
+ * directives that Angular includes. This means:
+ *
+ *  - The handler function is passed an object with an $event property
+ *  - The handler function is executed in the context of `$scope.$apply()`
+ *
+ * @param {Element} element
+ * @param {Array<string>} events
+ * @param {Function} handler
+ */
+function addEventHandler($scope, element, events, handler) {
+  var callback = function (event) {
+    $scope.$apply(function () {
+      handler($scope, {$event: event});
+    });
+  };
+  events.forEach(function (name) {
+    element.addEventListener(name, callback);
+  });
+}
+
+/**
+ * A directive which adds an event handler for mouse press or touch to
+ * a directive. This is similar to `ng-click` etc. but reacts either on
+ * mouse press OR touch.
+ */
+// @ngInject
+module.exports = function ($parse) {
+  return {
+    restrict: 'A',
+    link: function ($scope, $element, $attrs) {
+      var fn = $parse($attrs.hOnTouch, null /* interceptor */);
+      addEventHandler(
+        $scope,
+        $element[0],
+        ['click', 'mousedown', 'touchstart'],
+        fn
+      );
+    },
+  };
+};

h/static/scripts/directive/selection-tabs.js

@@ -7,7 +7,7 @@ module.exports = function () {
     bindToController: true,
     controllerAs: 'vm',
     //@ngInject
-    controller: function (annotationUI) {
+    controller: function ($element, annotationUI) {
       this.TAB_ANNOTATIONS = uiConstants.TAB_ANNOTATIONS;
       this.TAB_NOTES = uiConstants.TAB_NOTES;
 

h/static/scripts/directive/test/h-on-touch-test.js

+'use strict';
+
+var angular = require('angular');
+
+var unroll = require('../../test/util').unroll;
+var util = require('./util');
+
+function testComponent() {
+  return {
+    bindToController: true,
+    controllerAs: 'vm',
+    controller: function () {
+      this.tapCount = 0;
+      this.tap = function () {
+        ++this.tapCount;
+      };
+    },
+    restrict: 'E',
+    template: '<div h-on-touch="vm.tap()">Tap me</div>',
+  };
+}
+
+describe('hOnTouch', function () {
+  var testEl;
+
+  before(function () {
+    angular.module('app', [])
+      .directive('hOnTouch', require('../h-on-touch'))
+      .directive('test', testComponent);
+  });
+
+  beforeEach(function () {
+    angular.mock.module('app');
+    testEl = util.createDirective(document, 'test', {});
+  });
+
+  unroll('calls the handler when activated with a "#event" event', function (testCase) {
+    util.sendEvent(testEl[0].querySelector('div'), testCase.event);
+    assert.equal(testEl.ctrl.tapCount, 1);
+  },[{
+    event: 'touchstart',
+  },{
+    event: 'mousedown',
+  },{
+    event: 'click',
+  }]);
+});

h/static/scripts/directive/test/selection-tabs-test.js

@@ -25,7 +25,7 @@ describe('selectionTabs', function () {
         totalNotes: '456',
       });
 
-      var tabs = elem[0].querySelectorAll('li');
+      var tabs = elem[0].querySelectorAll('a');
 
       assert.include(tabs[0].textContent, "Annotations");
       assert.include(tabs[1].textContent, "Notes");
@@ -40,9 +40,8 @@ describe('selectionTabs', function () {
         totalNotes: '456',
       });
 
-      var tabs = elem[0].querySelectorAll('li');
-
-      assert.include(tabs[0].className, "selection-tabs--selected");
+      var tabs = elem[0].querySelectorAll('a');
+      assert.isTrue(tabs[0].classList.contains('is-selected'));
     });
 
     it('should display notes tab as selected', function () {
@@ -52,9 +51,8 @@ describe('selectionTabs', function () {
         totalNotes: '456',
       });
 
-      var tabs = elem[0].querySelectorAll('li');
-
-      assert.include(tabs[1].className, "selection-tabs--selected");
+      var tabs = elem[0].querySelectorAll('a');
+      assert.isTrue(tabs[1].classList.contains('is-selected'));
     });
   });
 });

h/static/styles/selection-tabs.scss

@@ -28,16 +28,19 @@
   padding-right: 5px;
 }
 
-.selection-tabs--selected {
-  color: $grey-6;
-  font-weight: bold;
-}
-
 .selection-tabs__type {
+  color: $grey-6;
   margin-right: 20px;
   cursor: pointer;
   min-width: 85px;
   min-height: 18px;
+
+  // Disable focus ring for selected tab
+  outline: none;
+}
+
+.selection-tabs__type.is-selected {
+  font-weight: bold;
 }
 
 .selection-tabs__count {

h/templates/client/selection_tabs.html

 <!-- Tabbed display of annotations and notes. -->
-<ul class="selection-tabs">
-  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.TAB_ANNOTATIONS}" ng-click="vm.selectTab('annotation')">
+<div class="selection-tabs">
+  <a class="selection-tabs__type"
+     href="#"
+     ng-class="{'is-selected': vm.selectedTab === vm.TAB_ANNOTATIONS}"
+     h-on-touch="vm.selectTab('annotation')">
     Annotations
     <span class="selection-tabs__count" ng-if="vm.totalAnnotations > 0">{{ vm.totalAnnotations }}</sup>
-  </li>
-  <li class="selection-tabs__type" ng-class="{'selection-tabs--selected': vm.selectedTab === vm.TAB_NOTES}" ng-click="vm.selectTab('note')">
+  </a>
+  <a class="selection-tabs__type"
+     href="#"
+     ng-class="{'is-selected': vm.selectedTab === vm.TAB_NOTES}"
+     h-on-touch="vm.selectTab('note')">
     Notes
     <span class="selection-tabs__count" ng-if="vm.totalNotes > 0">{{ vm.totalNotes }}</sup>
-  </li>
-</ul>
+  </a>
+</div>
 <div ng-if="!vm.isLoading()">
   <div ng-if="vm.selectedTab === vm.TAB_NOTES && vm.totalNotes === 0"  class="annotation-unavailable-message">
     <p class="annotation-unavailable-message__label">