bug(modal): nested modal backdrops don't obscure or disable parent modal

[jmwolfe]

Hello all! This is my first entry on GitHub, but I've been using angular-ui and angular-bootstrap components for a few months.

I really appreciate the rewrite of the modal and I'm using it on a big enterprise app.

There seems to still be an issue with 'nested' modals. ie a modal opened from another modal. The backdrop from the second modal does not obscure and deactivate the layer modal beneath it. Like the old $dialog, the new backdrop adds opacity to the backdrop but is not z-indexed right to cover the parent dialog/modal.

Admittedly, it's not a true defect at this second, because all dialogs are the same size, so the child modal obscures the parent modal and prevents clicks getting to it. To see this issue, you have to have the css extension fix in place (as noted on Issue #892, custom css class in $modal template) so that the child dialog can be smaller than the parent. Until @pkozlowski-opensource makes his fix in the master, I just used the tweak noted by @richardcrichardc in the issue.

Perhaps its something that can be fixed when the window class extension is fixed, because once you can change the size of the window, you will have these operational issues like Issue #738, modal does not stop tabbing on the background, and of course the problem noted in this issue item.

If we can brainstorm on the best design to fix the issue, I would be happy to try a pull request and to code it.

[jmwolfe]

Well, I have it working now in my local copy... here's what I did. Would appreciate any feedback to make it contribution worthy:

• modified $$stackedMap service to append a stackIndex value to the 'value' so that the window itself will know its order.
• added $modalStack to the list of injectables to modalWindow
• in the modalBackdrop and modalWindow directives, I use element[0].setAttribute('style') to set a z-index based on the stackIndex value posted on the modal window object (internal, not HTML DOM). The first pair gets 1040 and 1050 (the default zvalues for bootstrap's .modal CSS classes) and each window pushed on the stack increases the zindex by 10.

This works perfectly, even in IE8.

I used the style DIV attribute, because I was unsure of how to to be sure how to get at the styles for the div after all classes were applied. If someone could explain how to do that from within the directive, then I could probably just set the z-index on the div directly ( not use the style attribute on the DIV).

if that is the best way, I suppose I would need to add code to be sure I am not clobbering some existing style attribute. But for my project I know there is no style on the template so it works in the small.

Feedback?

[pkozlowski-opensource]

@jmwolfe thnx for bringing this up. I think that your approach sounds resonable, the only thing I would do is to move more logic to a templates, using the ng-style directive. So yes, making a window aware of its order (position in a stack) and injecting this position in the window directive makes sense.

I've proposed something for #892 would be great if you could see if this would work for you.

[jmwolfe]

@pkozlowski-opensource Thanks for the reply. Not sure I follow how I could move any of this logic to the template. Are you suggesting using ng-class to call a scope callback that gets the z-index somehow from the scope (that might be tricky.. would transclusion bollux things up?) and then map it to one of many z-index classes which implement the stacking?

We would have to create a whole bunch of classes to support that, it seems. As far as I can see we don't generally add any classes with these AngularUI modules, right? Is there some way to dynamically inject classes?

Ah... hm... we could inline a stylesheet into backdrop.html and window.html to support up to 10 layers of popups. But this would make even harder the issue we're working out on #892 I think.

So something like the above is better because it lets ngClass take care of duplicate classes (and doesn't require parsing of the existing style attribute? It seems more convoluted and complicated. What big principle am I violating by just enforcing a zindex in the directive on the HTML element?

The fix for #892 sounds great, although I assume you're asking regarding THAT issue... it won't fix this issue or provide any better solutions. I guess I can wait until you have your solution in, and then implement one for this issue. I don't think I'll get to it before you can do 892. not to mention I don't have an Angular dev environment set up so that will take me extra time there. Would be cool if there was a virtual image set up for Azure that we could just stand up and start going. :)

[jmwolfe]

Actually, I did find time. Submitted pull request #931 with commit 96fb17c.

[chuckd]

AFAICT this is broken in 1.2 due to the change that properly isolates isolate scopes. The index property is no longer accessible in this ng-style definition:

ng-style="{'z-index': 1050 + index*10}"

I'll keep investigating and open an issue.

[getuliojr]

Has this been fixed or is there a issue related ?

This is closed but I still get this bug. If this has not been fixed I will take a look.

[chuckd]

I believe it is fixed on master, waiting for a release to be cut. We are currently overriding the template to fix this - I'll look it up and post later.

[chuckd]

Here is our template override which makes the backdrops layer correctly on ui-bootstrap 0.10:

$templateCache.put("template/modal/backdrop.html",
                   """
                   <div class="modal-backdrop fade"
                        ng-class="{in: animate}"
                        ng-style="{'z-index': 1040 + (index && 1 || 0) + index*10}"></div>
                   """)

[getuliojr]

@chuckd worked just fine. Thanks.