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

Tooltip positioning & placement improvements #3980

Closed
wants to merge 2 commits into from
Closed

Tooltip positioning & placement improvements #3980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb15401
fix(tooltip): prevent 1px shift in Webkit/Blink
cvn Jul 20, 2015
c0b4e56
fix(tooltip): update tooltip placement dynamically
cvn Jul 20, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions src/tooltip/test/tooltip.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ describe('tooltip', function() {
expect( tooltipScope.placement ).toBe( 'bottom' );
}));

it('should update placement dynamically', inject( function( $compile, $timeout ) {
scope.place = 'bottom';
elm = $compile( angular.element(
'<span tooltip="tooltip text" tooltip-placement="{{place}}">Selector Text</span>'
) )( scope );
scope.$apply();
elmScope = elm.scope();
tooltipScope = elmScope.$$childTail;

elm.trigger( 'mouseenter' );
expect( tooltipScope.placement ).toBe( 'bottom' );

scope.place = 'right';
scope.$digest();
$timeout.flush();
expect(tooltipScope.placement).toBe( 'right' );
}));

it('should work inside an ngRepeat', inject( function( $compile ) {

elm = $compile( angular.element(
Expand Down Expand Up @@ -474,24 +492,26 @@ describe( 'tooltip positioning', function() {
tooltipScope = elmScope.$$childTail;
}));

it( 'should re-position on every digest', inject( function ($timeout) {
it( 'should re-position when value changes', inject( function ($timeout) {
elm.trigger( 'mouseenter' );

scope.$digest();
$timeout.flush();
var startingPositionCalls = $position.positionElements.calls.count();

scope.text = 'New Text';
scope.$digest();
$timeout.flush();
expect(elm.attr('tooltip')).toBe( 'New Text' );
expect($position.positionElements.calls.count()).toEqual(startingPositionCalls + 1);
// Check that positionElements was called with elm
expect($position.positionElements.calls.argsFor(startingPositionCalls)[0][0])
.toBe(elm[0]);

scope.$digest();
$timeout.flush();
expect($position.positionElements.calls.count()).toEqual(startingPositionCalls + 2);
expect($position.positionElements.calls.argsFor(startingPositionCalls + 1)[0][0])
$timeout.verifyNoPendingTasks();
expect($position.positionElements.calls.count()).toEqual(startingPositionCalls + 1);
expect($position.positionElements.calls.argsFor(startingPositionCalls)[0][0])
.toBe(elm[0]);
scope.$digest();
}));
Expand Down
20 changes: 16 additions & 4 deletions src/tooltip/tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
tooltip.css( ttPosition );
};

var positionTooltipAsync = function () {
$timeout(positionTooltip, 0, false);
};

// Set up the correct scope to allow transclusion later
ttScope.origScope = scope;

Expand Down Expand Up @@ -246,12 +250,9 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
}
});

tooltipLinkedScope.$watch(function () {
$timeout(positionTooltip, 0, false);
});

if (options.useContentExp) {
tooltipLinkedScope.$watch('contentExp()', function (val) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also need to schedule a reposition over here since if contentExp() changes, the content changes as well.

positionTooltipAsync();
if (!val && ttScope.isOpen ) {
hide();
}
Expand Down Expand Up @@ -287,6 +288,7 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
if (!options.useContentExp) {
attrs.$observe( type, function ( val ) {
ttScope.content = val;
positionTooltipAsync();

if (!val && ttScope.isOpen ) {
hide();
Expand All @@ -302,6 +304,16 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap

attrs.$observe( prefix+'Title', function ( val ) {
ttScope.title = val;
positionTooltipAsync();
});

attrs.$observe( prefix + 'Placement', function () {
if (ttScope.isOpen) {
$timeout(function () {
prepPlacement();
show()();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean that if the placement is changed while the tooltip is hidden, it'll show?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it only runs if ttScope.isOpen is true, so only when it's already open.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I missed that check, but there's also when the value is a blank string, see above at line #288: if (!val && ttScope.isOpen ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a slightly different case that warrants a different conditional. On 288, there's new content to deal with and if the content is blank, it should hide the tooltip. Here there's no new content, just a new placement.

}, 0, false);
}
});

function prepPopupClass() {
Expand Down