Skip to content

Commit

Permalink
refactor(tooltip): rollup commit for several issues
Browse files Browse the repository at this point in the history 
This is a rollup commit intended to address several
issues around the positioning and parsing of
attributes.

- Fixes issue introduced under PR angular-ui#4311 where setting
  height and width in tooltip position function
  messed up arrow placement.
- Fixes issue introduced under PR angular-ui#4363 where setting
  visibility to hidden in tooltip position function
  caused elements in popover to lose focus.
- Fixes issue angular-ui#1780 where tooltip would render if
  content was just whitespace.
- Fixes issue angular-ui#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 angular-ui#3557 by implementing evalAsync to set
  tooltip scope isOpen property.
- Fixes issue angular-ui#4335 where if model isOpen property is
  undefined, tooltip would call show/hide toggle function.
- Closes PR angular-ui#4429 where how the templated content
  was being evaluated could cause an infinite digest loop.

Closes angular-ui#4400
Closes angular-ui#4418
Closes angular-ui#4429
Closes angular-ui#4431
Closes angular-ui#4455

Fixes angular-ui#1780
Fixes angular-ui#3347
Fixes angular-ui#3557
Fixes angular-ui#4321
Fixes angular-ui#4335
  • Loading branch information
@RobJacobs @jasonaden
RobJacobs authored and jasonaden committed Sep 30, 2015
1 parent ff4bd50 commit 637721b
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 121 deletions.
23 changes: 17 additions & 6 deletions src/popover/test/popover-html.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,20 @@ describe('popover', function() {

it('should open on click', inject(function() {
elm.trigger('click');
expect( tooltipScope.isOpen ).toBe(true);
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

it('should close on second click', inject(function() {
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(false);
}));

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(false);

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

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

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

elm.trigger('mouseenter');
tooltipScope.$digest();
elm.trigger('mouseleave');
tooltipScope.$digest();
expect(scope.clicked).toBeFalsy();

elm.click();
expect(scope.clicked).toBeTruthy();
}));

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

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

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

expect(elmBody.children().length).toBe(2);
Expand All @@ -174,5 +187,3 @@ describe('popover', function() {
});
});
});


14 changes: 10 additions & 4 deletions src/popover/test/popover-template.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,32 @@ describe('popover template', function() {
}));

it('should open on click', inject(function() {
elm.trigger( 'click' );
expect( tooltipScope.isOpen ).toBe( true );
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(false);

expect(elmBody.children().length).toBe(1);
}));

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

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

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

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

expect(elmBody.children().length).toBe(2);
Expand Down
22 changes: 16 additions & 6 deletions src/popover/test/popover.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('popover', function() {

it('should open on click', inject(function() {
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

it('should close on second click', inject(function() {
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);
elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(false);
}));

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

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

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

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

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

elm.trigger('click');
tooltipScope.$digest();
expect(tooltipScope.isOpen).toBe(true);

expect(elmBody.children().length).toBe(2);
var ttipElement = elmBody.find('div.popover');
expect(ttipElement).toHaveClass('custom');
}));
});
describe( 'is-open', function() {

describe('is-open', function() {
beforeEach(inject(function ($compile) {
scope.isOpen = false;
elmBody = angular.element(
Expand All @@ -144,8 +152,8 @@ describe('popover', function() {
elmScope = elm.scope();
tooltipScope = elmScope.$$childTail;
}));
it( 'should show and hide with the controller value', function() {

it('should show and hide with the controller value', function() {
expect(tooltipScope.isOpen).toBe(false);
elmScope.isOpen = true;
elmScope.$digest();
Expand All @@ -154,11 +162,13 @@ describe('popover', function() {
elmScope.$digest();
expect(tooltipScope.isOpen).toBe(false);
});
it( 'should update the controller value', function() {

it('should update the controller value', function() {
elm.trigger('click');
tooltipScope.$digest();
expect(elmScope.isOpen).toBe(true);
elm.trigger('click');
tooltipScope.$digest();
expect(elmScope.isOpen).toBe(false);
});
});
Expand Down
3 changes: 2 additions & 1 deletion src/tooltip/test/tooltip-template.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('tooltip template', function() {
evt = new Event(evt);

element[0].dispatchEvent(evt);
element.scope().$$childTail.$digest();
}

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

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

trigger(elm, 'mouseenter');
expect(tooltipScope.isOpen).toBe(true);
scope.$digest();

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

Expand Down
Loading

0 comments on commit 637721b

Please sign in to comment.