Skip to content

Commit 7589efd

Browse files
Michael Chenkennethcachia
Michael Chen
authored and
kennethcachia
committed
fix(tooltip): Fix md-tooltip showing when window receives focus
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 angular#3035. Closes angular#4191.
1 parent 93557c0 commit 7589efd

File tree

2 files changed

+74
-37
lines changed

2 files changed

+74
-37
lines changed

‎src/components/tooltip/tooltip.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,25 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe
134134

135135
function bindEvents () {
136136
var mouseActive = false;
137-
var enterHandler = function() {
137+
138+
var ngWindow = angular.element($window);
139+
140+
// Store whether the element was focused when the window loses focus.
141+
var windowBlurHandler = function() {
142+
elementFocusedOnWindowBlur = document.activeElement === parent[0];
143+
};
144+
var elementFocusedOnWindowBlur = false;
145+
ngWindow.on('blur', windowBlurHandler);
146+
scope.$on('$destroy', function() {
147+
ngWindow.off('blur', windowBlurHandler);
148+
});
149+
150+
var enterHandler = function(e) {
151+
// Prevent the tooltip from showing when the window is receiving focus.
152+
if (e.type === 'focus' && elementFocusedOnWindowBlur) {
153+
elementFocusedOnWindowBlur = false;
154+
return;
155+
}
138156
parent.on('blur mouseleave touchend touchcancel', leaveHandler );
139157
setVisible(true);
140158
};

‎src/components/tooltip/tooltip.spec.js

Lines changed: 55 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ describe('<md-tooltip> directive', function() {
33
var element;
44

55
beforeEach(module('material.components.tooltip'));
6+
beforeEach(module('material.components.button'));
67
beforeEach(inject(function(_$compile_, _$rootScope_, _$animate_, _$timeout_){
78
$compile = _$compile_;
89
$rootScope = _$rootScope_;
@@ -19,7 +20,7 @@ describe('<md-tooltip> directive', function() {
1920
buildTooltip(
2021
'<md-button>' +
2122
'Hello' +
22-
'<md-tooltip md-visible="isVisible">Tooltip</md-tooltip>' +
23+
'<md-tooltip md-visible="testModel.isVisible">Tooltip</md-tooltip>' +
2324
'</md-button>'
2425
);
2526

@@ -29,7 +30,7 @@ describe('<md-tooltip> directive', function() {
2930
it('should label parent', function(){
3031
buildTooltip(
3132
'<md-button>' +
32-
'<md-tooltip md-visible="isVisible">' +
33+
'<md-tooltip md-visible="testModel.isVisible">' +
3334
'Tooltip' +
3435
'</md-tooltip>'+
3536
'</md-button>'
@@ -46,38 +47,38 @@ describe('<md-tooltip> directive', function() {
4647
buildTooltip(
4748
'<outer>' +
4849
'<inner>' +
49-
'<md-tooltip md-visible="isVisible">' +
50+
'<md-tooltip md-visible="testModel.isVisible">' +
5051
'Hello world' +
5152
'</md-tooltip>' +
5253
'</inner>' +
5354
'</outer>'
5455
);
5556

5657
triggerEvent('mouseenter', true);
57-
expect($rootScope.isVisible).toBeUndefined();
58+
expect($rootScope.testModel.isVisible).toBeUndefined();
5859

5960
}));
6061

6162
it('should show after tooltipDelay ms', function() {
6263
buildTooltip(
6364
'<md-button>' +
6465
'Hello' +
65-
'<md-tooltip md-visible="isVisible" md-delay="99">' +
66+
'<md-tooltip md-visible="testModel.isVisible" md-delay="99">' +
6667
'Tooltip' +
6768
'</md-tooltip>' +
6869
'</md-button>'
6970
);
7071

7172
triggerEvent('focus', true);
72-
expect($rootScope.isVisible).toBeFalsy();
73+
expect($rootScope.testModel.isVisible).toBeFalsy();
7374

7475
// Wait 1 below delay, nothing should happen
7576
$timeout.flush(98);
76-
expect($rootScope.isVisible).toBeFalsy();
77+
expect($rootScope.testModel.isVisible).toBeFalsy();
7778

7879
// Total 300 == tooltipDelay
7980
$timeout.flush(1);
80-
expect($rootScope.isVisible).toBe(true);
81+
expect($rootScope.testModel.isVisible).toBe(true);
8182

8283
});
8384

@@ -90,7 +91,7 @@ describe('<md-tooltip> directive', function() {
9091
buildTooltip(
9192
'<md-button>' +
9293
'Hello' +
93-
'<md-tooltip md-visible="isVisible">' +
94+
'<md-tooltip md-visible="testModel.isVisible">' +
9495
'Tooltip' +
9596
'</md-tooltip>' +
9697
'</md-button>'
@@ -111,84 +112,101 @@ describe('<md-tooltip> directive', function() {
111112
buildTooltip(
112113
'<md-button>' +
113114
'Hello' +
114-
'<md-tooltip md-visible="isVisible">' +
115+
'<md-tooltip md-visible="testModel.isVisible">' +
115116
'Tooltip' +
116117
'</md-tooltip>' +
117118
'</md-button>'
118119
);
119120

120121
triggerEvent('mouseenter');
121-
expect($rootScope.isVisible).toBe(true);
122+
expect($rootScope.testModel.isVisible).toBe(true);
122123

123124
triggerEvent('mouseleave');
124-
expect($rootScope.isVisible).toBe(false);
125+
expect($rootScope.testModel.isVisible).toBe(false);
125126
});
126127

127128
it('should set visible on focus and blur', function() {
128129
buildTooltip(
129130
'<md-button>' +
130131
'Hello' +
131-
'<md-tooltip md-visible="isVisible">' +
132+
'<md-tooltip md-visible="testModel.isVisible">' +
132133
'Tooltip' +
133134
'</md-tooltip>' +
134135
'</md-button>'
135136
);
136137

137138
triggerEvent('focus');
138-
expect($rootScope.isVisible).toBe(true);
139+
expect($rootScope.testModel.isVisible).toBe(true);
139140

140141
triggerEvent('blur');
141-
expect($rootScope.isVisible).toBe(false);
142+
expect($rootScope.testModel.isVisible).toBe(false);
142143
});
143144

144145
it('should set visible on touchstart and touchend', function() {
145146
buildTooltip(
146147
'<md-button>' +
147148
'Hello' +
148-
'<md-tooltip md-visible="isVisible">' +
149+
'<md-tooltip md-visible="testModel.isVisible">' +
149150
'Tooltip' +
150151
'</md-tooltip>' +
151152
'</md-button>'
152153
);
153154

154155

155156
triggerEvent('touchstart');
156-
expect($rootScope.isVisible).toBe(true);
157+
expect($rootScope.testModel.isVisible).toBe(true);
157158

158159
triggerEvent('touchend');
159-
expect($rootScope.isVisible).toBe(false);
160+
expect($rootScope.testModel.isVisible).toBe(false);
160161

161162
});
162163

163-
it('should not be visible on mousedown and then mouseleave', inject(function( $document) {
164-
jasmine.mockElementFocus(this);
165-
164+
it('should not be visible on mousedown and then mouseleave', inject(function($document) {
166165
buildTooltip(
167166
'<md-button>' +
168167
'Hello' +
169-
'<md-tooltip md-visible="isVisible">' +
168+
'<md-tooltip md-visible="testModel.isVisible">' +
170169
'Tooltip' +
171170
'</md-tooltip>' +
172171
'</md-button>'
173172
);
174173

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

180-
expect($document.activeElement).toBe(element[0]);
181-
expect($rootScope.isVisible).toBe(true);
179+
expect($document[0].activeElement).toBe(element[0]);
180+
expect($rootScope.testModel.isVisible).toBe(true);
182181

183182
triggerEvent('mouseleave');
183+
expect($rootScope.testModel.isVisible).toBe(false);
184+
}));
184185

185-
// very weak test since this is really always set to false because
186-
// we are not able to set `document.activeElement` to the parent
187-
// of `md-tooltip`. we compensate by testing `$document.activeElement`
188-
// which sort of mocks the behavior through `jasmine.mockElementFocus`
189-
// which should be replaced by a true `document.activeElement` check
190-
// if the problem gets fixed.
191-
expect($rootScope.isVisible).toBe(false);
186+
it('should not be visible when the window is refocused', inject(function($window, $document) {
187+
buildTooltip(
188+
'<md-button>' +
189+
'Hello' +
190+
'<md-tooltip md-visible="testModel.isVisible">' +
191+
'Tooltip' +
192+
'</md-tooltip>' +
193+
'</md-button>'
194+
);
195+
196+
// Append element to DOM so it can be set as activeElement.
197+
$document[0].body.appendChild(element[0]);
198+
element[0].focus();
199+
triggerEvent('focus,mousedown');
200+
expect(document.activeElement).toBe(element[0]);
201+
202+
triggerEvent('mouseleave');
203+
204+
// Simulate tabbing away.
205+
angular.element($window).triggerHandler('blur');
206+
207+
// Simulate focus event that occurs when tabbing back to the window.
208+
triggerEvent('focus');
209+
expect($rootScope.testModel.isVisible).toBe(false);
192210
}));
193211
});
194212

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

201219
element = $compile(markup)($rootScope);
220+
$rootScope.testModel = {};
202221

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

212-
$rootScope.$apply('isVisible = ' + (isVisible ? 'true' : 'false') );
231+
$rootScope.$apply('testModel.isVisible = ' + (isVisible ? 'true' : 'false') );
213232
$animate.triggerCallbacks();
214233
}
215234

0 commit comments

Comments
 (0)