fix(mdTooltip): fix regression that broke tooltips on Firefox

Remove check for pointer-events not equal to none to resolve non-visible tooltips on Firefox. Add Chrome & Firefox as default test runners in karma task to avoid regressions like this in the future. Improve and automate use of ngAnimateMock Fixes #3047. Fixes #3250. Fixes #3430. Closes #3467.
angular · Jul 1, 2015 · 6fc9212 · 6fc9212
1 parent ee60441
commit 6fc9212
Show file tree

Hide file tree

Showing 14 changed files with 41 additions and 33 deletions.
diff --git a/config/karma.conf.js b/config/karma.conf.js
@@ -65,7 +65,7 @@ module.exports = function(config) {
     // - Safari (only Mac; has to be installed with `npm install karma-safari-launcher`)
     // - PhantomJS
     // - IE (only Windows; has to be installed with `npm install karma-ie-launcher`)
-    browsers: ['PhantomJS'],
+    browsers: ['PhantomJS','Firefox'],
 
     // you can define custom flags
     customLaunchers: {

diff --git a/gulp/tasks/karma-fast.js b/gulp/tasks/karma-fast.js
@@ -4,7 +4,7 @@ var util = require('../util');
 var ROOT = require('../const').ROOT;
 var args = util.args;
 
-// NOTE: `karma-fas` does NOT pre-make a full build of JS and CSS
+// NOTE: `karma-fast` does NOT pre-make a full build of JS and CSS
 // exports.dependencies = ['build'];
 
 exports.task = function (done) {
@@ -19,6 +19,7 @@ exports.task = function (done) {
   if ( args.browsers ) {
     karmaConfig.browsers = args.browsers.trim().split(',');
   }
+  // NOTE: `karma-fast` does NOT test Firefox by default.
 
   gutil.log('Running unit tests on unminified source.');
   karma.start(karmaConfig, captureError(clearEnv,clearEnv));

diff --git a/gulp/tasks/karma.js b/gulp/tasks/karma.js
@@ -39,6 +39,8 @@ exports.task = function (done) {
 
   if ( args.browsers ) {
     karmaConfig.browsers = args.browsers.trim().split(',');
+  } else {
+    karmaConfig.browsers = ['Firefox', 'PhantomJS'];
   }
 
   gutil.log('Running unit tests on unminified source.');

diff --git a/src/components/bottomSheet/bottomSheet.spec.js b/src/components/bottomSheet/bottomSheet.spec.js
@@ -1,5 +1,5 @@
 describe('$mdBottomSheet service', function() {
-  beforeEach(module('material.components.bottomSheet', 'ngAnimateMock'));
+  beforeEach(module('material.components.bottomSheet'));
 
   describe('#build()', function() {
     it('should escapeToClose == true', inject(function($mdBottomSheet, $rootScope, $rootElement, $timeout, $animate, $mdConstant) {

diff --git a/src/components/dialog/dialog.spec.js b/src/components/dialog/dialog.spec.js
@@ -1,6 +1,6 @@
 describe('$mdDialog', function() {
 
-  beforeEach(module('material.components.dialog', 'ngAnimateMock'));
+  beforeEach(module('material.components.dialog'));
 
   beforeEach(inject(function spyOnMdEffects($$q, $animate) {
     spyOn($animate, 'leave').and.callFake(function(element) {
@@ -576,7 +576,7 @@ describe('$mdDialog', function() {
 });
 
 describe('$mdDialog with custom interpolation symbols', function() {
-  beforeEach(module('material.components.dialog', 'ngAnimateMock'));
+  beforeEach(module('material.components.dialog'));
 
   beforeEach(module(function($interpolateProvider) {
     $interpolateProvider.startSymbol('[[').endSymbol(']]');

diff --git a/src/components/menu/menu.spec.js b/src/components/menu/menu.spec.js
@@ -1,7 +1,7 @@
 describe('md-menu directive', function () {
   var $mdMenu, $timeout, something;
 
-  beforeEach(module('material.components.menu', 'ngAnimateMock'));
+  beforeEach(module('material.components.menu'));
   beforeEach(inject(function ($mdUtil, $$q, $document, _$mdMenu_, _$timeout_) {
     $mdMenu = _$mdMenu_;
     $timeout = _$timeout_;

diff --git a/src/components/select/select.spec.js b/src/components/select/select.spec.js
@@ -1,7 +1,7 @@
 describe('<md-select>', function() {
 
   beforeEach(module('material.components.input'));
-  beforeEach(module('material.components.select', 'ngAnimateMock'));
+  beforeEach(module('material.components.select'));
 
   beforeEach(inject(function($mdUtil, $$q) {
     $mdUtil.transitionEndPromise = function() {

diff --git a/src/components/sidenav/sidenav.spec.js b/src/components/sidenav/sidenav.spec.js
@@ -1,5 +1,5 @@
 describe('mdSidenav', function() {
-  beforeEach(module('material.components.sidenav', 'ngAnimateMock'));
+  beforeEach(module('material.components.sidenav'));
 
   function setup(attrs) {
     var el;

diff --git a/src/components/toast/toast.spec.js b/src/components/toast/toast.spec.js
@@ -1,5 +1,5 @@
 describe('$mdToast service', function() {
-  beforeEach(module('material.components.toast', 'ngAnimateMock', function($provide) {
+  beforeEach(module('material.components.toast', function($provide) {
   }));
 
   function setup(options) {

diff --git a/src/components/tooltip/tooltip.js b/src/components/tooltip/tooltip.js
@@ -29,7 +29,7 @@ angular
  *
  * @param {expression=} md-visible Boolean bound to whether the tooltip is
  * currently visible.
- * @param {number=} md-delay How many milliseconds to wait to show the tooltip after the user focuses, hovers, or touches the parent. Defaults to 400ms.
+ * @param {number=} md-delay How many milliseconds to wait to show the tooltip after the user focuses, hovers, or touches the parent. Defaults to 300ms.
  * @param {string=} md-direction Which direction would you like the tooltip to go?  Supports left, right, top, and bottom.  Defaults to bottom.
  * @param {boolean=} md-autohide If present or provided with a boolean value, the tooltip will hide on mouse leave, regardless of focus
  */
@@ -66,15 +66,14 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe
         tooltipParent = angular.element(current || document.body),
         debouncedOnResize = $$rAF.throttle(function () { if (scope.visible) positionTooltip(); });
 
-    return init();
+    // Initialize element
+
+    setDefaults();
+    manipulateElement();
+    bindEvents();
+    configureWatchers();
+    addAriaLabel();
 
-    function init () {
-      setDefaults();
-      manipulateElement();
-      bindEvents();
-      configureWatchers();
-      addAriaLabel();
-    }
 
     function setDefaults () {
       if (!angular.isDefined(attr.mdDelay)) scope.delay = TOOLTIP_SHOW_DELAY;
@@ -103,9 +102,12 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe
       element.attr('role', 'tooltip');
     }
 
+    /**
+     * Scan up dom hierarchy for enabled parent;
+     */
     function getParentWithPointerEvents () {
       var parent = element.parent();
-      while (parent && $window.getComputedStyle(parent[0])['pointer-events'] == 'none') {
+      while (parent && hasComputedStyleValue('pointer-events','none', parent[0])) {
         parent = parent.parent();
       }
       return parent;
@@ -120,22 +122,26 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe
        return current;
      }
 
-    function hasComputedStyleValue(key, value) {
-        // Check if we should show it or not...
-        var computedStyles = $window.getComputedStyle(element[0]);
-        return angular.isDefined(computedStyles[key]) && (computedStyles[key] == value);
+
+    function hasComputedStyleValue(key, value, target) {
+      key    = attr.$normalize(key);
+      target = target || element[0];
+
+      var computedStyles = $window.getComputedStyle(target);
+
+      return angular.isDefined(computedStyles[key]) && (computedStyles[key] == value);
     }
 
     function bindEvents () {
       var mouseActive = false;
       var enterHandler = function() {
-        if (!hasComputedStyleValue('pointer-events','none')) {
-          setVisible(true);
-        }
+        parent.on('blur mouseleave touchend touchcancel', leaveHandler );
+        setVisible(true);
       };
       var leaveHandler = function () {
         var autohide = scope.hasOwnProperty('autohide') ? scope.autohide : attr.hasOwnProperty('mdAutohide');
         if (autohide || mouseActive || ($document[0].activeElement !== parent[0]) ) {
+          parent.off('blur mouseleave touchend touchcancel', leaveHandler );
           setVisible(false);
         }
         mouseActive = false;
@@ -144,7 +150,6 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe
       // to avoid `synthetic clicks` we listen to mousedown instead of `click`
       parent.on('mousedown', function() { mouseActive = true; });
       parent.on('focus mouseenter touchstart', enterHandler );
-      parent.on('blur mouseleave touchend touchcancel', leaveHandler );
 
 
       angular.element($window).on('resize', debouncedOnResize);

diff --git a/src/components/tooltip/tooltip.spec.js b/src/components/tooltip/tooltip.spec.js
@@ -2,7 +2,7 @@ describe('<md-tooltip> directive', function() {
   var $compile, $rootScope, $animate, $timeout;
   var element;
 
-  beforeEach(module('material.components.tooltip', 'ngAnimateMock'));
+  beforeEach(module('material.components.tooltip'));
   beforeEach(inject(function(_$compile_, _$rootScope_, _$animate_, _$timeout_){
     $compile   = _$compile_;
     $rootScope = _$rootScope_;
@@ -75,7 +75,7 @@ describe('<md-tooltip> directive', function() {
     $timeout.flush(98);
     expect($rootScope.isVisible).toBeFalsy();
 
-    // Total 99 == tooltipDelay
+    // Total 300 == tooltipDelay
     $timeout.flush(1);
     expect($rootScope.isVisible).toBe(true);
 
@@ -170,7 +170,7 @@ describe('<md-tooltip> directive', function() {
             'Tooltip' +
           '</md-tooltip>' +
         '</md-button>'
-      )
+      );
 
       // this focus is needed to set `$document.activeElement`
       // and wouldn't be required if `document.activeElement` was settable.

diff --git a/src/components/virtualRepeat/virtualRepeater.spec.js b/src/components/virtualRepeat/virtualRepeater.spec.js
@@ -1,5 +1,5 @@
 describe('<md-virtual-repeat>', function() {
-  beforeEach(module('ngMaterial-mock', 'material.components.virtualRepeat'));
+  beforeEach(module('material.components.virtualRepeat'));
 
   var VirtualRepeatController = { NUM_EXTRA : 3 };
 

diff --git a/src/core/services/interimElement/interimElement.spec.js b/src/core/services/interimElement/interimElement.spec.js
@@ -3,7 +3,7 @@ describe('$$interimElement service', function() {
   var $compilerSpy, $themingSpy, resolvingPromise;
 
   function setup() {
-    module('material.core', 'ngAnimateMock', function($provide) {
+    module('material.core', function($provide) {
       var $mdCompiler = { compile: angular.noop };
       $compilerSpy = spyOn($mdCompiler, 'compile');
       $themingSpy = jasmine.createSpy('$mdTheming');

diff --git a/test/angular-material-spec.js b/test/angular-material-spec.js
@@ -34,7 +34,7 @@
      * Before each test, require that the 'ngMaterial-mock' module is ready for injection
      * NOTE: assumes that angular-material-mocks.js has been loaded.
      */
-    module('ngMaterial-mock')
+    module('ngAnimateMock','ngMaterial-mock');
 
     /**
      * Mocks angular.element#focus ONLY for the duration of a particular test.