Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(tooltip): Fix md-tooltip showing when window receives focus
Browse files Browse the repository at this point in the history 
When the parent of an md-tooltip was clicked, it received focus. When
the window was blurred and re-focused (e.g. on tab change), the tooltip
shows again due to a new focus event firing. This fix prevents this
first focus event from having any effect. Note that this bug only
affected browsers that focus buttons on click.

Fixes #3035. Closes #4191.
  • Loading branch information
@ThomasBurleson
Michael Chen authored and ThomasBurleson committed Aug 19, 2015
1 parent e3fcf3c commit 954cab5
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 37 deletions.
20 changes: 19 additions & 1 deletion src/components/tooltip/tooltip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,25 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe

function bindEvents () {
var mouseActive = false;
var enterHandler = function() {

var ngWindow = angular.element($window);

// Store whether the element was focused when the window loses focus.
var windowBlurHandler = function() {
elementFocusedOnWindowBlur = document.activeElement === parent[0];
};
var elementFocusedOnWindowBlur = false;
ngWindow.on('blur', windowBlurHandler);
scope.$on('$destroy', function() {
ngWindow.off('blur', windowBlurHandler);
});

var enterHandler = function(e) {
// Prevent the tooltip from showing when the window is receiving focus.
if (e.type === 'focus' && elementFocusedOnWindowBlur) {
elementFocusedOnWindowBlur = false;
return;
}
parent.on('blur mouseleave touchend touchcancel', leaveHandler );
setVisible(true);
};
Expand Down
91 changes: 55 additions & 36 deletions src/components/tooltip/tooltip.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ describe('<md-tooltip> directive', function() {
var element;

beforeEach(module('material.components.tooltip'));
beforeEach(module('material.components.button'));
beforeEach(inject(function(_$compile_, _$rootScope_, _$animate_, _$timeout_){
$compile = _$compile_;
$rootScope = _$rootScope_;
Expand All @@ -19,7 +20,7 @@ describe('<md-tooltip> directive', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">Tooltip</md-tooltip>' +
'<md-tooltip md-visible="testModel.isVisible">Tooltip</md-tooltip>' +
'</md-button>'
);

Expand All @@ -29,7 +30,7 @@ describe('<md-tooltip> directive', function() {
it('should label parent', function(){
buildTooltip(
'<md-button>' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>'+
'</md-button>'
Expand All @@ -46,38 +47,38 @@ describe('<md-tooltip> directive', function() {
buildTooltip(
'<outer>' +
'<inner>' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Hello world' +
'</md-tooltip>' +
'</inner>' +
'</outer>'
);

triggerEvent('mouseenter', true);
expect($rootScope.isVisible).toBeUndefined();
expect($rootScope.testModel.isVisible).toBeUndefined();

}));

it('should show after tooltipDelay ms', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible" md-delay="99">' +
'<md-tooltip md-visible="testModel.isVisible" md-delay="99">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);

triggerEvent('focus', true);
expect($rootScope.isVisible).toBeFalsy();
expect($rootScope.testModel.isVisible).toBeFalsy();

// Wait 1 below delay, nothing should happen
$timeout.flush(98);
expect($rootScope.isVisible).toBeFalsy();
expect($rootScope.testModel.isVisible).toBeFalsy();

// Total 300 == tooltipDelay
$timeout.flush(1);
expect($rootScope.isVisible).toBe(true);
expect($rootScope.testModel.isVisible).toBe(true);

});

Expand All @@ -90,7 +91,7 @@ describe('<md-tooltip> directive', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
Expand All @@ -111,84 +112,101 @@ describe('<md-tooltip> directive', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);

triggerEvent('mouseenter');
expect($rootScope.isVisible).toBe(true);
expect($rootScope.testModel.isVisible).toBe(true);

triggerEvent('mouseleave');
expect($rootScope.isVisible).toBe(false);
expect($rootScope.testModel.isVisible).toBe(false);
});

it('should set visible on focus and blur', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);

triggerEvent('focus');
expect($rootScope.isVisible).toBe(true);
expect($rootScope.testModel.isVisible).toBe(true);

triggerEvent('blur');
expect($rootScope.isVisible).toBe(false);
expect($rootScope.testModel.isVisible).toBe(false);
});

it('should set visible on touchstart and touchend', function() {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);


triggerEvent('touchstart');
expect($rootScope.isVisible).toBe(true);
expect($rootScope.testModel.isVisible).toBe(true);

triggerEvent('touchend');
expect($rootScope.isVisible).toBe(false);
expect($rootScope.testModel.isVisible).toBe(false);

});

it('should not be visible on mousedown and then mouseleave', inject(function( $document) {
jasmine.mockElementFocus(this);

it('should not be visible on mousedown and then mouseleave', inject(function($document) {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="isVisible">' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);

// this focus is needed to set `$document.activeElement`
// and wouldn't be required if `document.activeElement` was settable.
element.focus();
triggerEvent('focus, mousedown');
// Append element to DOM so it can be set as activeElement.
$document[0].body.appendChild(element[0]);
element[0].focus();
triggerEvent('focus,mousedown');

expect($document.activeElement).toBe(element[0]);
expect($rootScope.isVisible).toBe(true);
expect($document[0].activeElement).toBe(element[0]);
expect($rootScope.testModel.isVisible).toBe(true);

triggerEvent('mouseleave');
expect($rootScope.testModel.isVisible).toBe(false);
}));

// very weak test since this is really always set to false because
// we are not able to set `document.activeElement` to the parent
// of `md-tooltip`. we compensate by testing `$document.activeElement`
// which sort of mocks the behavior through `jasmine.mockElementFocus`
// which should be replaced by a true `document.activeElement` check
// if the problem gets fixed.
expect($rootScope.isVisible).toBe(false);
it('should not be visible when the window is refocused', inject(function($window, $document) {
buildTooltip(
'<md-button>' +
'Hello' +
'<md-tooltip md-visible="testModel.isVisible">' +
'Tooltip' +
'</md-tooltip>' +
'</md-button>'
);

// Append element to DOM so it can be set as activeElement.
$document[0].body.appendChild(element[0]);
element[0].focus();
triggerEvent('focus,mousedown');
expect(document.activeElement).toBe(element[0]);

triggerEvent('mouseleave');

// Simulate tabbing away.
angular.element($window).triggerHandler('blur');

// Simulate focus event that occurs when tabbing back to the window.
triggerEvent('focus');
expect($rootScope.testModel.isVisible).toBe(false);
}));
});

Expand All @@ -199,6 +217,7 @@ describe('<md-tooltip> directive', function() {
function buildTooltip(markup) {

element = $compile(markup)($rootScope);
$rootScope.testModel = {};

$rootScope.$apply();
$animate.triggerCallbacks();
Expand All @@ -209,7 +228,7 @@ describe('<md-tooltip> directive', function() {
function showTooltip(isVisible) {
if (angular.isUndefined(isVisible)) isVisible = true;

$rootScope.$apply('isVisible = ' + (isVisible ? 'true' : 'false') );
$rootScope.$apply('testModel.isVisible = ' + (isVisible ? 'true' : 'false') );
$animate.triggerCallbacks();
}

Expand Down

0 comments on commit 954cab5

Please sign in to comment.