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

Commit 29aed22

Browse files
Josh David Millerpkozlowski-opensource
Josh David Miller
authored andcommitted
fix(tooltip): Attributes no longer evalued in isolate scope
The tooltip directive has been re-architected to use a new scope rather than an isolate scope, but the templated tooltipPopup directive now has an isolate scope to introduce a similar level of scope security. As the isolate scope is gone, the transclusion logic has been removed. In addition, tooltip attributes are $observed manually to assign the necessary variables on the child scope needed by the tooltipPopup directive. Closes #78.
1 parent b19b9c5 commit 29aed22

File tree

3 files changed

+63
-30
lines changed

3 files changed

+63
-30
lines changed

src/tooltip/test/tooltipSpec.js

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('tooltip', function() {
2323
}));
2424

2525
it('should not be open initially', inject(function() {
26-
expect( elmScope.isOpen ).toBe( false );
26+
expect( elmScope.tt_isOpen ).toBe( false );
2727

2828
// We can only test *that* the tooltip-popup element wasn't created as the
2929
// implementation is templated and replaced.
@@ -32,7 +32,7 @@ describe('tooltip', function() {
3232

3333
it('should open on mouseenter', inject(function() {
3434
elm.trigger( 'mouseenter' );
35-
expect( elmScope.isOpen ).toBe( true );
35+
expect( elmScope.tt_isOpen ).toBe( true );
3636

3737
// We can only test *that* the tooltip-popup element was created as the
3838
// implementation is templated and replaced.
@@ -42,12 +42,12 @@ describe('tooltip', function() {
4242
it('should close on mouseleave', inject(function() {
4343
elm.trigger( 'mouseenter' );
4444
elm.trigger( 'mouseleave' );
45-
expect( elmScope.isOpen ).toBe( false );
45+
expect( elmScope.tt_isOpen ).toBe( false );
4646
}));
4747

4848
it('should have default placement of "top"', inject(function() {
4949
elm.trigger( 'mouseenter' );
50-
expect( elmScope.placement ).toBe( "top" );
50+
expect( elmScope.tt_placement ).toBe( "top" );
5151
}));
5252

5353
it('should allow specification of placement', inject( function( $compile ) {
@@ -57,15 +57,15 @@ describe('tooltip', function() {
5757
elmScope = elm.scope();
5858

5959
elm.trigger( 'mouseenter' );
60-
expect( elmScope.placement ).toBe( "bottom" );
60+
expect( elmScope.tt_placement ).toBe( "bottom" );
6161
}));
6262

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

6565
elm = $compile( angular.element(
6666
'<ul>'+
6767
'<li ng-repeat="item in items">'+
68-
'<span id="selector" tooltip="{{item.tooltip}}">{{item.name}}</span>'+
68+
'<span tooltip="{{item.tooltip}}">{{item.name}}</span>'+
6969
'</li>'+
7070
'</ul>'
7171
) )( scope );
@@ -80,15 +80,37 @@ describe('tooltip', function() {
8080

8181
tt.trigger( 'mouseenter' );
8282

83-
// Due to the transclusion, the contents of the element are in a span, so
84-
// we select the tooltip's child and ensure its content matches.
85-
expect( tt.children().text() ).toBe( scope.items[0].name );
86-
87-
// And the tooltip text should still match.
88-
expect( tt.scope().tooltipTitle ).toBe( scope.items[0].tooltip );
83+
expect( tt.text() ).toBe( scope.items[0].name );
84+
expect( tt.scope().tt_tooltip ).toBe( scope.items[0].tooltip );
8985

9086
tt.trigger( 'mouseleave' );
9187
}));
88+
89+
it('should only have an isolate scope on the popup', inject( function ( $compile ) {
90+
var ttScope;
91+
92+
scope.tooltipMsg = "Tooltip Text";
93+
scope.alt = "Alt Message";
94+
95+
elmBody = $compile( angular.element(
96+
'<div><span alt={{alt}} tooltip="{{tooltipMsg}}">Selector Text</span></div>'
97+
) )( scope );
98+
99+
$compile( elmBody )( scope );
100+
scope.$digest();
101+
elm = elmBody.find( 'span' );
102+
elmScope = elm.scope();
103+
104+
elm.trigger( 'mouseenter' );
105+
expect( elm.attr( 'alt' ) ).toBe( scope.alt );
106+
107+
ttScope = angular.element( elmBody.children()[1] ).scope();
108+
expect( ttScope.placement ).toBe( 'top' );
109+
expect( ttScope.tooltipTitle ).toBe( scope.tooltipMsg );
110+
111+
elm.trigger( 'mouseleave' );
112+
}));
113+
92114
});
93115

94116

src/tooltip/tooltip.js

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,42 @@ angular.module( 'ui.bootstrap.tooltip', [] )
88
return {
99
restrict: 'E',
1010
replace: true,
11+
scope: { tooltipTitle: '@', placement: '@', animation: '&', isOpen: '&' },
1112
templateUrl: 'template/tooltip/tooltip-popup.html'
1213
};
1314
})
14-
.directive( 'tooltip', [ '$compile', '$timeout', function ( $compile, $timeout ) {
15+
.directive( 'tooltip', [ '$compile', '$timeout', '$parse', function ( $compile, $timeout, $parse ) {
1516

1617
var template =
17-
'<tooltip-popup></tooltip-popup>';
18+
'<tooltip-popup '+
19+
'tooltip-title="{{tt_tooltip}}" '+
20+
'placement="{{tt_placement}}" '+
21+
'animation="tt_animation()" '+
22+
'is-open="tt_isOpen"'+
23+
'>'+
24+
'</tooltip-popup>';
1825

1926
return {
20-
transclude: true,
21-
scope: { tooltipTitle: '@tooltip', placement: '@tooltipPlacement', animation: '&tooltipAnimation' },
22-
controller: ['$transclude', '$element', function($transclude, $element) {
23-
$transclude(function(clone) {
24-
$element.append(clone);
25-
});
26-
}],
27+
scope: true,
2728
link: function ( scope, element, attr ) {
2829
var tooltip = $compile( template )( scope ),
2930
transitionTimeout;
3031

32+
attr.$observe( 'tooltip', function ( val ) {
33+
scope.tt_tooltip = val;
34+
});
35+
36+
attr.$observe( 'tooltipPlacement', function ( val ) {
37+
// If no placement was provided, default to 'top'.
38+
scope.tt_placement = val || 'top';
39+
});
40+
41+
attr.$observe( 'tooltipAnimation', function ( val ) {
42+
scope.tt_animation = $parse( val );
43+
});
44+
3145
// By default, the tooltip is not open.
32-
scope.isOpen = false;
46+
scope.tt_isOpen = false;
3347

3448
// Calculate the current position and size of the directive element.
3549
function getPosition() {
@@ -48,9 +62,6 @@ angular.module( 'ui.bootstrap.tooltip', [] )
4862
ttHeight,
4963
ttPosition;
5064

51-
// If no placement was provided, default to 'top'.
52-
scope.placement = scope.placement || 'top';
53-
5465
// If there is a pending remove transition, we must cancel it, lest the
5566
// toolip be mysteriously removed.
5667
if ( transitionTimeout ) {
@@ -73,7 +84,7 @@ angular.module( 'ui.bootstrap.tooltip', [] )
7384

7485
// Calculate the tooltip's top and left coordinates to center it with
7586
// this directive.
76-
switch ( scope.placement ) {
87+
switch ( scope.tt_placement ) {
7788
case 'right':
7889
ttPosition = {
7990
top: (position.top + position.height / 2 - ttHeight / 2) + 'px',
@@ -104,19 +115,19 @@ angular.module( 'ui.bootstrap.tooltip', [] )
104115
tooltip.css( ttPosition );
105116

106117
// And show the tooltip.
107-
scope.isOpen = true;
118+
scope.tt_isOpen = true;
108119
}
109120

110121
// Hide the tooltip popup element.
111122
function hide() {
112123
// First things first: we don't show it anymore.
113124
//tooltip.removeClass( 'in' );
114-
scope.isOpen = false;
125+
scope.tt_isOpen = false;
115126

116127
// And now we remove it from the DOM. However, if we have animation, we
117128
// need to wait for it to expire beforehand.
118129
// FIXME: this is a placeholder for a port of the transitions library.
119-
if ( angular.isDefined( scope.animation ) && scope.animation() ) {
130+
if ( angular.isDefined( scope.tt_animation ) && scope.tt_animation() ) {
120131
transitionTimeout = $timeout( function () { tooltip.remove(); }, 500 );
121132
} else {
122133
tooltip.remove();

template/tooltip/tooltip-popup.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<div class="tooltip {{placement}}" ng-class="{ in: isOpen, fade: animation() }">
1+
<div class="tooltip {{placement}}" ng-class="{ in: isOpen(), fade: animation() }">
22
<div class="tooltip-arrow"></div>
33
<div class="tooltip-inner" ng-bind="tooltipTitle"></div>
44
</div>

0 commit comments

Comments
 (0)