From 0eb196a65bda0866fed8857594e2d951418eb3f1 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Mon, 23 Dec 2013 16:10:43 -0800 Subject: [PATCH 1/3] test(tooltip): check scope after hiding and showing 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. Reproduces #1191 --- src/tooltip/test/tooltip.spec.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/tooltip/test/tooltip.spec.js b/src/tooltip/test/tooltip.spec.js index 01f6423fe7..b2aeba9a92 100644 --- a/src/tooltip/test/tooltip.spec.js +++ b/src/tooltip/test/tooltip.spec.js @@ -98,7 +98,7 @@ describe('tooltip', function() { scope.alt = "Alt Message"; elmBody = $compile( angular.element( - '
Selector Text
' + '
Selector Text
' ) )( scope ); $compile( elmBody )( scope ); @@ -114,6 +114,13 @@ describe('tooltip', function() { expect( ttScope.content ).toBe( scope.tooltipMsg ); elm.trigger( 'mouseleave' ); + + //Isolate scope contents should be the same after hiding and showing again (issue 1191) + elm.trigger( 'mouseenter' ); + + ttScope = angular.element( elmBody.children()[1] ).scope(); + expect( ttScope.placement ).toBe( 'top' ); + expect( ttScope.content ).toBe( scope.tooltipMsg ); })); it('should not show tooltips if there is nothing to show - issue #129', inject(function ($compile) { @@ -136,6 +143,24 @@ describe('tooltip', function() { expect( elmBody.children().length ).toBe( 0 ); })); + it('issue 1191 - isolate scope on the popup should always be child of correct element scope', inject( function ( $compile ) { + var ttScope; + elm.trigger( 'mouseenter' ); + + ttScope = angular.element( elmBody.children()[1] ).scope(); + expect( ttScope.$parent ).toBe( elmScope ); + + elm.trigger( 'mouseleave' ); + + // After leaving and coming back, the scope's parent should be the same + elm.trigger( 'mouseenter' ); + + ttScope = angular.element( elmBody.children()[1] ).scope(); + expect( ttScope.$parent ).toBe( elmScope ); + + elm.trigger( 'mouseleave' ); + })); + describe('with specified enable expression', function() { beforeEach(inject(function ($compile) { From d7896f64991fd851eb868fd00e289a851c1f3ab3 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Mon, 23 Dec 2013 14:49:26 -0800 Subject: [PATCH 2/3] 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. Fixes #1450 and #1191 --- src/tooltip/test/tooltip.spec.js | 8 +------ src/tooltip/tooltip.js | 41 ++++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/tooltip/test/tooltip.spec.js b/src/tooltip/test/tooltip.spec.js index b2aeba9a92..90be4f6907 100644 --- a/src/tooltip/test/tooltip.spec.js +++ b/src/tooltip/test/tooltip.spec.js @@ -348,18 +348,12 @@ describe('tooltip', function() { elm = elmBody.find('input'); elmScope = elm.scope(); + elm.trigger('fooTrigger'); tooltipScope = elmScope.$$childTail; })); - it( 'should not contain a cached reference', function() { - expect( inCache() ).toBeTruthy(); - elmScope.$destroy(); - expect( inCache() ).toBeFalsy(); - }); - it( 'should not contain a cached reference when visible', inject( function( $timeout ) { expect( inCache() ).toBeTruthy(); - elm.trigger('fooTrigger'); elmScope.$destroy(); expect( inCache() ).toBeFalsy(); })); diff --git a/src/tooltip/tooltip.js b/src/tooltip/tooltip.js index f6d116647f..b81b4fd51e 100644 --- a/src/tooltip/tooltip.js +++ b/src/tooltip/tooltip.js @@ -108,8 +108,11 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return { restrict: 'EA', scope: true, - link: function link ( scope, element, attrs ) { - var tooltip = $compile( template )( scope ); + compile: function (tElem, tAttrs) { + var tooltipLinker = $compile( template ); + + return function link ( scope, element, attrs ) { + var tooltip; var transitionTimeout; var popupTimeout; var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false; @@ -184,10 +187,10 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return; } if ( scope.tt_popupDelay ) { - popupTimeout = $timeout( show, scope.tt_popupDelay ); + popupTimeout = $timeout( show, scope.tt_popupDelay, false ); popupTimeout.then(function(reposition){reposition();}); } else { - scope.$apply( show )(); + show()(); } } @@ -206,6 +209,8 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap return angular.noop; } + createTooltip(); + // If there is a pending remove transition, we must cancel it, lest the // tooltip be mysteriously removed. if ( transitionTimeout ) { @@ -227,6 +232,7 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap // And show the tooltip. scope.tt_isOpen = true; + scope.$digest(); // digest required as $apply is not called // Return positioning function as promise callback for correct // positioning after draw. @@ -245,11 +251,27 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap // need to wait for it to expire beforehand. // FIXME: this is a placeholder for a port of the transitions library. if ( scope.tt_animation ) { - transitionTimeout = $timeout(function () { - tooltip.remove(); - }, 500); + transitionTimeout = $timeout(removeTooltip, 500); } else { + removeTooltip(); + } + } + + function createTooltip() { + // There can only be one tooltip element per directive shown at once. + if (tooltip) { + removeTooltip(); + } + tooltip = tooltipLinker(scope, function () {}); + + // Get contents rendered into the tooltip + scope.$digest(); + } + + function removeTooltip() { + if (tooltip) { tooltip.remove(); + tooltip = null; } } @@ -322,10 +344,9 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap $timeout.cancel( transitionTimeout ); $timeout.cancel( popupTimeout ); unregisterTriggers(); - tooltip.remove(); - tooltip.unbind(); - tooltip = null; + removeTooltip(); }); + }; } }; }; From 3c37e07d0206874f8da6d2ba3217fe0ca832526b Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Sat, 28 Dec 2013 14:02:03 -0800 Subject: [PATCH 3/3] style(tooltip): fix indentation --- src/tooltip/tooltip.js | 404 ++++++++++++++++++++--------------------- 1 file changed, 202 insertions(+), 202 deletions(-) diff --git a/src/tooltip/tooltip.js b/src/tooltip/tooltip.js index b81b4fd51e..c6a610f8f3 100644 --- a/src/tooltip/tooltip.js +++ b/src/tooltip/tooltip.js @@ -112,241 +112,241 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap var tooltipLinker = $compile( template ); return function link ( scope, element, attrs ) { - var tooltip; - var transitionTimeout; - var popupTimeout; - var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false; - var triggers = getTriggers( undefined ); - var hasRegisteredTriggers = false; - var hasEnableExp = angular.isDefined(attrs[prefix+'Enable']); - - var positionTooltip = function (){ - var position, - ttWidth, - ttHeight, - ttPosition; - // Get the position of the directive element. - position = appendToBody ? $position.offset( element ) : $position.position( element ); - - // Get the height and width of the tooltip so we can center it. - ttWidth = tooltip.prop( 'offsetWidth' ); - ttHeight = tooltip.prop( 'offsetHeight' ); - - // Calculate the tooltip's top and left coordinates to center it with - // this directive. - switch ( scope.tt_placement ) { - case 'right': - ttPosition = { - top: position.top + position.height / 2 - ttHeight / 2, - left: position.left + position.width - }; - break; - case 'bottom': - ttPosition = { - top: position.top + position.height, - left: position.left + position.width / 2 - ttWidth / 2 - }; - break; - case 'left': - ttPosition = { - top: position.top + position.height / 2 - ttHeight / 2, - left: position.left - ttWidth - }; - break; - default: - ttPosition = { - top: position.top - ttHeight, - left: position.left + position.width / 2 - ttWidth / 2 - }; - break; - } - - ttPosition.top += 'px'; - ttPosition.left += 'px'; - - // Now set the calculated positioning. - tooltip.css( ttPosition ); - - }; - - // By default, the tooltip is not open. - // TODO add ability to start tooltip opened - scope.tt_isOpen = false; + var tooltip; + var transitionTimeout; + var popupTimeout; + var appendToBody = angular.isDefined( options.appendToBody ) ? options.appendToBody : false; + var triggers = getTriggers( undefined ); + var hasRegisteredTriggers = false; + var hasEnableExp = angular.isDefined(attrs[prefix+'Enable']); + + var positionTooltip = function (){ + var position, + ttWidth, + ttHeight, + ttPosition; + // Get the position of the directive element. + position = appendToBody ? $position.offset( element ) : $position.position( element ); + + // Get the height and width of the tooltip so we can center it. + ttWidth = tooltip.prop( 'offsetWidth' ); + ttHeight = tooltip.prop( 'offsetHeight' ); + + // Calculate the tooltip's top and left coordinates to center it with + // this directive. + switch ( scope.tt_placement ) { + case 'right': + ttPosition = { + top: position.top + position.height / 2 - ttHeight / 2, + left: position.left + position.width + }; + break; + case 'bottom': + ttPosition = { + top: position.top + position.height, + left: position.left + position.width / 2 - ttWidth / 2 + }; + break; + case 'left': + ttPosition = { + top: position.top + position.height / 2 - ttHeight / 2, + left: position.left - ttWidth + }; + break; + default: + ttPosition = { + top: position.top - ttHeight, + left: position.left + position.width / 2 - ttWidth / 2 + }; + break; + } + + ttPosition.top += 'px'; + ttPosition.left += 'px'; + + // Now set the calculated positioning. + tooltip.css( ttPosition ); + + }; + + // By default, the tooltip is not open. + // TODO add ability to start tooltip opened + scope.tt_isOpen = false; - function toggleTooltipBind () { - if ( ! scope.tt_isOpen ) { - showTooltipBind(); - } else { - hideTooltipBind(); + function toggleTooltipBind () { + if ( ! scope.tt_isOpen ) { + showTooltipBind(); + } else { + hideTooltipBind(); + } } - } - - // Show the tooltip with delay if specified, otherwise show it immediately - function showTooltipBind() { - if(hasEnableExp && !scope.$eval(attrs[prefix+'Enable'])) { - return; + + // Show the tooltip with delay if specified, otherwise show it immediately + function showTooltipBind() { + if(hasEnableExp && !scope.$eval(attrs[prefix+'Enable'])) { + return; + } + if ( scope.tt_popupDelay ) { + popupTimeout = $timeout( show, scope.tt_popupDelay, false ); + popupTimeout.then(function(reposition){reposition();}); + } else { + show()(); + } } - if ( scope.tt_popupDelay ) { - popupTimeout = $timeout( show, scope.tt_popupDelay, false ); - popupTimeout.then(function(reposition){reposition();}); - } else { - show()(); + + function hideTooltipBind () { + scope.$apply(function () { + hide(); + }); } - } - function hideTooltipBind () { - scope.$apply(function () { - hide(); - }); - } - - // Show the tooltip popup element. - function show() { + // Show the tooltip popup element. + function show() { - // Don't show empty tooltips. - if ( ! scope.tt_content ) { - return angular.noop; - } + // Don't show empty tooltips. + if ( ! scope.tt_content ) { + return angular.noop; + } - createTooltip(); + createTooltip(); - // If there is a pending remove transition, we must cancel it, lest the - // tooltip be mysteriously removed. - if ( transitionTimeout ) { - $timeout.cancel( transitionTimeout ); - } - - // Set the initial positioning. - tooltip.css({ top: 0, left: 0, display: 'block' }); - - // Now we add it to the DOM because need some info about it. But it's not - // visible yet anyway. - if ( appendToBody ) { - $document.find( 'body' ).append( tooltip ); - } else { - element.after( tooltip ); - } + // If there is a pending remove transition, we must cancel it, lest the + // tooltip be mysteriously removed. + if ( transitionTimeout ) { + $timeout.cancel( transitionTimeout ); + } - positionTooltip(); + // Set the initial positioning. + tooltip.css({ top: 0, left: 0, display: 'block' }); - // And show the tooltip. - scope.tt_isOpen = true; - scope.$digest(); // digest required as $apply is not called + // Now we add it to the DOM because need some info about it. But it's not + // visible yet anyway. + if ( appendToBody ) { + $document.find( 'body' ).append( tooltip ); + } else { + element.after( tooltip ); + } - // Return positioning function as promise callback for correct - // positioning after draw. - return positionTooltip; - } - - // Hide the tooltip popup element. - function hide() { - // First things first: we don't show it anymore. - scope.tt_isOpen = false; + positionTooltip(); - //if tooltip is going to be shown after delay, we must cancel this - $timeout.cancel( popupTimeout ); - - // And now we remove it from the DOM. However, if we have animation, we - // need to wait for it to expire beforehand. - // FIXME: this is a placeholder for a port of the transitions library. - if ( scope.tt_animation ) { - transitionTimeout = $timeout(removeTooltip, 500); - } else { - removeTooltip(); + // And show the tooltip. + scope.tt_isOpen = true; + scope.$digest(); // digest required as $apply is not called + + // Return positioning function as promise callback for correct + // positioning after draw. + return positionTooltip; } - } - function createTooltip() { - // There can only be one tooltip element per directive shown at once. - if (tooltip) { - removeTooltip(); + // Hide the tooltip popup element. + function hide() { + // First things first: we don't show it anymore. + scope.tt_isOpen = false; + + //if tooltip is going to be shown after delay, we must cancel this + $timeout.cancel( popupTimeout ); + + // And now we remove it from the DOM. However, if we have animation, we + // need to wait for it to expire beforehand. + // FIXME: this is a placeholder for a port of the transitions library. + if ( scope.tt_animation ) { + transitionTimeout = $timeout(removeTooltip, 500); + } else { + removeTooltip(); + } } - tooltip = tooltipLinker(scope, function () {}); - // Get contents rendered into the tooltip - scope.$digest(); - } + function createTooltip() { + // There can only be one tooltip element per directive shown at once. + if (tooltip) { + removeTooltip(); + } + tooltip = tooltipLinker(scope, function () {}); - function removeTooltip() { - if (tooltip) { - tooltip.remove(); - tooltip = null; + // Get contents rendered into the tooltip + scope.$digest(); } - } - - /** - * Observe the relevant attributes. - */ - attrs.$observe( type, function ( val ) { - scope.tt_content = val; - if (!val && scope.tt_isOpen ) { - hide(); + function removeTooltip() { + if (tooltip) { + tooltip.remove(); + tooltip = null; + } } - }); - attrs.$observe( prefix+'Title', function ( val ) { - scope.tt_title = val; - }); + /** + * Observe the relevant attributes. + */ + attrs.$observe( type, function ( val ) { + scope.tt_content = val; - attrs.$observe( prefix+'Placement', function ( val ) { - scope.tt_placement = angular.isDefined( val ) ? val : options.placement; - }); + if (!val && scope.tt_isOpen ) { + hide(); + } + }); - attrs.$observe( prefix+'PopupDelay', function ( val ) { - var delay = parseInt( val, 10 ); - scope.tt_popupDelay = ! isNaN(delay) ? delay : options.popupDelay; - }); + attrs.$observe( prefix+'Title', function ( val ) { + scope.tt_title = val; + }); - var unregisterTriggers = function() { - if (hasRegisteredTriggers) { - element.unbind( triggers.show, showTooltipBind ); - element.unbind( triggers.hide, hideTooltipBind ); - } - }; + attrs.$observe( prefix+'Placement', function ( val ) { + scope.tt_placement = angular.isDefined( val ) ? val : options.placement; + }); - attrs.$observe( prefix+'Trigger', function ( val ) { - unregisterTriggers(); + attrs.$observe( prefix+'PopupDelay', function ( val ) { + var delay = parseInt( val, 10 ); + scope.tt_popupDelay = ! isNaN(delay) ? delay : options.popupDelay; + }); - triggers = getTriggers( val ); + var unregisterTriggers = function() { + if (hasRegisteredTriggers) { + element.unbind( triggers.show, showTooltipBind ); + element.unbind( triggers.hide, hideTooltipBind ); + } + }; - if ( triggers.show === triggers.hide ) { - element.bind( triggers.show, toggleTooltipBind ); - } else { - element.bind( triggers.show, showTooltipBind ); - element.bind( triggers.hide, hideTooltipBind ); - } + attrs.$observe( prefix+'Trigger', function ( val ) { + unregisterTriggers(); - hasRegisteredTriggers = true; - }); + triggers = getTriggers( val ); - var animation = scope.$eval(attrs[prefix + 'Animation']); - scope.tt_animation = angular.isDefined(animation) ? !!animation : options.animation; + if ( triggers.show === triggers.hide ) { + element.bind( triggers.show, toggleTooltipBind ); + } else { + element.bind( triggers.show, showTooltipBind ); + element.bind( triggers.hide, hideTooltipBind ); + } - attrs.$observe( prefix+'AppendToBody', function ( val ) { - appendToBody = angular.isDefined( val ) ? $parse( val )( scope ) : appendToBody; - }); + hasRegisteredTriggers = true; + }); - // if a tooltip is attached to we need to remove it on - // location change as its parent scope will probably not be destroyed - // by the change. - if ( appendToBody ) { - scope.$on('$locationChangeSuccess', function closeTooltipOnLocationChangeSuccess () { - if ( scope.tt_isOpen ) { - hide(); + var animation = scope.$eval(attrs[prefix + 'Animation']); + scope.tt_animation = angular.isDefined(animation) ? !!animation : options.animation; + + attrs.$observe( prefix+'AppendToBody', function ( val ) { + appendToBody = angular.isDefined( val ) ? $parse( val )( scope ) : appendToBody; + }); + + // if a tooltip is attached to we need to remove it on + // location change as its parent scope will probably not be destroyed + // by the change. + if ( appendToBody ) { + scope.$on('$locationChangeSuccess', function closeTooltipOnLocationChangeSuccess () { + if ( scope.tt_isOpen ) { + hide(); + } + }); } - }); - } - - // Make sure tooltip is destroyed and removed. - scope.$on('$destroy', function onDestroyTooltip() { - $timeout.cancel( transitionTimeout ); - $timeout.cancel( popupTimeout ); - unregisterTriggers(); - removeTooltip(); - }); - }; + + // Make sure tooltip is destroyed and removed. + scope.$on('$destroy', function onDestroyTooltip() { + $timeout.cancel( transitionTimeout ); + $timeout.cancel( popupTimeout ); + unregisterTriggers(); + removeTooltip(); + }); + }; } }; };