From 6fc9212cb96cd3542015eea57fad20a1b6cf24af Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Thu, 25 Jun 2015 23:24:50 -0400 Subject: [PATCH] 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. --- config/karma.conf.js | 2 +- gulp/tasks/karma-fast.js | 3 +- gulp/tasks/karma.js | 2 + .../bottomSheet/bottomSheet.spec.js | 2 +- src/components/dialog/dialog.spec.js | 4 +- src/components/menu/menu.spec.js | 2 +- src/components/select/select.spec.js | 2 +- src/components/sidenav/sidenav.spec.js | 2 +- src/components/toast/toast.spec.js | 2 +- src/components/tooltip/tooltip.js | 41 +++++++++++-------- src/components/tooltip/tooltip.spec.js | 6 +-- .../virtualRepeat/virtualRepeater.spec.js | 2 +- .../interimElement/interimElement.spec.js | 2 +- test/angular-material-spec.js | 2 +- 14 files changed, 41 insertions(+), 33 deletions(-) diff --git a/config/karma.conf.js b/config/karma.conf.js index 479d9fdfd73..a6292135965 100644 --- 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 index 144bf57d037..d9db0b75fa3 100644 --- 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 index 7cb26edccd5..ba9615e82a6 100644 --- 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 index 19545426489..c3e57c840ca 100644 --- 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 index 42d49dcf985..104556e8d4b 100644 --- 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 index 2e26ffb22cb..6fd2bb2e7a1 100644 --- 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 index cc8aaf9c68c..c552513d23a 100755 --- a/src/components/select/select.spec.js +++ b/src/components/select/select.spec.js @@ -1,7 +1,7 @@ describe('', 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 index 95e91c4d85b..4c006b2ffd9 100644 --- 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 index e94cc0225dc..6b640b5d781 100644 --- 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 index 3c62cea0a8b..7a3687c5c32 100644 --- 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 index 15978b985dc..197b8549cfb 100644 --- a/src/components/tooltip/tooltip.spec.js +++ b/src/components/tooltip/tooltip.spec.js @@ -2,7 +2,7 @@ describe(' 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(' 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(' directive', function() { 'Tooltip' + '' + '' - ) + ); // 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 index 6e96228158a..69d9c15ed50 100644 --- a/src/components/virtualRepeat/virtualRepeater.spec.js +++ b/src/components/virtualRepeat/virtualRepeater.spec.js @@ -1,5 +1,5 @@ describe('', 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 index 9d8b4783ede..62144f97c0d 100644 --- 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 index 0377775fd71..0643a1c32eb 100644 --- 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.