readthedocs.org icon indicating copy to clipboard operation
readthedocs.org copied to clipboard

Defer loading of badge_only.css?

Open mgeier opened this issue 2 years ago • 5 comments

Details

  • Read the Docs project URL: https://readthedocs.org/projects/insipid-sphinx-theme/

The RTD theme is not used, therefore badge_only.css is loaded.

Expected Result

Faster page load?

Actual Result

I don't know if that makes a meaningful difference, but the file https://assets.readthedocs.org/static/css/badge_only.css is loaded right at the beginning of the page load, even though the HTML elements it applies to are inserted into the page much later. This seems like a waste, doesn't it?

I've played around with https://pagespeed.web.dev/, which reports potential savings of 780 ms (maybe to be taken with a grain of salt, but anyway).

I'm not at all an expert at any of this, but I've read that the CSS can be deferred with something like this:

<link rel="preload" href="my_style.css" as="style" onload="this.onload=null;this.rel='stylesheet'">
<noscript><link rel="stylesheet" href="my_style.css"></noscript>

The <noscript> fallback is probably not even necessary, since the "badge" itself is loaded with JavaScript.

Has something like this been considered before?

mgeier avatar Apr 22 '22 16:04 mgeier

For testing, I hacked together a version to try this: https://insipid-sphinx-theme.readthedocs.io/en/temp-defer-css/

TBH, I can't tell if that improves anything.

mgeier avatar Apr 22 '22 19:04 mgeier

Hi! Thanks for reporting this. I'm not an expert in this field either so, I'm pinging @agjohnson and @davidfischer so they can chime in.

humitos avatar Apr 25 '22 07:04 humitos

I tried this for my own custom CSS overrides (https://github.com/mgeier/insipid-sphinx-theme/pull/89) and it seems to work fine on Firefox and Chromium.

Any comments from the experts?

mgeier avatar May 22 '22 16:05 mgeier

This could/should of course also be applied to https://assets.readthedocs.org/static/css/readthedocs-doc-embed.css

mgeier avatar Jun 04 '22 16:06 mgeier

It might be complicated to remove FontAwesome from badge_only.css (see #9297), but I think trying to defer loading it would already help creating a more responsive experience.

Is anything speaking against that?

mgeier avatar Aug 05 '22 07:08 mgeier

This badge_only.css is added by our custom Sphinx extension at https://github.com/rtfd/readthedocs-sphinx-ext/blob/83cf6ecae26d787c65ab848a7414f2b30f8c7299/readthedocs_ext/readthedocs.py#L80-L95

As I mentioned in another issue (#8323), we are working on a complete re-design of the flyout as part of a new project called addons (see https://blog.readthedocs.com/addons-flyout-menu-beta/)

We are testing "removing all these old files on the fly when serving the page" and opt-in into the new addons. Would you like to test this behavior on your insipid-sphinx-theme project and share some feedback about this?

humitos avatar Sep 15 '23 15:09 humitos

We are testing "removing all these old files on the fly when serving the page" and opt-in into the new addons. Would you like to test this behavior on your insipid-sphinx-theme project and share some feedback about this?

@mgeier this is already deployed. You can enable it from the new beta dashboard (see the comment on https://github.com/readthedocs/readthedocs.org/issues/8497#issuecomment-1798264710). Please, let me know if this works for you.

humitos avatar Nov 07 '23 15:11 humitos

Sorry for my late response!

I didn't activate the addons in my real project (because it breaks my custom flyout placement), but I created a dummy copy for experimentation: https://just-testing-the-insipid-sphinx-theme.readthedocs.io/.

I have run the pagespeed test again (https://pagespeed.web.dev/analysis/https-just-testing-the-insipid-sphinx-theme-readthedocs-io-en-latest-showcase-tables-html/q75ory8lmr?form_factor=mobile) and it doesn't complain about badge_only.css anymore, which I guess is good!

However, the CSS file still seems to be loaded the same way (e.g. https://just-testing-the-insipid-sphinx-theme.readthedocs.io/en/latest/index.html):

<link rel="stylesheet" type="text/css" href="/_/static/css/badge_only.css" />

mgeier avatar Jan 11 '24 19:01 mgeier

I didn't activate the addons in my real project (because it breaks my custom flyout placement)

Would you help me to get some feedback about the integration I've been working on for the new addons at https://github.com/readthedocs/addons/pull/64? That will help theme authors to integrate the Read the Docs data (flyout included) into the documentation in a nice and customized way. I created an example in the Read the Docs Theme at https://github.com/readthedocs/sphinx_rtd_theme/pull/1526?

humitos avatar Jan 15 '24 11:01 humitos

However, the CSS file still seems to be loaded the same way (e.g. just-testing-the-insipid-sphinx-theme.readthedocs.io/en/latest/index.html):

Gotcha! Thanks for testing this out. It seems that I missed this URL when removing old stuffs. I added this file to the list of removals and deployed the fix. I tested your dummy copy and it doesn't load the badge_only.css anymore there. I'm closing this issue, but feel free to re-open if you consider that's not solved.

humitos avatar Jan 15 '24 11:01 humitos