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

multiple modals & z-index issue #5670

Closed
anakreon opened this issue Mar 22, 2016 · 6 comments
Closed

multiple modals & z-index issue #5670

anakreon opened this issue Mar 22, 2016 · 6 comments

Comments

@anakreon
Copy link

Bug description:

Let's say multiple modals are opened at the same time - modals [A, B, C, D], opened in this order. If I close the first opened modal A, then open another modal E -> [B, C, D, E]. The newly opened modal E has the same index & z-index as the previous modal D. With the current reversed order of modals in DOM & same z-index, modal E will not be visible (will be covered by modal D).

Link to minimally-working plunker that reproduces the issue:

https://plnkr.co/edit/GNZtzlaxDM0X8STp8fTD
In the plunkr, i set the limit to 5. So the issue is obvious after 5 opened modals...

Version of Angular, UIBS, and Bootstrap

Angular 1.4.9
UIBS 1.1.2
Bootstrap 3.3.6

@josetaira
Copy link
Contributor

File reference: src/modal/modal.js

Seems like it's related to the calculation of z-index (1050 + 10*index). The "backdropScope.index" of each existing modal is not updated when you remove a modal. I think the assumption was that normally you'd only remove the top modal?

The function backdropIndex() returns the value that's expected for the topmost modal you're inserting it seems.

So in your example, the modals are stuck with the z-index 1090 as long as 5 modals are open when you try to add another one then all succeeding modals are added with z-index 1090.

@wesleycho
Copy link
Contributor

Hm, it sounds like the whole idea of the index needs to be rethought - the index is just computed once as openedWindows.length() - 1, which introduces this problem.

The z-index calculation needs to accommodate changing window lengths and modal indices indicating the order they were opened.

PRs welcome.

@wesleycho
Copy link
Contributor

This is a low priority issue - this looks like it requires a not-insignificant amount of new logic to accommodate this feature properly, so if this is to be fixed, filing a PR is the best route.

@icfantv
Copy link
Contributor

icfantv commented Apr 4, 2016

@anakreon, I'm also going to add that an application with multiple modals beyond two is a pretty bad design and UX, IMO. I could easily see having two in the following use case:

  1. user clicks something which opens a modal dialog for editing.
  2. user makes changes to form.
  3. user attempts to navigate away or close modal without saving changes.
  4. modal is displayed warning of unsaved changes.

@wesleycho unless someone can convince me of another use case where 3+ modals are appropriate, I suggest we close this as won't fix and update the documentation indicating that the use case put forth above (i.e., 3+ modals and closing some in the middle) will not be supported due to poor UX.

@wesleycho
Copy link
Contributor

Well, one could presumably come across this bug even with 3 modals.

@icfantv
Copy link
Contributor

icfantv commented Apr 4, 2016

Yup, which is why I asked for a use case of using 3 or more modals.

@wesleycho wesleycho modified the milestones: 1.3.1, Backlog Apr 5, 2016
@wesleycho wesleycho removed the PRs plz! label Apr 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants