Closed (fixed)
Project:
Drupal core
Version:
9.4.x-dev
Component:
Claro theme
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
23 Mar 2022 at 19:57 UTC
Updated:
3 May 2022 at 14:59 UTC
Jump to comment: Most recent, Most recent file

Comments
Comment #2
m4oliveiTesting on the Drupal 10.0.x branch.
I'm not able to reproduce that. Using Windows 11, Edge 99, Night Sky Contrast Theme.
I got the same result on Windows 10, Edge 99, High Contrast On, High Contrast Black Theme.
Comment #3
mherchelThats weird. That seems to match my environment exactly.
I'm using Parallels for my VM environment, but I can't imagine that affecting anything.
Can anyone else reproduce this?
Comment #4
mherchelI was able to reproduce on my personal computer (other gif was from work computer) using Windows 7, and Edge 99 (this time using VirtualBox as the VM).
Comment #5
m4olivei@mherchel What is that Tugboat environment your looking at? Can you share the URL? Would make it easier for me to look on some other machines. I suppose I could type it out from your gif, but I'm way too lazy for that.
Comment #6
m4oliveiFor the record I was using VirtualBox on PopOS, but I agree that shouldn't matter.
Comment #7
mherchelyeah, I'm looking at the demo Olivero at lb.cm/olivero. Will DM you the credentials
Comment #8
m4oliveiWell I feel silly. I was looking at Seven admin theme, not Claro. Sorry about that. I can reproduce it now!
Comment #10
m4oliveiSeems pretty simple in the end. The
.form-booleanclass (covers checkboxes and radios) had a default background image of a checkbox. When your looking at Claro without high contrast mode, the background of the checkbox / radio is white, which matches the fill of the SVG checkbox, which is why even though it was there, you couldn't see it. Until you browse in high contrast.Just removing it seems to fix it, and the checked states are correctly covered, so I think that's all that is needed. See MR.
Comment #11
mherchelThis definitely improves the situation. That background image should not be there.
However, we still need work here because the color of the checkbox/circle doesn't adjust to the high contrast theme. In the picture below (with a black background), the selected "dot" has a contrast ratio of 2.35:1. 3:1 is the minimum for this type of things, but I'd argue that this should be even higher since the user obviously prefers (and needs) high contrast to operate.
I would suggest using a combination of the forced colors media query, and
appearance: revertto revert the booleans back to browser defaults when in high contrast. This would let the browser handle the selected "dot" and checkbox, and that would have decent contrast irregardless of the user's high contrast theme.for Drupal 9.4.x, you'd want to support IE, so it'd look something like
Comment #12
mherchelComment #13
rafuel92 commentedComment #14
m4olivei@rafuel92 presume you're working on @mherchel's feedback? Otherwise I have time this week to finish up. lmk.
Comment #15
rafuel92 commentedSure, @m4olivei, i'm actually working on it at DrupalDevDays in Ghent
Comment #16
rafuel92 commentedComment #17
rafuel92 commentedchanges are applied, setting the issue to needs review
Comment #18
mherchelThanks for the work @rafuel92!
There are a couple issues:
1) You need to compile and commit the CSS (this is the reason why the patch says "Custom Commands Failed")

2) The radio buttons are unaffected by this change
3) No need for the
forced-colors: activemedia query when settingappearance: autoComment #19
m4oliveiLooks like @rafuel92 made some changes since the last feedback but maybe not all required changes. I'm going to take a look and see if there is anything I can help with today.
Comment #20
m4oliveiforced-colors: activemedia query per Mike's comment. Also, similar to above, I couldn't find any examples of it's use together with-ms-high-contrastHere is what I see now:
Comment #21
cindytwilliams commentedI was able to duplicate the problem in Drupal 10 on Edge 99:

Applying @m4olivei's latest changes resulted in this:

I also checked the changes in non-high-contrast mode and don't see any regressions. Marking RTBC.
Comment #22
lauriiiComment #23
lauriiiMoved back to needs work – if we implement the feedback from #11, it should address my feedback.
Comment #24
andy-blumComment #25
andy-blumComment #26
mherchelThis should be
forced-colors(note the plural).D'oh! I just noticed my comment under #23 had it wrong there too! Sorry! 🤦♂️🤦♂️🤦♂️😆
Comment #28
yogeshmpawarAddressed #26
Comment #29
mherchelThis is close, and this works as expected. But, we need to remove the IE11 specific media query for Drupal 10. The Drupal 9 version should have that.
Comment #30
m4oliveiWill update based on #29.
To be clear, we first want to remove IE specific things in the open merge request, which is against 10.0.x (will do that now).
After that are we wanting to switch target to 9.4.x and add back IE11 specific things?
Comment #31
m4oliveiRemoved
Comment #32
mherchelThe latest changes in #31 look perfect!
Screenshots attached. A couple notes:
Edge regular
Firefox regular
Safari regular
Firefox high contrast
Edge high contrast
Comment #33
mherchelThe MR above is for 10.0.x. Attached is a patch for 9.4.x with the same fix, but adding on the older version of the media query that supports IE11.
Comment #34
mherchelUpdated 9.4.x patch to remove the unnecessary background image.
Comment #37
lauriiiCommitted 91a01c8 and pushed to 10.0.x. Also committed patch from #34 to 9.4.x. Thanks!