Move theme dropdown out of HTML#16851
Move theme dropdown out of HTML#16851GuillaumeGomez wants to merge 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
the
S-waiting-on-review
|
|
There was a problem hiding this comment.
theme.js is render blocking so the page load time would be the same or slower (since the JS version is larger)
But mainly, the size of the dropdown is negligible post compression. If it were a straight improvement I wouldn't say it's worth the awkwardness of HTML in JS strings
There was a problem hiding this comment.
I can include all the changes at once if you prefer (ie, removing all the filters from HTML as well). Just thought it would make review simpler. As you prefer. :)
There was a problem hiding this comment.
The filter dropdowns themselves could go into script.js but not the buttons, but again I just don't think it's worth it. Currently all elements are present in the HTML template which is a lovely property to have, deferring a small amount of bytes isn't a good enough trade for that IMO
There was a problem hiding this comment.
Removing the dropdowns from the HTML would remove 4.7KB. Removing the theme dropdown from the HTML would remove 0.5KB.
So 5.3KB is a bit more than two lints "blocks". So not really a game-changer, but it allows to completely split the static content from the dynamic one, which I think is a very good thing. That would also allow to simplify the CSS since we won't need to have a CSS class to know if JS is enabled or not. A few small things that together seem worth it to me.
There was a problem hiding this comment.
The dropdown toggles and search bar still have to be in the HTML to avoid layout instability so there's no meaningful split between static/dynamic to be had, having some parts in one file and some in another is just annoying
The compressed size is what matters though, menu repetition compresses significantly better than ~unique prose. I measure it to be 402 byte difference in compressed files for the filter dropdowns (the ul.dropdown-menus)
There was a problem hiding this comment.
The dropdown toggles and search bar still have to be in the HTML to avoid layout instability so there's no meaningful split between static/dynamic to be had, having some parts in one file and some in another is just annoying
Just like in this PR, the HTML would be added before the page is even visible to the user since the loading of the script would block the rendering (a fraction of second without caching though).
The compressed size is what matters though, menu repetition compresses significantly better than ~unique prose. I measure it to be 402 byte difference in compressed files for the filter dropdowns (the
ul.dropdown-menus)
True, but that's about transmitted data, not what you have in the end. For noscript users, they would still have the 5KB extra.
There was a problem hiding this comment.
Just like in this PR, the HTML would be added before the page is even visible to the user since the loading of the script would block the rendering
Then what would it achieve? That would be the same issue as #16851 (comment) - it's only if you moved content into script.js that the page could start to display earlier due to a smaller download that the sizes would become relevant
True, but that's about transmitted data, not what you have in the end. For noscript users, they would still have the 5KB extra.
That's the part that matters for load times outside of extreme examples
I don't consider the tiny waste for noscript users to be a problem that needs solving
There was a problem hiding this comment.
Then what would it achieve? That would be the same issue as #16851 (comment) - it's only if you moved content into
script.jsthat the page could start to display earlier due to a smaller download that the sizes would become relevant
Page is smaller for noscript users.
I don't consider the tiny waste for noscript users to be a problem that needs solving
Arf, an opinion difference that locks this discussion I guess...
3 participants
I continue my serie of changes aiming to reduce HTML size. Since this button is hidden when JS is disabled, no point in keeping it in the HTML in the first place. The next PR will move filters into JS as well, which will allow to remove the
jsclass we set.r? @Alexendoo
changelog: none