Updated: Comment #9
Problem/Motivation
When viewing the test listing page (admin/config/development/testing) within the overlay, the test group for overlay module doesn't appear correctly.
Proposed resolution
Adjust the ID in the simpletest module by prefixing "module-".
Before:
After:
Remaining tasks
None
User interface changes
Overlay module on admin/config/development/testing will appear correctly in overlay.
API changes
n/a
Related Issues
None yet.
Original report by @sayela
There is an ID (overlay) in the overlay.tpl template of the core overlay module that serves as a wrapper for the contents. This is however conflicting with another ID (overlay) that identifies the overlay module in the module listings. This results in the latter inheriting the CSS properties of the former thereby causing undesirable results.
Proposed Solution:
Change the ID wrapping the overlay (line 25 of overlay.tpl) from "overlay" to "overlay-wrapper" and then edit the overlay-child.css (line 17) accordingly.
Thank you.
Comment | File | Size | Author |
---|---|---|---|
#9 | 1961946-9.png | 13.62 KB | star-szr |
#4 | simpletest-css-id-1961946-4.patch | 741 bytes | TR |
#3 | 1961946-2.png | 17.11 KB | star-szr |
d8-overlay-bug.png | 14.05 KB | sayela |
Comments
Comment #2
mandclu CreditAttribution: mandclu commentedUnable to replicate. On a fresh Drupal 8 install it appears that the IDs are now prefixed with "module-" so it seems that this is unlikely to be an issue anymore.
Comment #3
star-szrFound a very similar bug on the testing page (admin/config/development/testing) with overlay enabled. There is a conflict with a td that has an ID of overlay. So I think #overlay-wrapper or another solution might be worth exploring.
Comment #4
TR CreditAttribution: TR commentedI can confirm the behavior reported by Cottser.
The SimpleTest module assigns a CSS ID for each of the check boxes on this page, and uses the name of the module as the ID. So the <tr> holding the Overlay module's checkbox has an ID of #overlay. Because this page is an admin page, it's shown within an overlay, so the overlay CSS gets loaded. But the #overlay ID is already targeted by CSS in the Overlay module, namely overlay-child.css. The properties assigned to #overly in overlay-child.css (specifically, the
min-height: 100px
andpadding: 2em 40ex
properties) are the ones that add the extra horizontal and vertical spacing.Probably the simplest fix is to rename all the IDs on this page -- choosing the module name as an ID is highly risky and is something that should only be done from within the module. The attached patch prefixes the IDs on this page with 'module-'.
Note, the original post is exactly this same problem, except on admin/modules instead of admin/configuration/development/testing. As stated in #2, admin/modules was fixed by changing the CSS ID on that page, which is exactly the same fix as I'm proposing here in my patch for admin/configuration/development/testing.
Comment #5
TR CreditAttribution: TR commentedBetter title.
Comment #6
star-szr@TR - Thanks, that seems reasonable. Longer term, do you think it would make sense to change the #overlay ID to something else, or should that be owned by the overlay module's "namespace"?
Comment #7
TR CreditAttribution: TR commentedYes, I think it probably makes sense to take the additional step for the overlay module to choose a less generic ID in the long run, but that's more of a "nice-to-have" rather than a bug. Re-writing the CSS generated and used by overlay isn't something that's going to happen anytime soon.
It's not an either/or fix - both can be done, and I certainly don't want this current issue to be sidetracked by an overlay rewrite effort. Let's fix the bug here, in the same way that admin/modules was already fixed.
Comment #8
star-szrI agree. Can you please open another issue for the overlay ID update and link it back here? The new issue could simply be changing #overlay to #overlay-wrapper to start with. That can happen sometime soon :)
Manually tested to confirm the bug is fixed, adding before/after screenshots to the issue summary.
Comment #9
star-szrIt helps if you attach the screenshot :)
Comment #10
webchickNice catch.
Committed and pushed to 8.x. Thanks!
Comment #11
star-szrHere's the follow-up issue to discuss reworking the #overlay ID: #2091665: Change #overlay ID to avoid conflicts
Comment #12.0
(not verified) CreditAttribution: commentedUpdate summary to use full template and add before/after screenshots