It appears the CSS has been copied over from the Drupal 7 module. In D8, the Seven admin theme has slightly different input & page background colors, which makes the contrast look off.
Currently: https://i.imgur.com/UK9OP74.png
Expected: https://i.imgur.com/jnEsDwA.png
Decide if this needs to be done. See #11 and #12
| Comment | File | Size | Author |
|---|---|---|---|
| #36 | after-patch.png | 71.73 KB | Madhu Kumar M E |
| #32 | After Patch 2756483 up.png | 373.44 KB | chetanbharambe |
| #32 | Before Patch 2756483 Up.png | 367.11 KB | chetanbharambe |
| #26 | After-patch.png | 49.18 KB | bhumikavarshney |
| #26 | before-patch.png | 43.09 KB | bhumikavarshney |
Comments
Comment #2
Algeron commentedProposed patch in attachment
Comment #3
Algeron commentedComment #4
akhil.desai commentedI have tested the above patch #2 and its working fine for me
the screenshot before applying the patch
the screenshot after applying the patch
Comment #6
akhil.desai commentedchecked patch #2 and it is working fine for me
before applying the patch
after applying the patch
Comment #7
manjit.singhComment #8
xjmAssigning frontend issues to Cottser for potential review. Thanks!
Comment #9
akhil.desai commentedComment #10
xjmUnless the current contrast is insufficient for accessibility (etc.) this is a task. As a design chance this would have a beta deadline.
Comment #11
star-szrAt least if we go based on the Seven Style Guide the existing colour seems correct.
https://groups.drupal.org/node/283223#Color
eggshell:
#fcfcfa"used for: text fields, UI panel surfaces (fieldsets, etc.)"
I think this would fall under that UI panel surface definition.
See also https://groups.drupal.org/node/283223#Vertical_Tabs
Not to say the Seven Style Guide is set in stone but it seems like this has definitely been thought of. Might need more motivation here as to why this should be changed.
Comment #12
tim.plunkettWhat is the contrast ratio before and after?
See https://www.drupal.org/node/464500 for our requirement.
That link also provides tools for checking the contrast.
Comment #13
star-szrYup +1 to checking for contrast, thanks @tim.plunkett.
Comment #21
xjmComment #23
janmejaig commentedI am able to applied the patch for the Drupal 9.1.x , this seems to be working fine for me , this can be move to RTBC .
Comment #25
vikashsoni commentedI have apply the patch #2 and its working fine for me
sharing the screenshot ...
Comment #26
bhumikavarshney commentedI am able to applied the patch for the Drupal 9.2.x , this seems to be working fine for me.
RTBC+1
Comment #27
mgiffordThat will help the color contrast for sure. Just needs the issue summary update.
Comment #28
Poojita0802 commentedComment #29
Poojita0802 commentedComment #31
chetanbharambe commentedVerified and tested patch #2.
Patch applied successfully and looks good to me.
Testing Steps:
# Goto: Appearance -> Apply Seven theme
# Goto: Any Block and Configure that block
# Click on any vertical tabs and observe the background-color
Expected Results:
# After applying Patch #2, [Vertical tabs] Background color is matching with Seven admin theme.
Actual Results:
# Currently, [Vertical tabs] Background color is not optimal for the Seven admin theme.
Please refer attached screenshots for the same
Looks good to me.
Can be a move to RTBC
Comment #32
chetanbharambe commentedForgot to attach screenshots in Comment #31
Comment #33
quietone commentedI have read the issue and there is still work to do here.
In #11 Cottser questioned if this change was needed at all. In #12 a review of the contrast was asked for as well. Neither of these comment have been addressed by the following work, which are mostly screenshots.
This was tagged for an Issue Summary update, which still needs to be done.
Setting to NR to get a decision on whether this change should be implemented or not. Adding this to the IS as the only remaining task.
@janmejaig, @vikashsoni, @BhumikaVarshney, @chetanbharambe thanks for the interest in helping with this issue. It is good practice to read all the comments before working on an issue.In this case testing and screenshots are not needed. Previous comments are questioning if this work needs to be done at all. That needs to be resolved first. And there is no need to add another set of screenshots based on the same patch. Therefor, credit has been removed per How is credit granted for Drupal core issues.
Comment #36
Madhu Kumar M E commentedI applied patch #2 it working fine, it looks good if we apply some dark colour or we can increase the opacity, as of now there is no such big difference after applying patch.
Comment #37
bnjmnmPre-emptive credit removal for @vikashsoni as the screenshots in #25 are renamed copies of the same shots uploaded in #23 (which as mentioned in #33 aren't needed at this stage anyway.
Comment #38
Harish1688 commentedTested the patch (2756483-vertical_tab_bg_color.patch) working fine, looks good for RTBC.
Comment #39
bnjmnmIt's already been confirmed in #12 this is a correct implementation of the Seven style guide.
To confirm the style guide was not implementing something with insufficient contrast, I did a contrast check on vertical tabs.
Within active tabs, the lowest content/background contrast ration is 6.81:1 which is well above the 4.5:1 to meet AA requirements.
Within inactive tabs this gets a littler lower at 5.12:1, but still comfortably above the AA contrast requirement.
Given that this is the correct implementation of Seven designs, and it does not result in an accessibility bug, I'm going to mark this issue closed.