In #2113911: Modal style update nod_ mentioned that the contrast on the modal header was too low.

This isn't a hard accessibility requirement because the text in the header has excellent contrast, so I think it's just a design discussion.

Let's discuss and try out some variations.

CommentFileSizeAuthor
#25 2226193-25.patch488 bytesBarisW
#24 Screen Shot 2014-09-23 at 6.02.00 PM.png21.29 KBmgifford
#24 2226193-23.patch501 bytesmgifford
#20 2226193-1.png61.18 KBdavidhernandez
#18 Image-Without-Patch.png33.7 KBmgifford
#18 Image-Without-Patch-Contrast.png35.05 KBmgifford
#18 Image-Without-Patch-Contrast.png35.05 KBmgifford
#18 Image-With-Patch-Contrast.png32.48 KBmgifford
#18 Image-With-Patch.png29.58 KBmgifford
#17 2226193-17.patch913 bytesmgifford
#8 2226193-8.png52.59 KBdamiankloip
#8 2226193.patch897 bytesdamiankloip
#2 blue_header.png33.85 KBvorvor
#2 blue_gradient_textshadow.png30.84 KBvorvor
#2 black_header.png35.85 KBvorvor
contrast_0.png36.71 KBLewisNyman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Issue summary: View changes
vorvor’s picture

vorvor CreditAttribution: vorvor commented
FileSize
black_header.png35.85 KB
blue_gradient_textshadow.png30.84 KB
blue_header.png33.85 KB

some variatons

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

I'd like the following things first:
1) Remove transparency
2) Change background to better match https://groups.drupal.org/files/20.modal-dialog.png

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

I agree totally with #3, I like the new look. The header just needs to be easier on my eyes :)

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Yeah it doesn't need a drastic change. It's just that on a large screen the background and header just blend too much and you just don't know what's what. Tweaking opacity should be enough.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

I would prefer a tweak, during development full opacity looked a bit ugly.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Yes, in my mind tweaking the header colour/opacity is all that's needed.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Active » Needs review
FileSize
2226193.patch897 bytes
2226193-8.png52.59 KB

For me, something like this does the trick and appeases my eyes :)

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Needs work

error: core/themes/seven/dialog.theme.css: No such file or directory

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Looks like #2113911: Modal style update got reverted?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Postponed

yeah but when it gets in again, same issue.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

ok, cool. Comments etc.. still welcome on the screenshot above I guess :)

Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

This looks acceptable to me, could you share a few bigger screens?

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Postponed » Active
Issue tags: +CSS, +frontend

Reopening now that modal is back in

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

8: 2226193.patch queued for re-testing.

Status: Active » Needs work

The last submitted patch, 8: 2226193.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
FileSize
2226193-17.patch913 bytes

files moved.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: +Accessibility, +color contrast
FileSize
Image-With-Patch.png29.58 KB
Image-With-Patch-Contrast.png32.48 KB
Image-Without-Patch-Contrast.png35.05 KB
Image-Without-Patch-Contrast.png35.05 KB
Image-Without-Patch.png33.7 KB

It's not easy to track down contrast issues with rgba scripts. This contrast-ratio is helpful. As is Chrome's Color Contrast Analyzer it looks like:

This is what it is without the patch:

and With patch:

Maybe I'm getting something wrong here though.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Needs work
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez commented
FileSize
2226193-1.png61.18 KB

Part of the problem is the transparency. In one of your screenshots the header is sitting on top of the ckeditor toolbar which has a darker color, so it is picking up part of that making the background actually come out darker. The modal background, with transparency, is also somewhere around #4C4C4C (76,76,76) which is darker than both 107s and 120s. Making it less transparent would affectively make it lighter, right? I'm attaching a contrast scan with the header completely transparent to show the difference.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: +Needs reroll
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: +dcamsa11y
Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

I am fine with choosing a background without transparency.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
FileSize
2226193-23.patch501 bytes
Screen Shot 2014-09-23 at 6.02.00 PM.png21.29 KB

In that case, why not just this.

Screenshot of modal dialog title

BarisW’s picture

BarisW CreditAttribution: BarisW commented
FileSize
2226193-25.patch488 bytes

Looks good to me, except that we don't need rgba if the alpha is 1. So here's one with the same effect. but uses background: #6b6b6b;

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Reviewed & tested by the community

Ya, I wondered about that too.. Think RGB is better, thanks for the re-roll. It's one line of CSS. I'm going to mark it RTBC.

BarisW’s picture

BarisW CreditAttribution: BarisW commented
Issue tags: +Amsterdam2014
Bojhan’s picture

Bojhan CreditAttribution: Bojhan commented

Visual RTBC too :)

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue tags: -Needs reroll

LewisNyman queued 25: 2226193-25.patch for re-testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed c09e314 and pushed to 8.0.x. Thanks!

  • alexpott committed c09e314 on 8.0.x 
    Issue #2226193 by mgifford, BarisW, damiankloip | LewisNyman: Discuss...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.