Remove async CSS pattern where it may introduce layout shifts #2270
Conversation
|
I double-checked how this affects the page performance and I agree that it is a net-positive and we should ship it. I created some test themes with the affected sections on the index. As you say, it brings up the CSS files earlier in the network waterfall and reduces CLS. It does seem to increase blocking time but maybe that is to be expected. These lighthouse reports are the clearest examples: Before After Related note: I wonder if our pattern of having many small CSS files is hurting us and we should put them in one medium-sized CSS file that can easily be cached. But that's out of scope for this PR. Let's wait for one of the other devs with more context in the codebase to review for whether there are any other CSS files that should/should not be loaded synchronously instead of async. |
sections/footer.liquid
Outdated
| {{ 'component-newsletter.css' | asset_url | stylesheet_tag }} | ||
| {{ 'component-list-menu.css' | asset_url | stylesheet_tag }} | ||
| {{ 'component-list-payment.css' | asset_url | stylesheet_tag }} | ||
| {{ 'component-list-social.css' | asset_url | stylesheet_tag }} | ||
| {{ 'component-rte.css' | asset_url | stylesheet_tag }} | ||
| {{ 'disclosure.css' | asset_url | stylesheet_tag }} |
There was a problem hiding this comment.
component-rte.css is now part of base.css so you might have to do a rebase and remove the calls for this CSS file 👍
Also here in the footer, shouldn't we stick to loading the asset as non blocking ? Since it's at the bottom of the page 🤔
There was a problem hiding this comment.
It depends, there are pages with short content where footer is visible right away on desktop. Example: https://theme-dawn-demo.myshopify.com/pages/shipping
There was a problem hiding this comment.
One last comment 😅
I see two spots that are untouched, featured-product.liquid with the component-deferred-media.css as well as the main-search.liquid page with the component-search.css file.
Should those be updated as well ?
main-search.liquid would see to make sense 🤔
not as sure for featured-product.liquid but since it's a section, it can be anywhere on a page, above and below the fold.
True, I overlooked them, I also tested |
Remove async CSS pattern where it may introduce layout shifts (Shopify#2270)
* shopify/main: Improve image sizes in the multicolumn section (Shopify#2349) Fix the Page section's width. (Shopify#2364) Update 12 translation files (Shopify#2366) Removing "my" from cart popup notification (Shopify#2353) [Cart.js] Fix fetch url so it's not hard coded (Shopify#2357) (Shopify#2365) Update 1 translation file (Shopify#2352) Default Follow on Shop to on [Header] Add localization selectors (Shopify#2258) Remove async CSS pattern where it may introduce layout shifts (Shopify#2270) Change rich text section heading to be of type inline_richtext, also moved rte styling into base.css (Shopify#2326) Add drawer menu desktop (Shopify#2195) Make header image preload and proper width (Shopify#2307)
PR Summary:
This PR removes the usage of asynchronous CSS pattern from stylesheets that may actually be required to render the initial screen. Having those stylesheets as not render blocking leads to unstyled content being shown and shifting when they are downloaded and applied.
Why are these changes introduced?
Marking some of the stylesheets as targeted for printing with
media="print"and then switching them to apply for all devices withonload="this.media='all'"is a common pattern that lets developers download additional CSS with low priority and without blocking the initial render. As a rule, this technique should only be applied for styles that do not influence the look of anything that users may see without interacting with the page.Unfortunately, it seems like this solution ended up in too many places when it comes to Dawn. Below you can see the experiment that I run using WebPageTest where I remove the pattern for some of the stylesheets.
You may notice the whole
Obsessive Attention. Intelligent Effort.shifting to the right in the control run because of asynchronouscomponent-rte.css. This is not the case after applying changes from this PR in the experiment run, improving Cumulative Layout Shift score by 0.065 (95%). This is especially important, because sites need to be below 0.1 to be considered as "Good" within this metric.What approach did you take?
I analysed all of the occurrences of this pattern and checked if a certain CSS is required to render anything that may be visible for users before they interact with the page.
Other considerations
Initially, I thought that the pattern would have to be removed from everywhere. This could slow down the initial render as pointed out by @siakaramalegos as the browser would have to download more files. Fortunately, there's only a bunch of stylesheets that needed adjusting. I also made multiple tests across different Dawn-based stores and all rendering metrics land within the margin of error.
Decision log
Visual impact on existing themes
Parts of the page that relied on styles from asynchronous stylesheets will no longer appear initially unstyled, especially on first visits and/or slower connections.
Testing steps/scenarios
Demo links
Checklist