If the active theme is a subtheme, and icons are defined in the base theme, then the SVG is not added to the page.

May be addressable via #3023153: Consider refactoring to use a twig template

CommentFileSizeAuthor
#6 twig_svg-base_theme-3107012-6.patch466 bytestaherpro
#2 twig_svg-base_theme-3107012-2.patch1.19 KBmalcomio

Comments

malcomio created an issue. See original summary.

malcomio’s picture

malcomio commented
Status: Active » Needs review
StatusFileSize
new twig_svg-base_theme-3107012-2.patch1.19 KB
aaron.ferris’s picture

aaron.ferris commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. Thanks.

  • aaron.ferris committed 673c219 on 8.x-1.x 
    Issue #3107012 by malcomio,aaron.ferris: Icons from base theme not added
aaron.ferris’s picture

aaron.ferris commented
Status: Reviewed & tested by the community » Fixed
taherpro’s picture

taherpro commented
Priority: Normal » Critical
Status: Fixed » Needs review
StatusFileSize
new twig_svg-base_theme-3107012-6.patch466 bytes

Thanks for reporting and supplying a patch @malcomio

The patch in #2 is incomplete.

The base svg sprite in base theme `base_theme/images/icons.svg ` gets replaced by the active sub theme svg sprite at `sub_theme/images/icons.svg` in line #21

I am attaching a new patch herein in which the icon span will be appended instead of replacing the icon sprite content on the page.

The patch literally consists on of one dot :D One of the reasons why I love PHP :)

  • aaron.ferris committed 3ae1082 on 8.x-1.x 
    #3107012 by aaron.ferris, malcomio, devtaher: Append sub theme icons...
aaron.ferris’s picture

aaron.ferris commented
Status: Needs review » Fixed

Makes sense to me, thanks for the contribution.

malcomio’s picture

malcomio commented
Status: Fixed » Needs work

I think the new patch will cause PHP notices because $icons isn't initialised before appending to it - we should initialise it as an empty string before entering the loop

taherpro’s picture

taherpro commented
Status: Needs work » Fixed

@malcomio Yes, That issue is fixed part of this issue in this commit by Aaron.

aaron.ferris’s picture

aaron.ferris commented

Yes thats already fixed.

Status: Fixed » Closed (fixed)

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