More flexible modal box centering

[ydmitry]

When you trying to change inner content by javascript, the margin-top position stays the same and window looks like uncentered. See screen from example:

If you use css centering, you can avoid this

[ghost]

That patch seems to work on wip 2.1 branch.
Thank you @ydmitry !

[jschr]

Hey thanks for the pull request, the seems like a more robust solution. I'd like to push out the 2.1 branch first before merging this just to get it out of the way. I'll be doing this sometime this week, I just haven't had a chance to fully test it yet.

Out of curiousity, why do we need the css rules for .modal-scrollable:after?

Thanks

[ydmitry]

vertical-align property applies to a neighbor element, not a parent. .modal-scrollable:after creates such an element with height: 100%; width: 0; and vertical-align: middle; properties that will behave like one that appies to a parent block.

[ghost]

Mmmh that patch works perfectly on Firefox 17 and Firefox 18 but doesn't seem to work on Chromium 26; the popup is full width instead to be centered.

Can you confirm ?

Edit:
Seems to be that code in bootstrap-modalmanager.js in the average of line 71 ~75:

if (transition) {
    modal.$element[0].style.display = 'run-in';
    modal.$element[0].offsetWidth;
    modal.$element.one($.support.transition.end, function () { modal.$element.show(); /*modal.$element[0].style.display = 'block' */});
}

becomes

if (transition) {
    /*modal.$element[0].style.display = 'run-in'; */
    modal.$element[0].offsetWidth;
    modal.$element.one($.support.transition.end, function () { modal.$element.show(); /*modal.$element[0].style.display = 'block' */});
}

[ydmitry]

@cmenassa 👍
Yes, the reason is in display: run-in; property... What for it was necessary?

[jschr]

That block of code is there to force a reflow due to a webkit bug (See http://stackoverflow.com/questions/3485365/how-can-i-force-webkit-to-redraw-repaint-to-propagate-style-changes).

I'm unsure why 'run-in' was chosen, but probably because I couldn't use display: none or transitions would break.

I took the entire block of code out and am not seeing any issues in the latest version of chrome. I will do some more testing on the wip-2.1 branch but it looks like it can be removed entirely.

[ghost]

The problem is still here on Firefox. I've to resize my browser window to force modal to be centered.
On Chromium, no problem.

[codegoat]

With the addition of display: inline-block to .modal, it overrides the .hide class and the modal ends up appearing in the page before it has been activated (on load of the page, it appears where ever the modal is in the html).

If the modal has .fade instead of .hide, it will be less noticeable as it will have an opacity of 0, but because .modal is now position: relative, it will take up space on the page.

I really like the look of these changes, but these are the issues I have encountered with them so far.

I am working on a resolution right now, and will post further when I finalize one.

[jschr]

I don't think I will be accepting this PR at this time. It's a pretty neat enhancement but I'm concerned about the potential jarring experience if the user does not expect the modal to grow. Consider the case with tabs inside a modal. The modal would automatically reposition on tab click interrupting the flow.

I would feel more comfortable with a manual reposition which I have included in the latest version using the layout function.

This could potentially be made even easier with some of the ideas in #87.

Thanks, appreciate the pull request.