Automattic / themes

Free WordPress themes made by Automattic for WordPress.org and WordPress.com.
https://themeshaper.com
GNU General Public License v2.0
861 stars 348 forks source link

Penscratch 2: The code for SVG icons are loaded even if they are not being used anywhere #5112

Open devNigel opened 2 years ago

devNigel commented 2 years ago

Quick summary

The user was trying to remove any unnecessary HTML codes loaded on their website. They noticed over 15 lines of definitions for SVG icons were loaded on the site despite not using them anywhere.

The SVG definitions are loaded only on Atomic sites.

image

Steps to reproduce

  1. On an Atomic site, activate the theme Penscratch 2.
  2. Check the page source code.
  3. You will notice SVG definitions are loaded even if those icons are not used anywhere.

What you expected to happen

The theme should not load unnecessary cods.

What actually happened

The theme loaded HTML codes for SVG icons that are not used anywhere on the site.

Context

Chat: 32285375-hc

Operating System

No response

Browser

No response

Simple, Atomic or both?

Atomic

Theme-specific issue?

Penscratch 2

Other notes

No response

Reproducibility

Consistent

Severity

One

Available workarounds?

No but the platform is still usable

Workaround details

No response

kosiew commented 2 years ago

Hi @devNigel ,

Thanks for reporting this and the detailed steps.

I checked this for you and the SVG icons are used for social links.

image

Closing this. Please reopen if you have additional context to add.

devNigel commented 2 years ago

Hi @kosiew, the bug / enhacement report was that those SVG codes need not be loaded if they are not being used on the site.

kosiew commented 2 years ago

@devNigel , Got it. Thanks for the additional information.

In this case, it is not a bug. It is a feature request for lazy loading of assets. Lazy loading is justified if the assets are "heavy".

In this case, the svgs are not heavy.

image

Did the Customer share any specific concern with the svgs?

kosiew commented 2 years ago

@devNigel ,

To elaborate further, adding code to implement lazy loading may add more weight to the theme than the SVG themselves. In addition, we can also prioritise our efforts to move to block based themes, instead of adding feature to older themes.

What do you think?

devNigel commented 2 years ago

@kosiew

I think you misunderstood. I am not referring to the SVG assets ( files with .svg extension ), but the SVG definition codes in the HTML ( see the screenshot of page source ) of the theme for the icons supported by the theme. Lazy loading does not apply to HTML codes.

To clarify further, the SVG definition is actual code for those icons. Those SVG definitions are not related to any .svg files seen in Network Inspector.

The user was weighing on the fact that over 200 lines of unnecessary HTML SVG codes were being loaded on each page load.

I think a basic conditional check to load only the required SVG definition codes might not be that heavy. With that said, it is not a high-priority case but I think it was a reasonable request by the user.

kosiew commented 2 years ago

@devNigel ,

The user was weighing on the fact that over 200 lines of unnecessary HTML SVG codes were being loaded on each page load.

I tested this on an AT site, there were at most 25 svg lines.

Details of test: Activated Penscratch 2 on AT site - https://wpstore101.wpcomstaging.com/ View page source Did a count of replacements of svg with SVG. Count returned 50 - <svg .... Since each pair is a line, there are at most 25 lines.

Can you share on the steps to reproduce the 200 lines?

devNigel commented 2 years ago

I can see it on this site too: https://wpstore101.wpcomstaging.com/

image

image

That's 164 lines of code.

kosiew commented 2 years ago

@devNigel ,

Thanks for the clarificaiton.

Triaging for developers' further investigation.

danieldudzic commented 2 years ago

This is a Jetpack issue. I was able to replicate it with other themes too.

Themes add support for SVG icons via:

// Add theme support for Social Menu.
    add_theme_support( 'jetpack-social-menu', 'svg' );

Ideally, Jetpack should only output SVG code for icons that are present.