Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
off-canvas.theme.css
has a CSS rule that intends to override jQuery UI's default styling:
.ui-dialog.ui-dialog-off-canvas {
...
border: 0 solid transparent;
...
}
However, this specific override is not actually applied because jQuery UI's style rule wins, as can be seen in the "before" screenshot.
Proposed resolution
A naive implementation would be to make the off-canvas rule more specific.
Are there any other options?
Remaining tasks
Discuss, review.
User interface changes
The off canvas dialog won't have the 1px white border anymore.
API changes
Nope.
Data model changes
Nope.
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff-10.txt | 1.57 KB | amateescu |
#10 | 2976287-10.patch | 1.47 KB | amateescu |
#8 | 2976287-Firefox60-before.PNG | 43.17 KB | andrewmacpherson |
#8 | 2976287-IE11-before.PNG | 39.29 KB | andrewmacpherson |
#8 | 2976287-IE11-after-patch-2.PNG | 39.34 KB | andrewmacpherson |
Comments
Comment #2
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedHere's my naive attempt at fixing this problem :)
Before:
After:
I didn't send the patch for testing because it's a css only change.
Comment #3
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedHaving a 1px border is actually helpful. Who says it shouldn't be there? It conveys the extent of the settings tray, and reinforces a sense of "this is a separate area for admin tasks".
Without this, we'd be relying on the background-color alone to convey the extent of the settings tray. But when a Windows high-contrast theme is being used, background colours are removed, so you won't get the "separate area" visual affordance.
If we decide the 1px white border should not be there, it would be a good idea to keep a
1px solid transparent
border in place. When a Windows high-contrast theme is in use, this will become a visible 1px border, so the "separate area" will still be conveyed, even though background colours have been stripped out.Note there are lots of other issues with core themes breaking in Windows high-contrast mode, described in #2894237: Make core themes more robust in Windows High-Contrast mode. For an example of how a 1px transparent boxer helps, see the dilog screenshots in comment 3 of that issue.
Comment #4
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThanks, @andrewmacpherson, for the very thorough and helpful explanation!
My initial assessment that the border should not be there was based on the existing core style, which had
border: 0 solid transparent;
, but it wasn't applied because the CSS selector was not specific enough.However, I fully agree that the intent was most likely to remove 1px white border, and your suggested solution makes perfect sense :)
Comment #5
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRetitling the issue based on #3 / #4.
Comment #6
GrandmaGlassesRopeManAlright. This does change the stable theme, but it's so minor I think that's alright. 👍
Comment #7
lauriiiComment #8
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedSome screenshots from Windows with the high-contrast black theme in use. No patch, patch 2, and patch 4.
Chrome and Opera don't adjust to Windows high-contrast themes at all, that's beyond our control.
Here's what we get from patch #4 (IE 11, high-contrast):
I'm agnostic about whether we have the 1px white border in the full-colour space, but if we get rid of it, patch #4 is the best way.
Comment #9
lauriiiWe have to increase the weight of the selector to be able to override the JQuery defaults. Could we create a separate selector for the border with the higher weight to not increase the weight for all the CSS properties?
Comment #10
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented@lauriii, sure thing! And thanks for reviewing :)
Not sending the patch to the testbot because it's functionally equivalent to the previous one.
Comment #11
timmillwoodI guess this can go back to RTBC.
Looks fine to me anyway 😉
Comment #13
lauriiiThank you for making the changes @amateescu and @andrewmacpherson for the a11y review! This looks great. ✨
Committed abac90f and pushed to 8.6.x. 🚀Thanks!
Comment #14
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedAdding the related accessibility plan I mentioned in #3, so it shows up there.