search

YaleSTC / reservations

Manage equipment loans & reservations. Who can borrow what, for how long?
yalestc.github.io/reservations
MIT License
139 stars 58 forks source link

Refactor modals #1143

Open mnquintana opened 9 years ago

mnquintana commented 9 years ago

From working on #1116, I've realized that the way we handle modals right now is really inconsistent and can cause display bugs. Currently, we define modals as needed by inserting them in the views that require them, even multiple modal components in the same view if multiple different modals need to be displayed. This means that modal markup is often deep in the document structure, which Bootstrap's docs warn us about:

Modal markup placement

Always try to place a modal's HTML code in a top-level position in your document to avoid other components affecting the modal's appearance and/or functionality.

This is already a (out of scope) problem in #1116 with the Quick New User modal, which renders under the navbar because it is nested too deep (z-index doesn't fix this).

We should have only one modal component, nested only inside <body> whose content is loaded dynamically either by the server on page load or with an AJAX request, whichever is more appropriate.

orenyk commented 9 years ago

Good call. I feel like this would fall nicely under the #1061 rewrite; what do you think?

mnquintana commented 9 years ago

Hmm, so the way I've been thinking of #1061 is that we're going to need to do a lot of setup and refactoring before it's even possible, so in that sense, yes I think it fits as "part" of #1061. But I don't think we should wait until we have a JS front-end before we try doing this, if that makes sense.

orenyk commented 9 years ago

Hmm, I meant more as a function of the fact that the overall #1061 / v6.0.0 process will include significant view rewriting, so fixing the modal would naturally fit in as part of that process. I'll stick it in that milestone for now, we can revisit it later.