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

Commit 81a38d7

Browse files
RobJacobsRobJacobs
committed
refactor(tooltip): rollup commit for several issues
This is a rollup commit intended to address several issues around the positioning and parsing of attributes. - Fixes issue introduced under PR #4311 where setting height and width in tooltip position function messed up arrow placement. - Fixes issue introduced under PR #4363 where setting visibility to hidden in tooltip position function caused elements in popover to lose focus. - Fixes issue #1780 where tooltip would render if content was just whitespace. - Fixes issue #3347 where tooltip isolate scope was being accessed after it was set to null. Observers will now be created/destroyed as tooltip opens/closes which will also offer a performance improvement. - Fixes issue #3557 by implementing evalAsync to set tooltip scope isOpen property. - Fixes issue #4335 where if model isOpen property is undefined, tooltip would call show/hide toggle function. - Closes PR #4429 where how the templated content was being evaluated could cause an infinite digest loop. Closes #4400 Closes #4418 Closes #4429 Closes #4431 Closes #4455 Fixes #1780 Fixes #3347 Fixes #3557 Fixes #4321 Fixes #4335
1 parent a59caf1 commit 81a38d7

File tree

7 files changed

+220
-121
lines changed

7 files changed

+220
-121
lines changed

‎src/popover/test/popover-html.spec.js

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,20 @@ describe('popover', function() {
3535

3636
it('should open on click', inject(function() {
3737
elm.trigger('click');
38-
expect( tooltipScope.isOpen ).toBe(true);
38+
tooltipScope.$digest();
39+
expect(tooltipScope.isOpen).toBe(true);
3940

4041
// We can only test *that* the popover-popup element was created as the
4142
// implementation is templated and replaced.
42-
expect( elmBody.children().length ).toBe(2);
43+
expect(elmBody.children().length).toBe(2);
4344
}));
4445

4546
it('should close on second click', inject(function() {
4647
elm.trigger('click');
48+
tooltipScope.$digest();
49+
expect(tooltipScope.isOpen).toBe(true);
4750
elm.trigger('click');
51+
tooltipScope.$digest();
4852
expect(tooltipScope.isOpen).toBe(false);
4953
}));
5054

@@ -53,6 +57,7 @@ describe('popover', function() {
5357
scope.$digest();
5458

5559
elm.trigger('click');
60+
tooltipScope.$digest();
5661
expect(tooltipScope.isOpen).toBe(false);
5762

5863
expect(elmBody.children().length).toBe(1);
@@ -63,6 +68,7 @@ describe('popover', function() {
6368
scope.$digest();
6469

6570
elm.trigger('click');
71+
tooltipScope.$digest();
6672
expect(tooltipScope.isOpen).toBe(true);
6773

6874
expect(elmBody.children().eq(1).text().trim()).toBe('My template');
@@ -75,6 +81,7 @@ describe('popover', function() {
7581

7682
it('should hide popover when template becomes empty', inject(function($timeout) {
7783
elm.trigger('click');
84+
tooltipScope.$digest();
7885
expect(tooltipScope.isOpen).toBe(true);
7986

8087
scope.template = '';
@@ -101,16 +108,19 @@ describe('popover', function() {
101108
elm = elmBody.find('input');
102109

103110
elm.trigger('mouseenter');
111+
tooltipScope.$digest();
104112
elm.trigger('mouseleave');
113+
tooltipScope.$digest();
105114
expect(scope.clicked).toBeFalsy();
106115

107116
elm.click();
108117
expect(scope.clicked).toBeTruthy();
109118
}));
110119

111120
it('should popup with animate class by default', inject(function() {
112-
elm.trigger( 'click' );
113-
expect( tooltipScope.isOpen ).toBe( true );
121+
elm.trigger('click');
122+
tooltipScope.$digest();
123+
expect(tooltipScope.isOpen).toBe(true);
114124

115125
expect(elmBody.children().eq(1)).toHaveClass('fade');
116126
}));
@@ -127,6 +137,7 @@ describe('popover', function() {
127137
tooltipScope = elmScope.$$childTail;
128138

129139
elm.trigger('click');
140+
tooltipScope.$digest();
130141
expect(tooltipScope.isOpen).toBe(true);
131142
expect(elmBody.children().eq(1)).not.toHaveClass('fade');
132143
}));
@@ -144,6 +155,7 @@ describe('popover', function() {
144155
tooltipScope = elmScope.$$childTail;
145156

146157
elm.trigger('click');
158+
tooltipScope.$digest();
147159
expect(tooltipScope.isOpen).toBe(true);
148160

149161
expect(elmBody.children().length).toBe(2);
@@ -165,6 +177,7 @@ describe('popover', function() {
165177
tooltipScope = elmScope.$$childTail;
166178

167179
elm.trigger('click');
180+
tooltipScope.$digest();
168181
expect(tooltipScope.isOpen).toBe(true);
169182

170183
expect(elmBody.children().length).toBe(2);
@@ -174,5 +187,3 @@ describe('popover', function() {
174187
});
175188
});
176189
});
177-
178-

‎src/popover/test/popover-template.spec.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,29 +32,32 @@ describe('popover template', function() {
3232
}));
3333

3434
it('should open on click', inject(function() {
35-
elm.trigger( 'click' );
36-
expect( tooltipScope.isOpen ).toBe( true );
35+
elm.trigger('click');
36+
tooltipScope.$digest();
37+
expect(tooltipScope.isOpen).toBe(true);
3738

38-
expect( elmBody.children().length ).toBe( 2 );
39+
expect(elmBody.children().length ).toBe(2);
3940
}));
4041

4142
it('should not open on click if templateUrl is empty', inject(function() {
4243
scope.templateUrl = null;
4344
scope.$digest();
4445

4546
elm.trigger('click');
47+
tooltipScope.$digest();
4648
expect(tooltipScope.isOpen).toBe(false);
4749

4850
expect(elmBody.children().length).toBe(1);
4951
}));
5052

5153
it('should show updated text', inject(function() {
5254
scope.myTemplateText = 'some text';
53-
scope.$digest();
5455

5556
elm.trigger('click');
57+
tooltipScope.$digest();
5658
expect(tooltipScope.isOpen).toBe(true);
5759

60+
scope.$digest();
5861
expect(elmBody.children().eq(1).text().trim()).toBe('some text');
5962

6063
scope.myTemplateText = 'new text';
@@ -65,6 +68,7 @@ describe('popover template', function() {
6568

6669
it('should hide popover when template becomes empty', inject(function($timeout) {
6770
elm.trigger('click');
71+
tooltipScope.$digest();
6872
expect(tooltipScope.isOpen).toBe(true);
6973

7074
scope.templateUrl = '';
@@ -89,6 +93,7 @@ describe('popover template', function() {
8993
tooltipScope = elmScope.$$childTail;
9094

9195
elm.trigger('click');
96+
tooltipScope.$digest();
9297
expect(tooltipScope.isOpen).toBe(true);
9398

9499
expect(elmBody.children().length).toBe(2);
@@ -110,6 +115,7 @@ describe('popover template', function() {
110115
tooltipScope = elmScope.$$childTail;
111116

112117
elm.trigger('click');
118+
tooltipScope.$digest();
113119
expect(tooltipScope.isOpen).toBe(true);
114120

115121
expect(elmBody.children().length).toBe(2);

‎src/popover/test/popover.spec.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe('popover', function() {
3434

3535
it('should open on click', inject(function() {
3636
elm.trigger('click');
37+
tooltipScope.$digest();
3738
expect(tooltipScope.isOpen).toBe(true);
3839

3940
// We can only test *that* the popover-popup element was created as the
@@ -43,7 +44,10 @@ describe('popover', function() {
4344

4445
it('should close on second click', inject(function() {
4546
elm.trigger('click');
47+
tooltipScope.$digest();
48+
expect(tooltipScope.isOpen).toBe(true);
4649
elm.trigger('click');
50+
tooltipScope.$digest();
4751
expect(tooltipScope.isOpen).toBe(false);
4852
}));
4953

@@ -70,6 +74,7 @@ describe('popover', function() {
7074

7175
it('should popup with animate class by default', inject(function() {
7276
elm.trigger('click');
77+
tooltipScope.$digest();
7378
expect(tooltipScope.isOpen).toBe(true);
7479

7580
expect(elmBody.children().eq(1)).toHaveClass('fade');
@@ -87,6 +92,7 @@ describe('popover', function() {
8792
tooltipScope = elmScope.$$childTail;
8893

8994
elm.trigger('click');
95+
tooltipScope.$digest();
9096
expect(tooltipScope.isOpen).toBe(true);
9197
expect(elmBody.children().eq(1)).not.toHaveClass('fade');
9298
}));
@@ -104,6 +110,7 @@ describe('popover', function() {
104110
tooltipScope = elmScope.$$childTail;
105111

106112
elm.trigger('click');
113+
tooltipScope.$digest();
107114
expect(tooltipScope.isOpen).toBe(true);
108115

109116
expect(elmBody.children().length).toBe(2);
@@ -124,15 +131,16 @@ describe('popover', function() {
124131
tooltipScope = elmScope.$$childTail;
125132

126133
elm.trigger('click');
134+
tooltipScope.$digest();
127135
expect(tooltipScope.isOpen).toBe(true);
128136

129137
expect(elmBody.children().length).toBe(2);
130138
var ttipElement = elmBody.find('div.popover');
131139
expect(ttipElement).toHaveClass('custom');
132140
}));
133141
});
134-
135-
describe( 'is-open', function() {
142+
143+
describe('is-open', function() {
136144
beforeEach(inject(function ($compile) {
137145
scope.isOpen = false;
138146
elmBody = angular.element(
@@ -144,8 +152,8 @@ describe('popover', function() {
144152
elmScope = elm.scope();
145153
tooltipScope = elmScope.$$childTail;
146154
}));
147-
148-
it( 'should show and hide with the controller value', function() {
155+
156+
it('should show and hide with the controller value', function() {
149157
expect(tooltipScope.isOpen).toBe(false);
150158
elmScope.isOpen = true;
151159
elmScope.$digest();
@@ -154,11 +162,13 @@ describe('popover', function() {
154162
elmScope.$digest();
155163
expect(tooltipScope.isOpen).toBe(false);
156164
});
157-
158-
it( 'should update the controller value', function() {
165+
166+
it('should update the controller value', function() {
159167
elm.trigger('click');
168+
tooltipScope.$digest();
160169
expect(elmScope.isOpen).toBe(true);
161170
elm.trigger('click');
171+
tooltipScope.$digest();
162172
expect(elmScope.isOpen).toBe(false);
163173
});
164174
});

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe('tooltip template', function() {
3434
evt = new Event(evt);
3535

3636
element[0].dispatchEvent(evt);
37+
element.scope().$$childTail.$digest();
3738
}
3839

3940
it('should open on mouseenter', inject(function() {
@@ -55,10 +56,10 @@ describe('tooltip template', function() {
5556

5657
it('should show updated text', inject(function() {
5758
scope.myTemplateText = 'some text';
58-
scope.$digest();
5959

6060
trigger(elm, 'mouseenter');
6161
expect(tooltipScope.isOpen).toBe(true);
62+
scope.$digest();
6263

6364
expect(elmBody.children().eq(1).text().trim()).toBe('some text');
6465

0 commit comments

Comments
 (0)