Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Commit c0df320

Browse files
chrisirhcpkozlowski-opensource
authored andcommittedDec 31, 2013
fix(tooltip): performance and scope fixes
Isolate scope contents should be the same after hiding and showing the tooltip. The isolate scope's parent should also always be set to the directive's scope correctly. fix(tooltip): link on demand - Calling $digest is enough as we only need to digest the watchers in this scope and its children. No need to call $apply. - Set invokeApply to false on $timeout for popUpDelay - No need to test for cached reference when tooltip isn't visible as the tooltip has no scope. Closes #1450 Closes #1191 Closes #1455
1 parent c4d0e2a commit c0df320

File tree

2 files changed

+235
-195
lines changed

2 files changed

+235
-195
lines changed
 

‎src/tooltip/test/tooltip.spec.js

+27-8
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe('tooltip', function() {
9898
scope.alt = "Alt Message";
9999

100100
elmBody = $compile( angular.element(
101-
'<div><span alt={{alt}} tooltip="{{tooltipMsg}}">Selector Text</span></div>'
101+
'<div><span alt={{alt}} tooltip="{{tooltipMsg}}" tooltip-animation="false">Selector Text</span></div>'
102102
) )( scope );
103103

104104
$compile( elmBody )( scope );
@@ -114,6 +114,13 @@ describe('tooltip', function() {
114114
expect( ttScope.content ).toBe( scope.tooltipMsg );
115115

116116
elm.trigger( 'mouseleave' );
117+
118+
//Isolate scope contents should be the same after hiding and showing again (issue 1191)
119+
elm.trigger( 'mouseenter' );
120+
121+
ttScope = angular.element( elmBody.children()[1] ).scope();
122+
expect( ttScope.placement ).toBe( 'top' );
123+
expect( ttScope.content ).toBe( scope.tooltipMsg );
117124
}));
118125

119126
it('should not show tooltips if there is nothing to show - issue #129', inject(function ($compile) {
@@ -136,6 +143,24 @@ describe('tooltip', function() {
136143
expect( elmBody.children().length ).toBe( 0 );
137144
}));
138145

146+
it('issue 1191 - isolate scope on the popup should always be child of correct element scope', inject( function ( $compile ) {
147+
var ttScope;
148+
elm.trigger( 'mouseenter' );
149+
150+
ttScope = angular.element( elmBody.children()[1] ).scope();
151+
expect( ttScope.$parent ).toBe( elmScope );
152+
153+
elm.trigger( 'mouseleave' );
154+
155+
// After leaving and coming back, the scope's parent should be the same
156+
elm.trigger( 'mouseenter' );
157+
158+
ttScope = angular.element( elmBody.children()[1] ).scope();
159+
expect( ttScope.$parent ).toBe( elmScope );
160+
161+
elm.trigger( 'mouseleave' );
162+
}));
163+
139164
describe('with specified enable expression', function() {
140165

141166
beforeEach(inject(function ($compile) {
@@ -323,18 +348,12 @@ describe('tooltip', function() {
323348

324349
elm = elmBody.find('input');
325350
elmScope = elm.scope();
351+
elm.trigger('fooTrigger');
326352
tooltipScope = elmScope.$$childTail;
327353
}));
328354

329-
it( 'should not contain a cached reference', function() {
330-
expect( inCache() ).toBeTruthy();
331-
elmScope.$destroy();
332-
expect( inCache() ).toBeFalsy();
333-
});
334-
335355
it( 'should not contain a cached reference when visible', inject( function( $timeout ) {
336356
expect( inCache() ).toBeTruthy();
337-
elm.trigger('fooTrigger');
338357
elmScope.$destroy();
339358
expect( inCache() ).toBeFalsy();
340359
}));

‎src/tooltip/tooltip.js

+208-187
Original file line numberDiff line numberDiff line change
@@ -108,224 +108,245 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
108108
return {
109109
restrict: 'EA',
110110
scope: true,
111-
link: function link ( scope, element, attrs ) {
112-
var tooltip = $compile( template )( scope );
113-
var transitionTimeout;
114-
var popupTimeout;
115-
var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false;
116-
var triggers = getTriggers( undefined );
117-
var hasRegisteredTriggers = false;
118-
var hasEnableExp = angular.isDefined(attrs[prefix+'Enable']);
119-
120-
var positionTooltip = function (){
121-
var position,
122-
ttWidth,
123-
ttHeight,
124-
ttPosition;
125-
// Get the position of the directive element.
126-
position = appendToBody ? $position.offset( element ) : $position.position( element );
127-
128-
// Get the height and width of the tooltip so we can center it.
129-
ttWidth = tooltip.prop( 'offsetWidth' );
130-
ttHeight = tooltip.prop( 'offsetHeight' );
131-
132-
// Calculate the tooltip's top and left coordinates to center it with
133-
// this directive.
134-
switch ( scope.tt_placement ) {
135-
case 'right':
136-
ttPosition = {
137-
top: position.top + position.height / 2 - ttHeight / 2,
138-
left: position.left + position.width
139-
};
140-
break;
141-
case 'bottom':
142-
ttPosition = {
143-
top: position.top + position.height,
144-
left: position.left + position.width / 2 - ttWidth / 2
145-
};
146-
break;
147-
case 'left':
148-
ttPosition = {
149-
top: position.top + position.height / 2 - ttHeight / 2,
150-
left: position.left - ttWidth
151-
};
152-
break;
153-
default:
154-
ttPosition = {
155-
top: position.top - ttHeight,
156-
left: position.left + position.width / 2 - ttWidth / 2
157-
};
158-
break;
111+
compile: function (tElem, tAttrs) {
112+
var tooltipLinker = $compile( template );
113+
114+
return function link ( scope, element, attrs ) {
115+
var tooltip;
116+
var transitionTimeout;
117+
var popupTimeout;
118+
var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false;
119+
var triggers = getTriggers( undefined );
120+
var hasRegisteredTriggers = false;
121+
var hasEnableExp = angular.isDefined(attrs[prefix+'Enable']);
122+
123+
var positionTooltip = function (){
124+
var position,
125+
ttWidth,
126+
ttHeight,
127+
ttPosition;
128+
// Get the position of the directive element.
129+
position = appendToBody ? $position.offset( element ) : $position.position( element );
130+
131+
// Get the height and width of the tooltip so we can center it.
132+
ttWidth = tooltip.prop( 'offsetWidth' );
133+
ttHeight = tooltip.prop( 'offsetHeight' );
134+
135+
// Calculate the tooltip's top and left coordinates to center it with
136+
// this directive.
137+
switch ( scope.tt_placement ) {
138+
case 'right':
139+
ttPosition = {
140+
top: position.top + position.height / 2 - ttHeight / 2,
141+
left: position.left + position.width
142+
};
143+
break;
144+
case 'bottom':
145+
ttPosition = {
146+
top: position.top + position.height,
147+
left: position.left + position.width / 2 - ttWidth / 2
148+
};
149+
break;
150+
case 'left':
151+
ttPosition = {
152+
top: position.top + position.height / 2 - ttHeight / 2,
153+
left: position.left - ttWidth
154+
};
155+
break;
156+
default:
157+
ttPosition = {
158+
top: position.top - ttHeight,
159+
left: position.left + position.width / 2 - ttWidth / 2
160+
};
161+
break;
162+
}
163+
164+
ttPosition.top += 'px';
165+
ttPosition.left += 'px';
166+
167+
// Now set the calculated positioning.
168+
tooltip.css( ttPosition );
169+
170+
};
171+
172+
// By default, the tooltip is not open.
173+
// TODO add ability to start tooltip opened
174+
scope.tt_isOpen = false;
175+
176+
function toggleTooltipBind () {
177+
if ( ! scope.tt_isOpen ) {
178+
showTooltipBind();
179+
} else {
180+
hideTooltipBind();
181+
}
159182
}
160183

161-
ttPosition.top += 'px';
162-
ttPosition.left += 'px';
184+
// Show the tooltip with delay if specified, otherwise show it immediately
185+
function showTooltipBind() {
186+
if(hasEnableExp && !scope.$eval(attrs[prefix+'Enable'])) {
187+
return;
188+
}
189+
if ( scope.tt_popupDelay ) {
190+
popupTimeout = $timeout( show, scope.tt_popupDelay, false );
191+
popupTimeout.then(function(reposition){reposition();});
192+
} else {
193+
show()();
194+
}
195+
}
163196

164-
// Now set the calculated positioning.
165-
tooltip.css( ttPosition );
197+
function hideTooltipBind () {
198+
scope.$apply(function () {
199+
hide();
200+
});
201+
}
166202

167-
};
203+
// Show the tooltip popup element.
204+
function show() {
168205

169-
// By default, the tooltip is not open.
170-
// TODO add ability to start tooltip opened
171-
scope.tt_isOpen = false;
172206

173-
function toggleTooltipBind () {
174-
if ( ! scope.tt_isOpen ) {
175-
showTooltipBind();
176-
} else {
177-
hideTooltipBind();
178-
}
179-
}
180-
181-
// Show the tooltip with delay if specified, otherwise show it immediately
182-
function showTooltipBind() {
183-
if(hasEnableExp && !scope.$eval(attrs[prefix+'Enable'])) {
184-
return;
185-
}
186-
if ( scope.tt_popupDelay ) {
187-
popupTimeout = $timeout( show, scope.tt_popupDelay );
188-
popupTimeout.then(function(reposition){reposition();});
189-
} else {
190-
scope.$apply( show )();
191-
}
192-
}
207+
// Don't show empty tooltips.
208+
if ( ! scope.tt_content ) {
209+
return angular.noop;
210+
}
193211

194-
function hideTooltipBind () {
195-
scope.$apply(function () {
196-
hide();
197-
});
198-
}
199-
200-
// Show the tooltip popup element.
201-
function show() {
212+
createTooltip();
202213

214+
// If there is a pending remove transition, we must cancel it, lest the
215+
// tooltip be mysteriously removed.
216+
if ( transitionTimeout ) {
217+
$timeout.cancel( transitionTimeout );
218+
}
203219

204-
// Don't show empty tooltips.
205-
if ( ! scope.tt_content ) {
206-
return angular.noop;
207-
}
220+
// Set the initial positioning.
221+
tooltip.css({ top: 0, left: 0, display: 'block' });
208222

209-
// If there is a pending remove transition, we must cancel it, lest the
210-
// tooltip be mysteriously removed.
211-
if ( transitionTimeout ) {
212-
$timeout.cancel( transitionTimeout );
213-
}
214-
215-
// Set the initial positioning.
216-
tooltip.css({ top: 0, left: 0, display: 'block' });
217-
218-
// Now we add it to the DOM because need some info about it. But it's not
219-
// visible yet anyway.
220-
if ( appendToBody ) {
221-
$document.find( 'body' ).append( tooltip );
222-
} else {
223-
element.after( tooltip );
223+
// Now we add it to the DOM because need some info about it. But it's not
224+
// visible yet anyway.
225+
if ( appendToBody ) {
226+
$document.find( 'body' ).append( tooltip );
227+
} else {
228+
element.after( tooltip );
229+
}
230+
231+
positionTooltip();
232+
233+
// And show the tooltip.
234+
scope.tt_isOpen = true;
235+
scope.$digest(); // digest required as $apply is not called
236+
237+
// Return positioning function as promise callback for correct
238+
// positioning after draw.
239+
return positionTooltip;
224240
}
225241

226-
positionTooltip();
242+
// Hide the tooltip popup element.
243+
function hide() {
244+
// First things first: we don't show it anymore.
245+
scope.tt_isOpen = false;
246+
247+
//if tooltip is going to be shown after delay, we must cancel this
248+
$timeout.cancel( popupTimeout );
249+
250+
// And now we remove it from the DOM. However, if we have animation, we
251+
// need to wait for it to expire beforehand.
252+
// FIXME: this is a placeholder for a port of the transitions library.
253+
if ( scope.tt_animation ) {
254+
transitionTimeout = $timeout(removeTooltip, 500);
255+
} else {
256+
removeTooltip();
257+
}
258+
}
227259

228-
// And show the tooltip.
229-
scope.tt_isOpen = true;
260+
function createTooltip() {
261+
// There can only be one tooltip element per directive shown at once.
262+
if (tooltip) {
263+
removeTooltip();
264+
}
265+
tooltip = tooltipLinker(scope, function () {});
230266

231-
// Return positioning function as promise callback for correct
232-
// positioning after draw.
233-
return positionTooltip;
234-
}
235-
236-
// Hide the tooltip popup element.
237-
function hide() {
238-
// First things first: we don't show it anymore.
239-
scope.tt_isOpen = false;
267+
// Get contents rendered into the tooltip
268+
scope.$digest();
269+
}
240270

241-
//if tooltip is going to be shown after delay, we must cancel this
242-
$timeout.cancel( popupTimeout );
243-
244-
// And now we remove it from the DOM. However, if we have animation, we
245-
// need to wait for it to expire beforehand.
246-
// FIXME: this is a placeholder for a port of the transitions library.
247-
if ( scope.tt_animation ) {
248-
transitionTimeout = $timeout(function () {
271+
function removeTooltip() {
272+
if (tooltip) {
249273
tooltip.remove();
250-
}, 500);
251-
} else {
252-
tooltip.remove();
274+
tooltip = null;
275+
}
253276
}
254-
}
255277

256-
/**
257-
* Observe the relevant attributes.
258-
*/
259-
attrs.$observe( type, function ( val ) {
260-
scope.tt_content = val;
278+
/**
279+
* Observe the relevant attributes.
280+
*/
281+
attrs.$observe( type, function ( val ) {
282+
scope.tt_content = val;
261283

262-
if (!val && scope.tt_isOpen ) {
263-
hide();
264-
}
265-
});
284+
if (!val && scope.tt_isOpen ) {
285+
hide();
286+
}
287+
});
266288

267-
attrs.$observe( prefix+'Title', function ( val ) {
268-
scope.tt_title = val;
269-
});
289+
attrs.$observe( prefix+'Title', function ( val ) {
290+
scope.tt_title = val;
291+
});
270292

271-
attrs.$observe( prefix+'Placement', function ( val ) {
272-
scope.tt_placement = angular.isDefined( val ) ? val : options.placement;
273-
});
293+
attrs.$observe( prefix+'Placement', function ( val ) {
294+
scope.tt_placement = angular.isDefined( val ) ? val : options.placement;
295+
});
274296

275-
attrs.$observe( prefix+'PopupDelay', function ( val ) {
276-
var delay = parseInt( val, 10 );
277-
scope.tt_popupDelay = ! isNaN(delay) ? delay : options.popupDelay;
278-
});
297+
attrs.$observe( prefix+'PopupDelay', function ( val ) {
298+
var delay = parseInt( val, 10 );
299+
scope.tt_popupDelay = ! isNaN(delay) ? delay : options.popupDelay;
300+
});
279301

280-
var unregisterTriggers = function() {
281-
if (hasRegisteredTriggers) {
282-
element.unbind( triggers.show, showTooltipBind );
283-
element.unbind( triggers.hide, hideTooltipBind );
284-
}
285-
};
302+
var unregisterTriggers = function() {
303+
if (hasRegisteredTriggers) {
304+
element.unbind( triggers.show, showTooltipBind );
305+
element.unbind( triggers.hide, hideTooltipBind );
306+
}
307+
};
286308

287-
attrs.$observe( prefix+'Trigger', function ( val ) {
288-
unregisterTriggers();
309+
attrs.$observe( prefix+'Trigger', function ( val ) {
310+
unregisterTriggers();
289311

290-
triggers = getTriggers( val );
312+
triggers = getTriggers( val );
291313

292-
if ( triggers.show === triggers.hide ) {
293-
element.bind( triggers.show, toggleTooltipBind );
294-
} else {
295-
element.bind( triggers.show, showTooltipBind );
296-
element.bind( triggers.hide, hideTooltipBind );
297-
}
314+
if ( triggers.show === triggers.hide ) {
315+
element.bind( triggers.show, toggleTooltipBind );
316+
} else {
317+
element.bind( triggers.show, showTooltipBind );
318+
element.bind( triggers.hide, hideTooltipBind );
319+
}
298320

299-
hasRegisteredTriggers = true;
300-
});
321+
hasRegisteredTriggers = true;
322+
});
301323

302-
var animation = scope.$eval(attrs[prefix + 'Animation']);
303-
scope.tt_animation = angular.isDefined(animation) ? !!animation : options.animation;
324+
var animation = scope.$eval(attrs[prefix + 'Animation']);
325+
scope.tt_animation = angular.isDefined(animation) ? !!animation : options.animation;
304326

305-
attrs.$observe( prefix+'AppendToBody', function ( val ) {
306-
appendToBody = angular.isDefined( val ) ? $parse( val )( scope ) : appendToBody;
307-
});
327+
attrs.$observe( prefix+'AppendToBody', function ( val ) {
328+
appendToBody = angular.isDefined( val ) ? $parse( val )( scope ) : appendToBody;
329+
});
308330

309-
// if a tooltip is attached to <body> we need to remove it on
310-
// location change as its parent scope will probably not be destroyed
311-
// by the change.
312-
if ( appendToBody ) {
313-
scope.$on('$locationChangeSuccess', function closeTooltipOnLocationChangeSuccess () {
314-
if ( scope.tt_isOpen ) {
315-
hide();
331+
// if a tooltip is attached to <body> we need to remove it on
332+
// location change as its parent scope will probably not be destroyed
333+
// by the change.
334+
if ( appendToBody ) {
335+
scope.$on('$locationChangeSuccess', function closeTooltipOnLocationChangeSuccess () {
336+
if ( scope.tt_isOpen ) {
337+
hide();
338+
}
339+
});
316340
}
317-
});
318-
}
319-
320-
// Make sure tooltip is destroyed and removed.
321-
scope.$on('$destroy', function onDestroyTooltip() {
322-
$timeout.cancel( transitionTimeout );
323-
$timeout.cancel( popupTimeout );
324-
unregisterTriggers();
325-
tooltip.remove();
326-
tooltip.unbind();
327-
tooltip = null;
328-
});
341+
342+
// Make sure tooltip is destroyed and removed.
343+
scope.$on('$destroy', function onDestroyTooltip() {
344+
$timeout.cancel( transitionTimeout );
345+
$timeout.cancel( popupTimeout );
346+
unregisterTriggers();
347+
removeTooltip();
348+
});
349+
};
329350
}
330351
};
331352
};

0 commit comments

Comments
 (0)
This repository has been archived.