User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Defer the loading of Fontawesome in a Progressive enhancement way:
rel="lazy-stylesheet"
and once the page is loaded javascript changes it to stylesheet (forcing it to load)No, but hopefully theme creators and 3rd PD could apply the same simple optimisations so the Joomla ecosystem is really competitive and performance is a serious selling point!!!
FWIW Google will start using Core Web Vitals from May 2021: https://developers.google.com/search/blog/2020/11/timing-for-page-experience In short performance will affect the SEO ranking
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End Templates (site) NPM Change |
Labels |
Added:
NPM Resource Changed
?
|
@dgrammatiko can we please pass the script via the document api. Only this way hashbased whitelisting works within the plugin. And the nonce is added via the api too.
Thanks!
Sorry, but no.
This is a bad example for others, use of javascript to load css.
"non dev" people will think that "it is normal", and will use same for their template.css, because why not?
It not good.
This is a bad example for others, use of javascript to load css.
It's not ANY css, it's the icon font which turns out to be as big as the bootstrap css and on top of that could load over 200kb of fonts. And until all those are loaded/parsed and applied the page IS BLANK. With this way you get an immediate rendering whenever the template.css is downloaded. The difference in UX is huge for slower networks...
Also, it's not an anit-pattern, lazily loaded fonts is a thing for many years now and actually still considered a best practice...
PS I think I made it clear that this only applies to the font awesome:
It's not ANY css
Well, I understood this, but I am not talking about me or you.
I am about Joe Average, who will see it and think: "Oh I will do the same for my template.css, and my site will be faster"
I am about Joe Average, who will see it and think: "Oh I will do the same for my template.css, and my site will be faster"
Well, we can't fix stupidity but that shouldn't hold us back from applying obvious optimizations and have easy gains. If we can make the code more obvious that it shouldn't be used for layout/styling css let's do that
It like with children. If you show them something once, they will copy it and repeat without knowing a meaning
So better do not show it, until we have a better solution.
So better do not show it, until we have a better solution.
Hurting the product's perf ranking which in turn hurts the SEO ranking and UX is way worse than preventing uneducated people from doing stupid stuff. I mean a simple search google search will give you pages that also suggest such technics. It's not some kind of exclusive information/methodology that we are applying here...
btw, I just checked my DEV installation, and it already 100% without any changes, and only 149Kb requests
You showing me the file size, it not the same as request size
If you have gzip or brotli enabled. Assuming everybody does that is something I would like to support bu tI don't have that data to prove it.
And by this, the template.css still bigger, maybe we should defer it
No! I mean you can defer it but the first rendering will be an unstyled page which is not something that you want to happen
No! I mean you can defer it but ...
Do not take it that serious, that was a joke
If you have gzip or brotli enabled.
It default apache installation.
So, that mean you can win much more from a proper server configuration than from this hacky code
@Fedik fwiw the root of the problem is that the CMS is using fonts for the icons, this is just a workaround. The solution would be to have an API for SVG icons so all these would be irrelevant. AFAIR the decision was to not include Font Awesome in the front end and inline all the icons but nobody did the work...
I do not see much problem, it already works good enough as it is.
Do not search for problem where there none.
And using "svg" or "non svg" it is a never ending story here
Category | Modules Front End Templates (site) NPM Change | ⇒ | JavaScript Repository Modules Front End Templates (site) NPM Change |
Category | Modules Front End Templates (site) NPM Change JavaScript Repository | ⇒ | Modules Front End Templates (site) JavaScript NPM Change |
thank you @dgrammatiko thats very nice now
it also would be nice if someone could share protostar make lighthouse happy hack
it also would be nice if someone could share protostar make lighthouse happy hack
Although this lazily loaded icon fonts could be backported in J3 there are fundamental problems in the J3 when it comes to performance:
So, the short answer is no, it's not possible without breaking badly B/C
the Prebuilt packages is unavailable for download. -> https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/32141/downloads/39823 -> Not Found The requested URL was not found on this server.
downloadable now
not sure if this is the desired result (working on windows with docker so it's quite slow)
Lighthouse should be used for comparison, eg before applying this PR and after. Probably docker has a very long time for time to first byte (or something else is ruin the results, can't really say without checking the actual trace).
this is with pr appliyed
dev4.loc-20210318T120341.html.zip
I have tested this item
I have tested this item
lighthouse more green now
Sorry, it still not acceptable.
It hardcoding fontawesome.css in a way that it not possible to disable it.
There already was questions how to disable it.
I would suggest just to remove fontawesome from the dependencies list of the template asset, and load in index.php/component.php/error.php.
In this way Users can disable it with the plugin onBeforeCompileHead event.
It hardcoding fontawesome.css in a way that it not possible to disable it.
Was there an option for disabling Font Awesome before this PR?
I have tested this item
My test site jumped from a performance score of 88 to 93.
Additionally, GTMetrix went from a B (94% performance, 74% structure) to a an A (99% performance, 74% structure) and then an A (100% performance, 89% structure) after I cleaned up some aspects on my test site as well as tweaking my server settings to serve pages using HTTP/2. Getting J4 to perform better each pass is always a good thing.
GTMetrix though in an early test also recommend "Consider using <link rel=preload>
to prioritize fetching resources that are currently requested later in page load." https://gtmetrix.com/preload-key-requests.html Is that something else to look at adding?
@Fedik isn't the code here loading Fontawesome viai templates/cassiopeia/component.php so your point above is already in place?
Being a fontawesome user myself, I know having it now in the core is a useful addition, and something I'd be looking to improve the loading of, and not something I'd be offloading.
You explain onBeforeCompileHead can be used, as an implementer, that type of cleverness is not typically something people would trouble themselves to know about as end users - comes back to a point I make in other conversations that the Joomla Documentation (https://docs.joomla.org/Plugin/Events/System in this case) really doesn't give much away at times with practical examples of why you'd do something with that function. https://docs.joomla.org/Plugin should have examples on writing various types of plugins - and for J4 include additions like Workflow and Web Services.
Was there an option for disabling Font Awesome before this PR?
It not about "option", but about possibility to do it in general, programatically.
Example, when you override the template asset at onBeforeCompileHead (or BeforeRender, not remember)
$wa->registerStyle('template.cassiopeia.ltr', 'template.min.css', [], [], [/* empty deps */])
And as I wrote before, this can be done without override if we remove fontawesome dependency from template (as you did here) and call it manually. Then you just use to disable the asset :
$wa->disableStyle('fontawesome');
@Fedik I'm getting what you're saying but asking a frontender to create a plugin for disabling some front end asset is not very nice advertisement of the friendliness of the product. Anyways I'm ok restoring this but honestly I don't really get what you mean here: I would suggest just to remove fontawesome from the dependencies list of the template asset, and load in index.php/component.php/error.php.
. Can you provide some sudo code and I'll patch this then
like you already did it here
joomla-cms/templates/cassiopeia/joomla.asset.json
Lines 8 to 19 in 016767a
and add $wa->useStyle('fontawesome');
to index.php/component.php/error.php
in this way it possible to disable it later one
but if I add $wa->useStyle('fontawesome');
the CSS will be loaded by the stylesRenderer or am I totally wrong?
if you want to have lazy loaded css, look how lazy loaded img
worked in past
that would be more wise move.
so you do:
<link rel="stylesheet" href="#" data-href="blabla/path/to/files.css" data-lazy-stylesheet/>
And write a little script for it. It probably even exist in the internet somewhere.
For this probably will be need to make a custom Joomla\CMS\WebAsset\AssetItem\WebAssetItem
(similar to LangActiveAssetItem )
That should return data-href
attribute with correct path.
but if I add $wa->useStyle('fontawesome'); the CSS will be loaded by the stylesRenderer or am I totally wrong?
yes, but this make possible to disable it via plugin
yes, but this make possible to disable it via plugin
Is there a way to quickly check if the asset is enabled?
Is there a way to quickly check if the asset is enabled?
sure $wa->isAssetActive('style', 'fontawesome')
@dgrammatiko Your PR has 2 good tests now so I could set it RTC, but I was waiting with that to see if you and @Fedik find a common sense and you maybe want to correct something. Now I'm not sure what to do. Shall I set RTC, or shall I wait?
@richard67 the changes @Fedik is asking here are a bit lower level (needs to be done in the Web Assets or the renderer) and need some thinking. Probably we should close this and start from another starting point: how do we lazyload css or js using the API?
seems a good question to ask...
Probably we should close this and start from another starting point:
how do we lazyload css or js using the API?
seems a good question to ask...
@dgrammatiko I'm leaving this decision up to you. Close it whenever you want, or ping me if I shall set RTC. We just should not leave it undecided for long, otherwise someone else might set it RTC and it gets merged before you can close it ;-)
@Fedik @richard67 I have a POC here: #32777 that solves this and also another case (inlining an asset)
My 2 cents - the more improvements that can be made in this area, the better the end result for making Joomla perform, and makes for an easier selling point to generate to say why use Joomla.
Category | Modules Front End Templates (site) NPM Change JavaScript | ⇒ | Libraries Modules Front End NPM Change Templates (site) JavaScript |
looks good to me
Since we have a consensus here, @alikon @particthistle can you re test this? Thanks
I have tested this item
I'm not actually spotting much in the way of improvement in lighthouse scores with the latest test and comparing code between the two instances of the page, but can see where things are now having "disabled" etc added. May need to test again with a clean build of J4 as my test environment is getting a bit out of hand over recent months.
Icons break:
Before: Mobile menu hamburger displays:
Return to top icon exists.
After: Menu Hamburger disappears
Return to top icon disappears
@particthistle how did you tested this? Patchtester probably won't work here...
Yep - lack of concentration there... It's NPM... new result in a moment...
I have tested this item
Let's try that again...
Installed full package to update NPM items.
Lighthouse was scoring 100 already after last tests, so I took a look at the various comments it provides. "Eliminate render-blocking resources" which is primarily what this PR does cleared from the second test.
A check of the icons this time around showed they load correctly.
Disabling Javascript in chrome: everything worked correctly.
Though you say Documentation changes aren't required, can someone please label this as Documentation Required as I think a guide for users on how to improve SEO - working on getting a team together to come back through and pick through the Documentation Required labeled PRs to make sure they're all actioned.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
I have tested this item disabled
attribute on link tags doesn't work anymore.
Status | Ready to Commit | ⇒ | Pending |
back to pending
@SharkyKZ that’s another false thing again from you: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link
@dgrammatiko actually I think @SharkyKZ is correct. Scroll further down that page
Actually he’s not referring to the platform support...
I assume he is referring to the "deprecated not for use in new websites"
Labels |
Added:
?
|
@alikon @particthistle can you please retest this? Sorry for the multiple tests here...
I assume he is referring to the "deprecated not for use in new websites"
Nope, the way I had coded this part was removing the disabled from all the styles, thus you couldn't use the attribute. Anyways, I'm switching to media=print
way which has exactly the same effect (stylesheet is not loaded by screen devices) until we remove the attribute media...
Lets use disabled as lazy-stylesheet is not valid
Did you found anything strict about it?
Because rel="apple-touch-icon"
also not valid (and many more similar), but everyone use it
Check this https://stackoverflow.com/a/16100892/1000711
"HTML specification" allows custom rel attribute values
I have tested this item
Accessibility and SEO I fixed up by adding my missing alt tags.
The only "Opportunity" suggested by lighthouse was to preload the link to fontawesome:
https://web.dev/uses-rel-preload/?utm_source=lighthouse&utm_medium=devtools
If I've read that link's info correctly, that would mean that the fontawesome would be deferred (which is what this PR does) and then pre-loaded before it was requested, so it is then ready to go when requested. Worth considering?
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-19 07:24:08 |
Closed_By | ⇒ | rdeutz | |
Labels |
Added:
?
|
Thank you!