From 667e4c244ccd9a1dfef6894a64b6df0c5e2f6305 Mon Sep 17 00:00:00 2001 From: Thomas Burleson Date: Fri, 29 May 2015 12:34:17 -0500 Subject: [PATCH] fix(tooltip): pointer-events 'none' used properly with activate events Mouseenter, focus, and touchstart properly checks computed style and will not allow tooltips to be shown when pointer-events == 'none' --- src/components/tooltip/tooltip.js | 19 +++++-- src/components/tooltip/tooltip.spec.js | 72 ++++++++++++-------------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/components/tooltip/tooltip.js b/src/components/tooltip/tooltip.js index 2c9bbdde966..051daebc3d6 100644 --- a/src/components/tooltip/tooltip.js +++ b/src/components/tooltip/tooltip.js @@ -104,7 +104,7 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe function getParentWithPointerEvents () { var parent = element.parent(); - while ($window.getComputedStyle(parent[0])['pointer-events'] == 'none') { + while (parent && $window.getComputedStyle(parent[0])['pointer-events'] == 'none') { parent = parent.parent(); } return parent; @@ -120,8 +120,19 @@ 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 bindEvents () { var mouseActive = false; + var enterHandler = function() { + if (!hasComputedStyleValue('pointer-events','none')) { + setVisible(true); + } + }; var leaveHandler = function () { var autohide = scope.hasOwnProperty('autohide') ? scope.autohide : attr.hasOwnProperty('mdAutohide'); if ($document[0].activeElement !== parent[0] || autohide || mouseActive) { @@ -132,7 +143,7 @@ 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', function() { setVisible(true); }); + parent.on('focus mouseenter touchstart', enterHandler ); parent.on('blur mouseleave touchend touchcancel', leaveHandler ); @@ -161,8 +172,8 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe // Check if we should display it or not. // This handles hide-* and show-* along with any user defined css - var computedStyles = $window.getComputedStyle(element[0]); - if (angular.isDefined(computedStyles.display) && computedStyles.display == 'none') { + if ( hasComputedStyleValue('display','none') ) { + scope.visible = false; element.detach(); return; } diff --git a/src/components/tooltip/tooltip.spec.js b/src/components/tooltip/tooltip.spec.js index 886472a55ad..34a677719f1 100644 --- a/src/components/tooltip/tooltip.spec.js +++ b/src/components/tooltip/tooltip.spec.js @@ -1,12 +1,13 @@ describe(' directive', function() { - var $compile, $rootScope, $animate; + var $compile, $rootScope, $animate, $timeout; var element; beforeEach(module('material.components.tooltip', 'ngAnimateMock')); - beforeEach(inject(function(_$compile_, _$rootScope_, _$animate_){ + beforeEach(inject(function(_$compile_, _$rootScope_, _$animate_, _$timeout_){ $compile = _$compile_; $rootScope = _$rootScope_; $animate = _$animate_; + $timeout = _$timeout_; })); afterEach(function() { // Make sure to remove/cleanup after each test @@ -39,7 +40,7 @@ describe(' directive', function() { it('should not set parent to items with no pointer events', inject(function($window){ spyOn($window, 'getComputedStyle').and.callFake(function(el) { - return { 'pointer-events': el.nodeName == 'INNER' ? 'none' : '' }; + return { 'pointer-events': el ? 'none' : '' }; }); buildTooltip( @@ -52,12 +53,12 @@ describe(' directive', function() { '' ); - element.triggerHandler('mouseenter'); + simulateEvent('mouseenter', true); expect($rootScope.isVisible).toBeUndefined(); })); - it('should show after tooltipDelay ms', inject(function($timeout) { + it('should show after tooltipDelay ms', function() { buildTooltip( '' + 'Hello' + @@ -78,7 +79,7 @@ describe(' directive', function() { $timeout.flush(1); expect($rootScope.isVisible).toBe(true); - })); + }); describe('show and hide', function() { @@ -106,7 +107,7 @@ describe(' directive', function() { expect(findTooltip().length).toBe(0); }); - it('should set visible on mouseenter and mouseleave', inject(function($timeout) { + it('should set visible on mouseenter and mouseleave', function() { buildTooltip( '' + 'Hello' + @@ -116,18 +117,14 @@ describe(' directive', function() { '' ); - element.triggerHandler('mouseenter'); - $timeout.flush(); - - expect($rootScope.isVisible).toBe(true); - - element.triggerHandler('mouseleave'); - $timeout.flush(); + simulateEvent('mouseenter'); + expect($rootScope.isVisible).toBe(true); - expect($rootScope.isVisible).toBe(false); - })); + simulateEvent('mouseleave'); + expect($rootScope.isVisible).toBe(false); + }); - it('should set visible on focus and blur', inject(function($timeout) { + it('should set visible on focus and blur', function() { buildTooltip( '' + 'Hello' + @@ -137,18 +134,14 @@ describe(' directive', function() { '' ); - element.triggerHandler('focus'); - $timeout.flush(); - + simulateEvent('focus'); expect($rootScope.isVisible).toBe(true); - element.triggerHandler('blur'); - $timeout.flush(); - + simulateEvent('blur'); expect($rootScope.isVisible).toBe(false); - })); + }); - it('should set visible on touchstart and touchend', inject(function($timeout) { + it('should set visible on touchstart and touchend', function() { buildTooltip( '' + 'Hello' + @@ -159,18 +152,15 @@ describe(' directive', function() { ); - element.triggerHandler('touchstart'); - $timeout.flush(); - - expect($rootScope.isVisible).toBe(true); + simulateEvent('touchstart'); + expect($rootScope.isVisible).toBe(true); - element.triggerHandler('touchend'); - $timeout.flush(); + simulateEvent('touchend'); + expect($rootScope.isVisible).toBe(false); - expect($rootScope.isVisible).toBe(false); - })); + }); - it('should not be visible on mousedown and then mouseleave', inject(function($timeout, $document) { + it('should not be visible on mousedown and then mouseleave', inject(function( $document) { jasmine.mockElementFocus(this); buildTooltip( @@ -185,15 +175,12 @@ describe(' directive', function() { // this focus is needed to set `$document.activeElement` // and wouldn't be required if `document.activeElement` was settable. element.focus(); - element.triggerHandler('focus'); - element.triggerHandler('mousedown'); - $timeout.flush(); + simulateEvent('focus, mousedown'); expect($document.activeElement).toBe(element[0]); expect($rootScope.isVisible).toBe(true); - element.triggerHandler('mouseleave'); - $timeout.flush(); + simulateEvent('mouseleave'); // very weak test since this is really always set to false because // we are not able to set `document.activeElement` to the parent @@ -228,4 +215,11 @@ describe(' directive', function() { } + function simulateEvent(eventType, skipFlush) { + angular.forEach(eventType.split(','),function(name) { + element.triggerHandler(name); + }); + !skipFlush && $timeout.flush(); + } + });