Skip to content

Conversation

svermillion
Copy link

Fixes #7917. This positioning shouldn't be left up to the theme's css.

@scottgonzalez
Copy link
Member

How is this the problem and why can't it be fixed in the stylesheet?

@svermillion
Copy link
Author

Without it, the overlay is added to the bottom of the page at a height that is approximately equal to the client viewport height an incorrect width (a bit wider than the client viewport). This is undesirable beyond the point of style:

1.) The modal dialog isn't truly modal since the overlay does not actually overlay the page's content and still allows, at the very least, mouseover events definitely still fire in the page.
2.) It drastically alters the page layout, making it about twice the height and wider, causing both scroll bars to appear and/or increase noticeably.

I'm not sure where the line is typically drawn in jQueryUI in regards to style vs. function, but this seems firmly on the function side.

@scottgonzalez
Copy link
Member

.ui-wdget-overlay is already absolutely positioned form jquery.ui.core.css. I don't see how your change helps.

@svermillion
Copy link
Author

It would fix modal for people not using the themes, which is how this ticket likely came about and how I have noticed it myself. It seems weird to me that modal = true breaks if you don't use the official themes, but maybe that is a normal/acceptable caveat to going custom on style. If so, I think the associated ticket should be closed as "won't fix".

@scottgonzalez
Copy link
Member

That's an invalid use of jQuery UI. Our functional CSS is required, our theme CSS is not. I've asked the original reporter of the ticket to provide a reduced test case so we can see if he's not including the core CSS (he's clearly using the base theme in the screenshots).

@svermillion
Copy link
Author

Wow, my bad. My apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants