Problem/Motivation

In the Olivero theme, color for the chevron icon is not as per the Figma design for all the screen sizes.
Before the patch
mobile before patch
tablet before patch
desktop before patch
After the patch
mobile after patch
tablet after patch
desktop after patch

Steps to reproduce

Make sure you have selected the Olivero theme and open it using Chrome or a similar browser inspect the chevron icon color and compare it to the Olivero theme Figma design

Proposed resolution

Replace the color with the one that is mentioned in the Figma design.

Remaining tasks

None

User interface changes

Changing the color from var(--color--gray-45) to var(--color--gray-90) as mentioned in the Figma design.

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#13 Screenshot 2023-07-17 at 5.01.01 PM.png78.67 KBsmustgrave
#9 after-patch.png150.49 KBlokeshsahu
#9 before-patch.png118.22 KBlokeshsahu
#9 patch-applied-successfully.png70.01 KBlokeshsahu
#6 Screenshot from 2023-06-28 17-16-04.png87.4 KBshiv_sharma
#6 Screenshot from 2023-06-28 17-35-14.png113.73 KBshiv_sharma
#5 Tablet-After-patch-3370131-4.png124.26 KBManibharathi E R
#5 Mobile-After-patch-3370131-4.png60.16 KBManibharathi E R
#5 Desktop-after-patch-3370131-4.png194.39 KBManibharathi E R
#5 Dersktop-Before-patch-3370131-4.png164.67 KBManibharathi E R
#5 Mobile-before-patch-3370131-4.png21.38 KBManibharathi E R
#5 Tablet-before-patch-3370131-4.png124.7 KBManibharathi E R
#3 desktop-after-3370131.png225.34 KBhamid.ali
#3 tablet-after-3370131.png130.15 KBhamid.ali
#3 mobile-after-3370131.png85.34 KBhamid.ali
#3 desktop-before-3370131.png232.24 KBhamid.ali
#3 tablet-before-3370131.png139.86 KBhamid.ali
#3 mobile-before-3370131.png84.45 KBhamid.ali
#3 3370131-3.patch1.24 KBhamid.ali

Issue fork drupal-3370131

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

Hamid.ali created an issue. See original summary.

hamid.ali’s picture

hamid.ali commented

Working on the issue will provide updated patch to fix the issue

hamid.ali’s picture

hamid.ali commented
Issue summary: View changes
StatusFileSize
new 3370131-3.patch1.24 KB
new mobile-before-3370131.png84.45 KB
new tablet-before-3370131.png139.86 KB
new desktop-before-3370131.png232.24 KB
new mobile-after-3370131.png85.34 KB
new tablet-after-3370131.png130.15 KB
new desktop-after-3370131.png225.34 KB

Have attached the updated patch to fix the issue.

hamid.ali’s picture

hamid.ali commented
Assigned: hamid.ali » Unassigned
Status: Active » Needs review

Kindly review the issue.

Manibharathi E R’s picture

Manibharathi E R commented
StatusFileSize
new Tablet-before-patch-3370131-4.png124.7 KB
new Mobile-before-patch-3370131-4.png21.38 KB
new Dersktop-Before-patch-3370131-4.png164.67 KB
new Desktop-after-patch-3370131-4.png194.39 KB
new Mobile-After-patch-3370131-4.png60.16 KB
new Tablet-After-patch-3370131-4.png124.26 KB

Patch #3 applied and verified successfully on Drupal 10.

shiv_sharma’s picture

shiv_sharma commented
StatusFileSize
new Screenshot from 2023-06-28 17-35-14.png113.73 KB
new Screenshot from 2023-06-28 17-16-04.png87.4 KB

I have reviewed the figma Design Figma Design with the patch color code, both are same.
screenshot
applied patch #3 successfully on Drupal 10.

shweta__sharma’s picture

shweta__sharma commented

Hi @Hamid.ali,
I have applied your patch but it's throwing an error.

Checking patch core/themes/olivero/css/components/breadcrumb.css...
error: while searching for:
  margin-inline: 1rem 1.25rem;
  content: "";
  transform: rotate(45deg); /* LTR */
  border-block-start: 2px solid var(--color--gray-45);
  border-inline-end: 2px solid var(--color--gray-45);
}

.breadcrumb__item:last-child {

error: patch failed: core/themes/olivero/css/components/breadcrumb.css:98
error: core/themes/olivero/css/components/breadcrumb.css: patch does not apply
Checking patch core/themes/olivero/css/components/breadcrumb.pcss.css...
error: while searching for:
    margin-inline: 16px 20px;
    content: "";
    transform: rotate(45deg); /* LTR */
    border-block-start: 2px solid var(--color--gray-45);
    border-inline-end: 2px solid var(--color--gray-45);
  }

  &:last-child {

error: patch failed: core/themes/olivero/css/components/breadcrumb.pcss.css:83
error: core/themes/olivero/css/components/breadcrumb.pcss.css: patch does not apply
lokeshsahu’s picture

lokeshsahu commented

Reviewing this issue

lokeshsahu’s picture

lokeshsahu commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new patch-applied-successfully.png70.01 KB
new before-patch.png118.22 KB
new after-patch.png150.49 KB

I have tested and verified patch #3 on Drupal-10.1.x-dev and Drupal-11.x. The patch was applied successfully without throwing any error and it's fixing the above-addressed issue. I am attaching a screenshot for reference, showing the state before and after applying the patch.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 3370131-3.patch, failed testing. View results

Shiv_Sharma opened merge request !4306

shiv_sharma’s picture

shiv_sharma commented
Status: Needs work » Needs review

I used @Hamid.ali code to create the merge request.

smustgrave’s picture

smustgrave commented
Status: Needs review » Postponed (maintainer needs more info)
Issue tags: +Needs subsystem maintainer review
StatusFileSize
new Screenshot 2023-07-17 at 5.01.01 PM.png78.67 KB

failure

So for me this is a won't fix. The proposed change introduce accessibility issue with poor color contrast.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Postponed (maintainer needs more info) » Closed (outdated)
Issue tags: +Bug Smash Initiative

Since hasn't been a follow up going to close out. If still an issue please reopen

quietone closed merge request !4306