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

fix(modal): compile modal before inserting into DOM #5224

Closed
wants to merge 1 commit into from
Closed

fix(modal): compile modal before inserting into DOM #5224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions src/modal/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,9 +477,8 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap'])
angularDomEl.attr('modal-animation', 'true');
}

$animate.enter(angularDomEl, appendToElement)
$animate.enter($compile(angularDomEl)(modal.scope), appendToElement)
.then(function() {
$compile(angularDomEl)(modal.scope);
$animate.addClass(appendToElement, modalBodyClass);
});

Expand Down
84 changes: 52 additions & 32 deletions src/modal/test/modal.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,16 @@ describe('$uibModal', function () {
element.trigger(e);
}

function open(modalOptions, noFlush) {
function open(modalOptions, noFlush, noDigest) {
var modal = $uibModal.open(modalOptions);
$rootScope.$digest();
if (!noFlush) {
$animate.flush();

if (!noDigest) {
$rootScope.$digest();
if (!noFlush) {
$animate.flush();
}
}

return modal;
}

Expand Down Expand Up @@ -261,6 +265,46 @@ describe('$uibModal', function () {
expect($document).not.toHaveBackdrop();
});

it('should compile modal before inserting into DOM', function() {
var topModal;
var modalInstance = {
result: $q.defer(),
opened: $q.defer(),
closed: $q.defer(),
rendered: $q.defer(),
close: function (result) {
return $uibModalStack.close(modalInstance, result);
},
dismiss: function (reason) {
return $uibModalStack.dismiss(modalInstance, reason);
}
};
var expectedText = 'test';

$uibModalStack.open(modalInstance, {
appendTo: angular.element(document.body),
scope: $rootScope.$new(),
deferred: modalInstance.result,
renderDeferred: modalInstance.rendered,
closedDeferred: modalInstance.closed,
content: '<div id="test">{{\'' + expectedText + '\'}}</div>'
});

topModal = $uibModalStack.getTop();

expect(topModal.value.modalDomEl.find('#test').length).toEqual(0);
expect(angular.element('#test').length).toEqual(0);

$rootScope.$digest();

expect(topModal.value.modalDomEl.find('#test').text()).toEqual(expectedText);
expect(angular.element('#test').text()).toEqual(expectedText);

$animate.flush();

close(modalInstance, 'closing in test', true);
});

it('should not throw an exception on a second dismiss', function() {
var modal = open({template: '<div>Content</div>'});

Expand Down Expand Up @@ -364,7 +408,6 @@ describe('$uibModal', function () {
expect(document.activeElement.tagName).toBe('A');

var modal = open({template: '<div>Content<button>inside modal</button></div>'});
$animate.flush();
$rootScope.$digest();
expect(document.activeElement.tagName).toBe('DIV');
expect($document).toHaveModalsOpen(1);
Expand All @@ -389,7 +432,6 @@ describe('$uibModal', function () {
expect(document.activeElement.tagName).toBe('A');

var modal = open({template: '<div>Content</div>'});
$animate.flush();
$rootScope.$digest();
expect(document.activeElement.tagName).toBe('DIV');
expect($document).toHaveModalsOpen(1);
Expand Down Expand Up @@ -468,7 +510,6 @@ describe('$uibModal', function () {
it('should focus on the element that has autofocus attribute when the modal is open/reopen and the animations have finished', function() {
function openAndCloseModalWithAutofocusElement() {
var modal = open({template: '<div><input type="text" id="auto-focus-element" autofocus></div>'});
$animate.flush();
$rootScope.$digest();
expect(angular.element('#auto-focus-element')).toHaveFocus();

Expand All @@ -485,7 +526,6 @@ describe('$uibModal', function () {
function openAndCloseModalWithAutofocusElement() {

var modal = open({template: '<div><input type="text" id="auto-focus-element" autofocus><input type="text" id="pre-focus-element" focus-me></div>'});
$animate.flush();
$rootScope.$digest();
expect(angular.element('#auto-focus-element')).not.toHaveFocus();
expect(angular.element('#pre-focus-element')).toHaveFocus();
Expand All @@ -501,11 +541,11 @@ describe('$uibModal', function () {

it('should wait until the in animation is finished before attempting to focus the modal or autofocus element', function() {
function openAndCloseModalWithAutofocusElement() {
var modal = open({template: '<div><input type="text" id="auto-focus-element" autofocus></div>'});
var modal = open({template: '<div><input type="text" id="auto-focus-element" autofocus></div>'}, true, true);
expect(angular.element('#auto-focus-element')).not.toHaveFocus();

$animate.flush();
$rootScope.$digest();
$animate.flush();

expect(angular.element('#auto-focus-element')).toHaveFocus();

Expand All @@ -521,11 +561,11 @@ describe('$uibModal', function () {
element.focus();
expect(document.activeElement.tagName).toBe('A');

var modal = open({template: '<div><input type="text"></div>'});
var modal = open({template: '<div><input type="text"></div>'}, true, true);
expect(document.activeElement.tagName).toBe('A');

$animate.flush();
$rootScope.$digest();
$animate.flush();

expect(document.activeElement.tagName).toBe('DIV');

Expand Down Expand Up @@ -788,7 +828,6 @@ describe('$uibModal', function () {
expect($document).toHaveModalsOpen(0);

$timeout.flush();
$animate.flush();
expect($document).toHaveModalOpenWithContent('Promise', 'div');
});

Expand Down Expand Up @@ -886,14 +925,12 @@ describe('$uibModal', function () {

it('should contain backdrop in classes on each modal opening', function() {
var modal = open({ template: '<div>With backdrop</div>' });
$animate.flush();
var backdropEl = $document.find('body > div.modal-backdrop');
expect(backdropEl).toHaveClass('in');

dismiss(modal);

modal = open({ template: '<div>With backdrop</div>' });
$animate.flush();
backdropEl = $document.find('body > div.modal-backdrop');
expect(backdropEl).toHaveClass('in');

Expand Down Expand Up @@ -1031,7 +1068,6 @@ describe('$uibModal', function () {
angular.element(document.body).append(element);

open({template: '<div child-directive>{{text}}</div>', appendTo: element});
$animate.flush();
expect($document.find('[child-directive]').text()).toBe('foo');

element.remove();
Expand All @@ -1050,8 +1086,6 @@ describe('$uibModal', function () {
template: '<div>dummy modal</div>'
});

$animate.flush();

expect(body).toHaveClass('modal-open');
});

Expand All @@ -1061,8 +1095,6 @@ describe('$uibModal', function () {
openedClass: 'foo'
});

$animate.flush();

expect(body).toHaveClass('foo');
expect(body).not.toHaveClass('modal-open');
});
Expand All @@ -1073,8 +1105,6 @@ describe('$uibModal', function () {
openedClass: 'foo'
});

$animate.flush();

expect(body).toHaveClass('foo');

close(modal);
Expand All @@ -1088,8 +1118,6 @@ describe('$uibModal', function () {
openedClass: 'foo'
});

$animate.flush();

expect(body).toHaveClass('foo');
expect(body).not.toHaveClass('modal-open');

Expand All @@ -1098,8 +1126,6 @@ describe('$uibModal', function () {
openedClass: 'bar'
});

$animate.flush();

expect(body).toHaveClass('foo');
expect(body).toHaveClass('bar');
expect(body).not.toHaveClass('modal-open');
Expand All @@ -1109,8 +1135,6 @@ describe('$uibModal', function () {
openedClass: 'foo'
});

$animate.flush();

expect(body).toHaveClass('foo');
expect(body).toHaveClass('bar');
expect(body).not.toHaveClass('modal-open');
Expand Down Expand Up @@ -1232,11 +1256,9 @@ describe('$uibModal', function () {
expect(body).not.toHaveClass('modal-open');

var modal1 = open({template: '<div>Content1</div>'});
$animate.flush();
expect(body).toHaveClass('modal-open');

var modal2 = open({template: '<div>Content1</div>'});
$animate.flush();
expect(body).toHaveClass('modal-open');

dismiss(modal1);
Expand All @@ -1254,14 +1276,12 @@ describe('$uibModal', function () {
expect(document.activeElement.tagName).toBe('A');

var modal1 = open({template: '<div>Modal1<button id="focus">inside modal1</button></div>'});
$animate.flush();
$rootScope.$digest();
document.getElementById('focus').focus();
expect(document.activeElement.tagName).toBe('BUTTON');
expect($document).toHaveModalsOpen(1);

var modal2 = open({template: '<div>Modal2</div>'});
$animate.flush();
$rootScope.$digest();
expect(document.activeElement.tagName).toBe('DIV');
expect($document).toHaveModalsOpen(2);
Expand Down
2 changes: 1 addition & 1 deletion template/modal/window.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
uib-modal-animation-class="fade"
modal-in-class="in"
ng-style="{'z-index': 1050 + index*10, display: 'block'}">
<div class="modal-dialog" ng-class="size ? 'modal-' + size : ''"><div class="modal-content" uib-modal-transclude></div></div>
<div class="modal-dialog {{size ? 'modal-' + size : ''}}"><div class="modal-content" uib-modal-transclude></div></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change made? This seems unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ngClass was going through the animation processes of all the before and after classes which was causing the size when picking small or large to be regular size when initially rendered and the jump to the correct size.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, that's a negative of it being tied to $animate...but I guess it can't be avoided then.

</div>