MWPW-149124 Improve Focus Page for Performance Improvement Tiger Team

[TsayAdobe]

• Improve LCP for a marquee with a modal video
• This PR only improves loading of icons .css and .svg.
• A more general improvement for blocks is proposed in this discussion. It can improve LCP more.

Resolves: MWPW-149124

Test URLs:

• Before: https://main--milo--adobecom.hlx.page/?martech=off
• After: https://MWPW-149124--milo--adobecom.hlx.page/?martech=off

DC Test URLs:

• Before: https://www.stage.adobe.com/acrobat/how-to/pdf-editor-pdf-files.html
• After: https://www.stage.adobe.com/acrobat/how-to/pdf-editor-pdf-files.html?milolibs=MWPW-149124

Before

After

[aem-code-sync]

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

• Re-run PSI checks
• Re-sync branch

Commits

• ce3f8ea ✅ (latest)
• 28f5656 ✅
• ef243e0 ✅
• 6de758b ✅

[aem-code-sync]

Page	Scores	Audits	Google
/?martech=off

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.86%. Comparing base (0428594) to head (ce3f8ea).
Report is 39 commits behind head on stage.

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #2391      +/-   ##
==========================================
+ Coverage   95.74%   95.86%   +0.12%     
==========================================
  Files         175      175              
  Lines       45876    46062     +186     
==========================================
+ Hits        43923    44157     +234     
+ Misses       1953     1905      -48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

[github-actions]

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

[auniverseaway]

Can we get better test URLs?

stage.adobe.com cannot be tested and the Milo homepage is not a valid test as there are no icons in it.

[auniverseaway]

Something to also note: your performance screenshots appear to be showing desktop experiences. It's really important when sharing performance related data that we are sharing mobile details and to be transparent about the ways in which we achieved certain numbers. I may test on 3G mobile, but you may test on 4G desktop, etc.

[auniverseaway]

Improve LCP for a marquee with a modal video

This is unfortunately not correct. This PR only impacts icons. If you link to a video modal with an icon button, sure... this will improve the LCP for that marquee. But we should not mis-represent what this PR is doing.

[auniverseaway]

I ended up using

https://main--dc--adobecom.hlx.live/acrobat/how-to/pdf-editor-pdf-files
VS
https://main--dc--adobecom.hlx.live/acrobat/how-to/pdf-editor-pdf-files?milolibs=MWPW-149124

Here are the deep PSI numbers for Mobile 4G:

[auniverseaway]

I think this is not currently a meaningful enough change to warrant the complexity and three extra lines in utils.

I do think you have a path to getting an improvement to icons, but will need some more tweaks in this PR.

1. 715: This works around a file called helpers.js that should be stripped out of icons altogether in favor of regular fetch. There is no need to have this dependency as it is only for cache busting content in specific situations for GWP. Icons does not fall into this.
   1. AI: remove the helpers.js preload
   2. AI: remove the use of helpers.js in icons.js
2. AI: Replace 711 & 712 with const { base } = config; on 711. This gains you a line.
3. 713 = Good.
4. 714 = probably needs to go. We don't want to preload icons if we can do something better. More on this below.

Better: we want to make icons as non-blocking as possible. Right now, everything is awaiting on icons. We await decorateIcons, we await loadIcons, we await the import.

icons.js unfortunately has critical parts to prevent CLS. there's styling and behavior in there that we do have to await for. However, we don't have to wait for the actual SVG to load. You'll notice at the beginning of the default function in icons.js, we await for the SVG to be fetched. This is wrong. We do not have to await that file. We can still kick it off, but there's no reason for the function to wait for that. We don't really care about the contents and it's not a dependency for the other work.

What will happen in practice is that everything will be decorated, icons.js will not block, and the rest of the milo machine can continue while the svg is downloaded. When the svg is downloaded, we simply inject it into the slots determined through the rest of the function.

My guess is the improvement is still tiny (or non-existent, or worse.. who knows), but utils can stay lighter while we also move in a more sustainable direction of lazily loading icons. I'd rather us try this (I see it as a 4 hour task) to help us make an informed decision going forward.

I'm happy to jump on a call if you want to walk through this. I know digesting a wall of text can be a challenge. LMK.

CC @ryanmparrish

[chrischrischris]

This is not directly related, but looking at icon.css the css in there only applies to tooltips. Thoughts on only loading icon.css when icon-tooltip is present? The only issue I can see is if other icon things are added to the file then that condition would have to be updated or removed. We could put a comment into icon.css as a warning to future devs.

[chrischrischris]

One other possible improvement: Put a dummy element in the icon location that matches the icon size, then do not await any of the icon code and let it lazy load. This may affect CLS but the tradeoff may be worth it for an improvement to LCP.

[chrischrischris]

One thing that I've noticed is that increasing the amount of concurrent downloads is more advantageous on desktop vs mobile due to the limited bandwidth on mobile. That may explain the numbers that @auniverseaway is seeing. As long as there isn't a negative impact to perf on mobile then I think it's still worthwhile doing, but it may not be as much of a perf win than expected.

[TsayAdobe]

I ended up using

https://main--dc--adobecom.hlx.live/acrobat/how-to/pdf-editor-pdf-files VS https://main--dc--adobecom.hlx.live/acrobat/how-to/pdf-editor-pdf-files?milolibs=MWPW-149124

Here are the deep PSI numbers for Mobile 4G:

I run two tests to see the measurement error of the tool.

A difference is considered significant only if it is larger than the possible error in measurement.

[github-actions]

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

Not responding to my code review request

[github-actions]

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.