fix(Mobile): Bind only to touch or mouse events

Binding to both for the same event causes issues for the grid menu as touch-enabled browsers will fire touch events as well as mouse events subsequently. NOTE: PhantomJS provides touch events though you wouldn't expect it. I've updated the header-cell test specs to allow choosing which sort of event types to trigger.
angular-ui · Dec 10, 2014 · 995d3c4 · 995d3c4
1 parent ac18398
commit 995d3c4
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 16 deletions.
diff --git a/src/js/core/directives/ui-grid-header-cell.js b/src/js/core/directives/ui-grid-header-cell.js
@@ -102,10 +102,10 @@
               $scope.colMenu = false;
             }
 
-            function handleClick(evt) {
+            function handleClick(event) {
               // If the shift key is being held down, add this column to the sort
               var add = false;
-              if (evt.shiftKey) {
+              if (event.shiftKey) {
                 add = true;
               }
 
@@ -140,7 +140,11 @@
               // Long-click (for mobile)
               var cancelMousedownTimeout;
               var mousedownStartTime = 0;
-              $contentsElm.on('mousedown touchstart', function(event) {
+
+              var downEvent = gridUtil.isTouchEnabled() ? 'touchstart' : 'mousedown';
+              $contentsElm.on(downEvent, function(event) {
+                event.stopPropagation();
+
                 if (typeof(event.originalEvent) !== 'undefined' && event.originalEvent !== undefined) {
                   event = event.originalEvent;
                 }
@@ -160,19 +164,20 @@
                   }
                 });
               });
-
-              $contentsElm.on('mouseup touchend', function () {
+
+              var upEvent = gridUtil.isTouchEnabled() ? 'touchend' : 'mouseup';
+              $contentsElm.on(upEvent, function () {
                 $timeout.cancel(cancelMousedownTimeout);
               });
 
               $scope.$on('$destroy', function () {
                 $contentsElm.off('mousedown touchstart');
-              });              
+              });
             }
 
 
-            $scope.toggleMenu = function($event) {
-              $event.stopPropagation();
+            $scope.toggleMenu = function(event) {
+              event.stopPropagation();
 
               // If the menu is already showing...
               if (uiGridCtrl.columnMenuScope.menuShown) {
@@ -196,8 +201,9 @@
 
             // If this column is sortable, add a click event handler
             if ($scope.sortable) {
-              $contentsElm.on('click touchend', function(evt) {
-                evt.stopPropagation();
+              var clickEvent = gridUtil.isTouchEnabled() ? 'touchend' : 'click';
+              $contentsElm.on(clickEvent, function(event) {
+                event.stopPropagation();
 
                 $timeout.cancel(cancelMousedownTimeout);
 
@@ -209,7 +215,7 @@
                 }
                 else {
                   // short click
-                  handleClick(evt);
+                  handleClick(event);
                 }
               });
 

diff --git a/test/unit/core/directives/ui-grid-header-cell.spec.js b/test/unit/core/directives/ui-grid-header-cell.spec.js
@@ -1,5 +1,7 @@
 describe('uiGridHeaderCell', function () {
-  var grid, $scope, $compile, $document, $timeout, $window, recompile, $animate, uiGridConstants, columnDefs;
+  var grid, $scope, $compile, $document, $timeout, $window, recompile, $animate, uiGridConstants, gridUtil, columnDefs;
+
+  var downEvent, upEvent, clickEvent;
 
   var data = [
     { "name": "Ethel Price", "gender": "female", "company": "Enersol" },
@@ -23,14 +25,27 @@ describe('uiGridHeaderCell', function () {
 
   beforeEach(module('ui.grid'));
 
-  beforeEach(inject(function (_$compile_, $rootScope, _$document_, _$timeout_, _$window_, _$animate_, _uiGridConstants_) {
+  beforeEach(inject(function (_$compile_, $rootScope, _$document_, _$timeout_, _$window_, _$animate_, _uiGridConstants_, _gridUtil_) {
     $scope = $rootScope;
     $compile = _$compile_;
     $document = _$document_;
     $timeout = _$timeout_;
     $window = _$window_;
     $animate = _$animate_;
     uiGridConstants = _uiGridConstants_;
+    gridUtil = _gridUtil_;
+
+    // Decide whether to use mouse or touch events based on which capabilities the browser has
+    if (gridUtil.isTouchEnabled()) {
+      downEvent = 'touchstart';
+      upEvent = 'touchend';
+      clickEvent = 'touchstart';
+    }
+    else {
+      downEvent = 'mousedown';
+      upEvent = 'mouseup';
+      clickEvent = 'click';
+    }
 
     $scope.gridOpts = {
       enableSorting: true,
@@ -70,7 +85,7 @@ describe('uiGridHeaderCell', function () {
     });
 
     function openMenu() {
-      headerCell1.trigger('mousedown');
+      headerCell1.trigger(downEvent);
       $scope.$digest();
       $timeout.flush();
       $scope.$digest();
@@ -87,7 +102,7 @@ describe('uiGridHeaderCell', function () {
       it('should do nothing', inject(function() {
         expect(menu.find('.ui-grid-menu-inner').length).toEqual(0, 'column menu is not initially visible');
 
-        headerCell1.trigger({ type: 'mousedown', button: 3 });
+        headerCell1.trigger({ type: downEvent, button: 3 });
 
         $timeout.flush();
         $scope.$digest();
@@ -101,7 +116,7 @@ describe('uiGridHeaderCell', function () {
         openMenu();
         expect(menu.find('.ui-grid-menu-inner').length).toEqual(1, 'column menu is visible');
 
-        $document.trigger('click');
+        $document.trigger(clickEvent);
 
         $timeout.flush();
         $scope.$digest();