Problem/Motivation

A small part of the chevron icon is visible when focusing on an element in forced colour mode. See the attached screenshot.

Steps to reproduce

  1. Set claro as an admin theme
  2. Enable forced colour mode
  3. Check any accordion element for example - go to /admin/config/system/site-information
  4. Check the element on focus mode and you will see the issue

Proposed resolution

Need to unset the background image in forced colour mode

Remaining tasks

User interface changes

Before

before

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 afterpatch_6238.png78.82 KBkanchan bhogade
#11 beforepatch_6238.png77.58 KBkanchan bhogade
#5 Patch error.png330.3 KBsandeep_k
#4 after-MR.png513.42 KBshweta__sharma
Screenshot at Jan 19 13-14-59.png514.62 KBshweta__sharma

Issue fork drupal-3415776

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:

About issue forks

Comments

shweta__sharma created an issue. See original summary.

shweta__sharma’s picture

shweta__sharma commented
Issue summary: View changes

shweta__sharma opened merge request !6238

shweta__sharma’s picture

shweta__sharma commented
Status: Active » Needs review
StatusFileSize
new after-MR.png513.42 KB

Pushed the code kindly review it.
Thanks

sandeep_k’s picture

sandeep_k commented
StatusFileSize
new Patch error.png330.3 KB

@shweta__sharma, I've tested the shared MR- MR !6238 mergeable I was able to reproduce the issue but while applying the patch, I was getting an error & patch didn't apply. Error attached.

smustgrave’s picture

smustgrave commented
Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community

Seems like a small change and confirmed the issue.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Needs work

Let's not resort to !important unless it's absolutely necessary.

A better fix would be to override the exact same selector (

  .claro-details[open]>.claro-details__summary:focus::before,

) within the forced-colors media query. You could add this on line 292.

Note that the details.pcss.css file needs quite a bit of refactoring at some point. It doesn't make use of modern CSS, or nesting like many other Claro stylesheets do. But in the meantime, let's keep the stylesheet consistent and keep the same patterns.

shweta__sharma’s picture

shweta__sharma commented
Status: Needs work » Needs review

Changes are updated as per #7

Thanks

Mithun S made their first commit to this issue’s fork.

mithun s’s picture

mithun s
Primary language English
Location Bangalore
commented

Removed the unnecessary trailing comma in the css files and updated the PR with a commit.

kanchan bhogade’s picture

kanchan bhogade commented
StatusFileSize
new beforepatch_6238.png77.58 KB
new afterpatch_6238.png78.82 KB

Hi
I've tested the shared MR- MR !6238 mergeable I was able to reproduce the issue and the issue is fixed with the Patch.

Attaching Screenshot

RTBC+1

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks @mhercel

Appears suggestions was implemented.

  • nod_ committed 331141c7 on 11.x 
    Issue #3415776 by shweta__sharma, Mithun S, Kanchan Bhogade, smustgrave...

nod_ closed merge request !6238

  • nod_ committed a86c8268 on 10.3.x 
    Issue #3415776 by shweta__sharma, Mithun S, Kanchan Bhogade, smustgrave...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 331141c and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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