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

Commit

Permalink
feat(modal): remove replace usage
Browse files Browse the repository at this point in the history 
- Remove `replace: true` usage

BREAKING CHANGE: This removes `replace: true` usage, causing some structural changes to the HTML - the major part here is that there is no more backdrop template, and the top level elements for the window/backdrop elements lose a little flexibility. See documentation examples for new structure.

Closes #5989
  • Loading branch information
@wesleycho
wesleycho committed Jun 13, 2016
1 parent 55cf26b commit 96d52ce
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 41 deletions.
39 changes: 26 additions & 13 deletions src/modal/modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap', 'ui.bootstrap.p
.directive('uibModalBackdrop', ['$animate', '$injector', '$uibModalStack',
function($animate, $injector, $modalStack) {
return {
replace: true,
templateUrl: 'uib/template/modal/backdrop.html',
restrict: 'A',
compile: function(tElement, tAttrs) {
tElement.addClass(tAttrs.backdropClass);
return linkFn;
Expand Down Expand Up @@ -136,13 +135,12 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap', 'ui.bootstrap.p
scope: {
index: '@'
},
replace: true,
restrict: 'A',
transclude: true,
templateUrl: function(tElement, tAttrs) {
return tAttrs.templateUrl || 'uib/template/modal/window.html';
},
link: function(scope, element, attrs) {
element.addClass(attrs.windowClass || '');
element.addClass(attrs.windowTopClass || '');
scope.size = attrs.size;

Expand All @@ -167,12 +165,9 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap', 'ui.bootstrap.p

// Deferred object that will be resolved when this modal is render.
var modalRenderDeferObj = $q.defer();
// Observe function will be called on next digest cycle after compilation, ensuring that the DOM is ready.
// In order to use this way of finding whether DOM is ready, we need to observe a scope property used in modal's template.
attrs.$observe('modalRender', function(value) {
if (value === 'true') {
modalRenderDeferObj.resolve();
}
// Resolve render promise post-digest
scope.$$postDigest(function() {
modalRenderDeferObj.resolve();
});

modalRenderDeferObj.promise.then(function() {
Expand Down Expand Up @@ -477,7 +472,16 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap', 'ui.bootstrap.p
backdropScope.modalOptions = modal;
backdropScope.index = currBackdropIndex;
backdropDomEl = angular.element('<div uib-modal-backdrop="modal-backdrop"></div>');
backdropDomEl.attr('backdrop-class', modal.backdropClass);
backdropDomEl.attr({
'class': 'modal-backdrop',
'ng-style': '{\'z-index\': 1040 + (index && 1 || 0) + index*10}',
'uib-modal-animation-class': 'fade',
'modal-in-class': 'in'
});
if (modal.backdropClass) {
backdropDomEl.addClass(modal.backdropClass);
}

if (modal.animation) {
backdropDomEl.attr('modal-animation', 'true');
}
Expand All @@ -493,13 +497,22 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap', 'ui.bootstrap.p
topModalIndex = previousTopOpenedModal ? parseInt(previousTopOpenedModal.value.modalDomEl.attr('index'), 10) + 1 : 0;
var angularDomEl = angular.element('<div uib-modal-window="modal-window"></div>');
angularDomEl.attr({
'class': 'modal',
'template-url': modal.windowTemplateUrl,
'window-class': modal.windowClass,
'window-top-class': modal.windowTopClass,
'role': 'dialog',
'size': modal.size,
'index': topModalIndex,
'animate': 'animate'
'animate': 'animate',
'ng-style': '{\'z-index\': 1050 + index*10, display: \'block\'}',
'tabindex': -1,
'uib-modal-animation-class': 'fade',
'modal-in-class': 'in'
}).html(modal.content);
if (modal.windowClass) {
angularDomEl.addClass(modal.windowClass);
}

if (modal.animation) {
angularDomEl.attr('modal-animation', 'true');
}
Expand Down
20 changes: 3 additions & 17 deletions src/modal/test/modal.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('$uibModal', function() {

beforeEach(module('ngAnimateMock'));
beforeEach(module('ui.bootstrap.modal'));
beforeEach(module('uib/template/modal/backdrop.html'));
beforeEach(module('uib/template/modal/window.html'));
beforeEach(module(function(_$controllerProvider_, _$uibModalProvider_, $compileProvider) {
$controllerProvider = _$controllerProvider_;
Expand Down Expand Up @@ -1415,15 +1414,6 @@ describe('$uibModal', function() {
expect($rootScope.foo).toBeTruthy();
});

it('should support custom CSS classes as string', function() {
$rootScope.animate = false;
var windowEl = $compile('<div uib-modal-window animate="animate" window-class="test foo">content</div>')($rootScope);
$rootScope.$digest();

expect(windowEl).toHaveClass('test');
expect(windowEl).toHaveClass('foo');
});

it('should support window top class', function () {
$rootScope.animate = false;
var windowEl = $compile('<div uib-modal-window animate="animate" window-top-class="test foo">content</div>')($rootScope);
Expand All @@ -1434,13 +1424,12 @@ describe('$uibModal', function() {
});

it('should support custom template url', inject(function($templateCache) {
$templateCache.put('window.html', '<div class="mywindow" ng-transclude></div>');
$templateCache.put('window.html', '<div ng-transclude></div>');

var windowEl = $compile('<div uib-modal-window template-url="window.html" window-class="test">content</div>')($rootScope);
var windowEl = $compile('<div uib-modal-window template-url="window.html">content</div>')($rootScope);
$rootScope.$digest();

expect(windowEl).toHaveClass('mywindow');
expect(windowEl).toHaveClass('test');
expect(windowEl.html()).toBe('<div ng-transclude=""><span class="ng-scope">content</span></div>');
}));
});

Expand Down Expand Up @@ -1726,7 +1715,6 @@ describe('$uibModal', function() {
content: '<div>Modal1</div>'
});

expect($document).toHaveModalsOpen(0);
$rootScope.$digest();
$animate.flush();
expect($document).toHaveModalsOpen(1);
Expand All @@ -1746,7 +1734,6 @@ describe('$uibModal', function() {
modal2Index = parseInt($uibModalStack.getTop().value.modalDomEl.attr('index'), 10);
});

expect($document).toHaveModalsOpen(1);
$rootScope.$digest();
$animate.flush();
expect($document).toHaveModalsOpen(2);
Expand All @@ -1768,7 +1755,6 @@ describe('$uibModal', function() {
modal3Index = parseInt($uibModalStack.getTop().value.modalDomEl.attr('index'), 10);
});

expect($document).toHaveModalsOpen(1);
$rootScope.$digest();
$animate.flush();
expect($document).toHaveModalsOpen(2);
Expand Down
5 changes: 0 additions & 5 deletions template/modal/backdrop.html
View file

This file was deleted.

7 changes: 1 addition & 6 deletions template/modal/window.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1 @@
<div modal-render="{{$isRendered}}" tabindex="-1" role="dialog" class="modal"
uib-modal-animation-class="fade"
modal-in-class="in"
ng-style="{'z-index': 1050 + index*10, display: 'block'}">
<div class="modal-dialog {{size ? 'modal-' + size : ''}}"><div class="modal-content" uib-modal-transclude></div></div>
</div>
<div class="modal-dialog {{size ? 'modal-' + size : ''}}"><div class="modal-content" uib-modal-transclude></div></div>

8 comments on commit 96d52ce

@fedyk
Copy link
Contributor

@fedyk fedyk commented on 96d52ce Jun 14, 2016

Choose a reason for hiding this comment

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

@wesleycho You have removed template/modal/backdrop.html but in src/modal/index-nocss.js you still require this template.

@wesleycho
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch - want to file a PR?

@fedyk
Copy link
Contributor

@fedyk fedyk commented on 96d52ce Jun 14, 2016

Choose a reason for hiding this comment

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

Yes, here you have PR #5999

@mischah
Copy link

@mischah mischah commented on 96d52ce Jul 5, 2016

Choose a reason for hiding this comment

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

and the top level elements for the window/backdrop elements lose a little flexibility

@angular-ui

This means I can’t update angular-ui cause I need that flexibility. Or will there be something like windowTopTemplateURL To override the most outer container?

//cc @krnlde

@wesleycho
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mischah can you open an issue with your use case/details as to what you need exactly? We can probably add support for what you need.

@manikandanvs
Copy link

@manikandanvs manikandanvs commented on 96d52ce Jul 5, 2016 via email

Choose a reason for hiding this comment

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

@wesleycho
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please do not post questions not relevant to this commit here - if it is a support issue, use the appropriate venue such as StackOverflow or the Angular IRC channel. If it is a legitimate library issue (or appears to be), open an issue with a reproduction.

@icfantv
Copy link
Contributor

@icfantv icfantv commented on 96d52ce Jul 5, 2016

Choose a reason for hiding this comment

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

@manikandanvs, what you did is called thread hijacking and is very much frowned upon as it can totally derail and distract from the conversation. This PR deals with modal and you posted a support question about pagination. First off, the issues forum (and certainly not the PR forum) is not for support requests. Rather you need to read the README and FAQs to see how to properly ask for (and receive) help.

Please sign in to comment.