Claro's checkboxes and radio buttons are broken when in high contrast.

This gif is in Windows 11 using MS Edge v99

Issue fork drupal-3271305

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mherchel created an issue. See original summary.

m4olivei’s picture

StatusFileSize
new516.34 KB

Testing 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.

mherchel’s picture

Thats 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?

mherchel’s picture

StatusFileSize
new452.75 KB

I 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).

m4olivei’s picture

@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.

m4olivei’s picture

For the record I was using VirtualBox on PopOS, but I agree that shouldn't matter.

mherchel’s picture

yeah, I'm looking at the demo Olivero at lb.cm/olivero. Will DM you the credentials

m4olivei’s picture

Well I feel silly. I was looking at Seven admin theme, not Claro. Sorry about that. I can reproduce it now!

m4olivei’s picture

Status: Active » Needs review

Seems pretty simple in the end. The .form-boolean class (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.

mherchel’s picture

Status: Needs review » Needs work

This 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: revert to 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.

@media (forced-colors: active) {
  .form-boolean {
    appearance: revert;
    background: none;
  }
}

for Drupal 9.4.x, you'd want to support IE, so it'd look something like

@media (-ms-high-contrast: active), (forced-colors: active) {
  .form-boolean {
    appearance: auto; /*test this in IE*/
    background: none;
  }
}
mherchel’s picture

StatusFileSize
new44.75 KB

rafuel92’s picture

Assigned: Unassigned » rafuel92
m4olivei’s picture

@rafuel92 presume you're working on @mherchel's feedback? Otherwise I have time this week to finish up. lmk.

rafuel92’s picture

Sure, @m4olivei, i'm actually working on it at DrupalDevDays in Ghent

rafuel92’s picture

Assigned: rafuel92 » Unassigned
Status: Needs work » Needs review
rafuel92’s picture

changes are applied, setting the issue to needs review

mherchel’s picture

Status: Needs review » Needs work
StatusFileSize
new40.15 KB

Thanks 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: active media query when setting appearance: auto

m4olivei’s picture

Assigned: Unassigned » m4olivei

Looks 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.

m4olivei’s picture

Assigned: m4olivei » Unassigned
Status: Needs work » Needs review
StatusFileSize
new12.89 KB
  • I adjusted the media query to match all other examples of high contrast media queries in Claro
  • I removed the forced-colors: active media query per Mike's comment. Also, similar to above, I couldn't find any examples of it's use together with -ms-high-contrast

Here is what I see now:

cindytwilliams’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new5.26 MB
new4.19 MB

I 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.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work
lauriii’s picture

Moved back to needs work – if we implement the feedback from #11, it should address my feedback.

andy-blum’s picture

Assigned: Unassigned » andy-blum
andy-blum’s picture

Assigned: andy-blum » Unassigned
Status: Needs work » Needs review
mherchel’s picture

Status: Needs review » Needs work
 themes/claro/css/components/form--checkbox-radio.pcss.css
 75:49  ✖  Unexpected unknown media feature name "forced-color"  media-feature-name-no-unknown
 

This should be forced-colors (note the plural).

D'oh! I just noticed my comment under #23 had it wrong there too! Sorry! 🤦‍♂️🤦‍♂️🤦‍♂️😆

yogeshmpawar made their first commit to this issue’s fork.

yogeshmpawar’s picture

Status: Needs work » Needs review

Addressed #26

mherchel’s picture

Status: Needs review » Needs work

This 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.

m4olivei’s picture

Assigned: Unassigned » m4olivei

Will 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?

m4olivei’s picture

Assigned: m4olivei » Unassigned
Status: Needs work » Needs review

Removed

mherchel’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new37.38 KB
new30.65 KB
new50.36 KB
new15.62 KB
new23.64 KB

The latest changes in #31 look perfect!

Screenshots attached. A couple notes:

  1. I'm including some "regular" (non high contrast) browser screenshots to show that there are no regressions
  2. The blue color of the checkboxes/radios in Firefox high contrast is hard coded and specific to that browser. I verified this on a codepen with no styling that showed a radio and checkbox

Edge regular

Firefox regular

Safari regular

Firefox high contrast

Edge high contrast

mherchel’s picture

StatusFileSize
new1.13 KB

The 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.

mherchel’s picture

StatusFileSize
new1.87 KB

Updated 9.4.x patch to remove the unnecessary background image.

  • lauriii committed fe58434 on 10.0.x
    Issue #3271305 by m4olivei, mherchel, rafuel92, yogeshmpawar, andy-blum...
lauriii’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 91a01c8 and pushed to 10.0.x. Also committed patch from #34 to 9.4.x. Thanks!

  • lauriii committed f892bac on 9.4.x
    Issue #3271305 by m4olivei, mherchel, rafuel92, yogeshmpawar, andy-blum...

Status: Fixed » Closed (fixed)

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