Problem/Motivation

Follow up from #3347693: Remove hardcoded CSS values, use css variables instead

In accordance with the ticket, there was an attempt to replace hardcoded values with variables.
However, when SVG images are used as background images, the variables do not function as expected. Consequently, a suitable solution approach is required to address this issue and enable the change of SVG background image colors through variables.

CommentFileSizeAuthor
#3 3357143 after implemented .png61.6 KBHarish1688

Comments

smustgrave created an issue. See original summary.

markconroy’s picture

Title: Discuss SVG for Olivero » Discuss SVG for Umami Theme
Harish1688’s picture

Status: Active » Needs review
StatusFileSize
new61.6 KB

Hi,

Based on the problem ticket, an approach has been devised to address the issue of setting the background color for SVG images using CSS.

1. The proposed solution ensures that the background color of all icons is not conflicting with each other. This can be achieved by utilizing CSS variables to modify the background image color.

file path : core/profiles/demo_umami/themes/umami/css/components/content-types/recipe/recipe.css

remove code this:

.node--type-recipe.node--view-mode-full .field--name-field-number-of-servings {
  background-image: url(../../../../images/svg/serves.svg);
}

replaces by this:

.node--type-recipe.node--view-mode-full .field--name-field-number-of-servings {
  position: relative;
}

.node--type-recipe.node--view-mode-full .field--name-field-number-of-servings::before {
   -webkit-mask: url(../../../../images/svg/serves.svg) no-repeat;
  mask: url(../../../../images/svg/serves.svg) no-repeat;
  background-color: var(--color-primary);
  position: absolute;
  content: "";
  width: 56px;
  height: 56px;
  top: 30%; 
  left: 50%;
  transform: translate(-50%,-50%);
}
aaron.ferris’s picture

Component: Olivero theme » Umami demo
smustgrave’s picture

Status: Needs review » Needs work

This needs an issue summary update before reviewing.

Thanks

Harish1688’s picture

Issue summary: View changes
Harish1688’s picture

Status: Needs work » Needs review

summary update, now moved to needs review.

mherchel’s picture

FYI, we're using the mask property in a similar way in Claro to make sure the icons support forced-colors mode.

The method above works in all of Drupal's supported browsers. If we do this, we can also add a forced-colors media query to make sure it works properly there. So if we have something like

.node--type-recipe.node--view-mode-full .field--name-field-number-of-servings::before {
   -webkit-mask: url(../../../../images/svg/serves.svg) no-repeat;
  mask: url(../../../../images/svg/serves.svg) no-repeat;
  background-color: var(--color-primary);
  position: absolute;
  content: "";
  width: 56px;
  height: 56px;
  top: 30%; 
  left: 50%;
  transform: translate(-50%,-50%);
}

We'd just need to add

@media (forced-colors: active) {
  .node--type-recipe.node--view-mode-full .field--name-field-number-of-servings::before {
    background-color: canvasText;
  }
}
markconroy’s picture

Status: Needs review » Needs work

Interesting approach @mherchel.

I'm not fully sure if we need to "fix" this issue since Umami has a set design and the colours are set in the SVGs. It only became an issue because one of the people working on the MR wanted to declare the CSS variables inside the SVG - which would mean the same variables declared in CSS and then multiple times in SVGs, which doesn't look very DRY to me. Instead, I decided we'd leave the Umami SVGs as they were and create this follow-up IF we needed it.

My preference would probably be to inline the SVGs instead of working with CSS workarounds, and then we could use whatever CSS we wanted to to add the colours for them.

Moving this to "Needs work" instead of "Needs review" since there is no actual code to review.

markconroy’s picture

Issue summary: View changes

Updating issue summary to remove

  1. Steps to reproduce - these steps do not reproduce the issue, since the issue is RTBC and the SVGs are not presenting a problem and not changed at all in that MR.
  2. Proposed resolutions - we have not agreed to proceed with the resolution in [#3]

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, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.