Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge settings.css into rustdoc.css #115812

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Merge settings.css into rustdoc.css #115812

merged 1 commit into from
Sep 13, 2023

Conversation

GuillaumeGomez
Copy link
Member

There aren't that many CSS rules in settings.css, however quite some code is needed around it, making it more costly than useful. I think it's better to merge into rustdoc.css to simplify this.

r? @notriddle

@GuillaumeGomez GuillaumeGomez marked this pull request as ready for review September 13, 2023 09:11
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2023

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 13, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the merge-settings-into-rustdoc-css branch from 602c5cc to 9944f01 Compare September 13, 2023 11:59
@GuillaumeGomez
Copy link
Member Author

Removed the loadCss completely as it's not used anywhere.

@notriddle
Copy link
Contributor

This does simplify things a lot, and I was thinking of doing the same thing.

But I wonder if it might instead be helpful to rename settings.css to extra.css, pull the search engine stylesheet rules into it, have search.js load it, and have it get preloaded when you click the search bar? The annoying part about merging everything into rustdoc.css is that it needs to get loaded before first contentful paint, so it would be nice to keep stuff that isn't needed for that out in separate files.

@GuillaumeGomez
Copy link
Member Author

The big being: is it really worth it? I think the perf impact is close to non-existent here. If we start having pages that have their own specific style and enough to make it actually worth it to put them into their own files, could be worth a try. Currently it's not the case as far as I can see.

@notriddle
Copy link
Contributor

Good point. It looks like, if we want to improve the TTFP, our best bet is to look at themes (light.css is the last stylesheet to load).

image

@notriddle
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2023

📌 Commit 9944f01 has been approved by notriddle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 13, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2023
@bors
Auto merge of rust-lang#115820 - matthiaskrgr:rollup-kyglvpu, r=matth…
eb2446a 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#115736 (Remove `verbose_generic_activity_with_arg`)
 - rust-lang#115771 (cleanup leftovers of const_err lint)
 - rust-lang#115798 (add helper method for finding the one non-1-ZST field)
 - rust-lang#115812 (Merge settings.css into rustdoc.css)
 - rust-lang#115815 (fix: return early when has tainted in mir pass)
 - rust-lang#115816 (Disabled socketpair for Vita)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 27c6099 into rust-lang:master Sep 13, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 13, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2023
@rust-timer
Unrolled build for rust-lang#115812
45d3ed0 
Rollup merge of rust-lang#115812 - GuillaumeGomez:merge-settings-into-rustdoc-css, r=notriddle

Merge settings.css into rustdoc.css

There aren't that many CSS rules in `settings.css`, however quite some code is needed around it, making it more costly than useful. I think it's better to merge into `rustdoc.css` to simplify this.

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez deleted the merge-settings-into-rustdoc-css branch September 13, 2023 20:27
notriddle pushed a commit to notriddle/rust that referenced this pull request Sep 13, 2023
@notriddle-cerebrum
rustdoc: merge theme css into rustdoc.css
e8cc7c6 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 13, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
a92c2f2 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 13, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
4a1f1ca 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
@notriddle notriddle mentioned this pull request Sep 13, 2023
Merged
notriddle added a commit to notriddle/rust that referenced this pull request Sep 13, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
5f9c3db 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 13, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
07754de 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 14, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
5b8ba88 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 14, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
1970d25 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 14, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
0d626c7 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
notriddle added a commit to notriddle/rust that referenced this pull request Sep 14, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
d18a5e5 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2023
@bors
Auto merge of rust-lang#115829 - notriddle:notriddle/rustdoc-theme-cs…
aa541a2 
…s-merge, r=GuillaumeGomez

rustdoc: merge theme css into rustdoc.css

Based on rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the migration to CSS variables made the theme files as small as they are nowadays. This is already how docs.rs and mdBook do it.

WebPageTest comparison page:

https://www.webpagetest.org/video/compare.php?tests=230913_AiDc3F_B9E,230913_AiDc7G_B9B

Filmstrip comparison:

![image](https://github.com/rust-lang/rust/assets/1593513/7ccad27b-7497-47ee-94c0-1a701b69c0c2)

Old waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/7a6e4375-226d-4205-8871-a4d775a70748)

New waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/e29112e3-84f7-417d-a250-cd6c10fa50f5)
notriddle added a commit to notriddle/rust that referenced this pull request Sep 15, 2023
@notriddle
rustdoc: merge theme css into rustdoc.css
7e86fd6 
Based on
rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the
migration to CSS variables made the theme files as small as they are
nowadays. This is already how docs.rs and mdBook do it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2023
@bors
Auto merge of rust-lang#115829 - notriddle:notriddle/rustdoc-theme-cs…
81326e1 
…s-merge, r=GuillaumeGomez

rustdoc: merge theme css into rustdoc.css

Based on rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the migration to CSS variables made the theme files as small as they are nowadays. This is already how docs.rs and mdBook do it.

WebPageTest comparison page:

https://www.webpagetest.org/video/compare.php?tests=230913_AiDc3F_B9E,230913_AiDc7G_B9B

Filmstrip comparison:

![image](https://github.com/rust-lang/rust/assets/1593513/7ccad27b-7497-47ee-94c0-1a701b69c0c2)

Old waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/7a6e4375-226d-4205-8871-a4d775a70748)

New waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/e29112e3-84f7-417d-a250-cd6c10fa50f5)
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 16, 2023
@bors
Auto merge of rust-lang#115829 - notriddle:notriddle/rustdoc-theme-cs…
c48e6ff 
…s-merge, r=GuillaumeGomez

rustdoc: merge theme css into rustdoc.css

Based on rust-lang#115812 (comment)

Having them in separate files used to make more sense, before the migration to CSS variables made the theme files as small as they are nowadays. This is already how docs.rs and mdBook do it.

WebPageTest comparison page:

https://www.webpagetest.org/video/compare.php?tests=230913_AiDc3F_B9E,230913_AiDc7G_B9B

Filmstrip comparison:

![image](https://github.com/rust-lang/rust/assets/1593513/7ccad27b-7497-47ee-94c0-1a701b69c0c2)

Old waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/7a6e4375-226d-4205-8871-a4d775a70748)

New waterfall:

![image](https://github.com/rust-lang/rust/assets/1593513/e29112e3-84f7-417d-a250-cd6c10fa50f5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@notriddle notriddle

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.74.0
Development

Successfully merging this pull request may close these issues.
5 participants
@GuillaumeGomez @rustbot @rust-log-analyzer @notriddle @bors