-
Notifications
You must be signed in to change notification settings - Fork 13k
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.
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
rustdoc: remove unused CSS class in-band
#102672
Conversation
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @Folyd, @jsha |
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon. Please see the contribution instructions for more information. |
<h1 class=\"fqn\">\ | ||
<span class=\"in-band\">Rustdoc settings</span>\ | ||
Rustdoc settings\ | ||
</h1>\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can inline it completely at this point:
<h1 class=\"fqn\">\ | |
<span class=\"in-band\">Rustdoc settings</span>\ | |
Rustdoc settings\ | |
</h1>\ | |
<h1 class=\"fqn\">Rustdoc settings</h1>\ |
src/librustdoc/html/render/mod.rs
Outdated
"<h1 class=\"fqn\">\ | ||
<span class=\"in-band\">List of all items</span>\ | ||
List of all items\ | ||
</h1>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here:
"<h1 class=\"fqn\">\ | |
<span class=\"in-band\">List of all items</span>\ | |
List of all items\ | |
</h1>", | |
"<h1 class=\"fqn\">List of all items</h1>", |
@@ -518,7 +518,7 @@ if (typeof exports !== 'undefined') {exports.searchIndex = searchIndex}; | |||
|
|||
let content = format!( | |||
"<h1 class=\"fqn\">\ | |||
<span class=\"in-band\">List of all crates</span>\ | |||
List of all crates\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
/* Then override it with `anywhere`, which is required to make non-Safari browsers break | ||
more aggressively when we want them to. */ | ||
overflow-wrap: anywhere; | ||
background-color: var(--main-background-color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more background?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not needed. The fqn
is always right on top of a regular background of the same color anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirmed it. 👍
Do you have an online by any chance? I'd love to be able to take a look at the result. |
Apart the small nits, looks all good to me. r=me once they're fixed. |
38971af
to
aa7aa91
Compare
This comment has been minimized.
This comment has been minimized.
aa7aa91
to
f7a1d69
Compare
@bors r=GuillaumeGomez rollup |
📌 Commit f7a1d695612d33680ec62273266c9d6a664f00e8 has been approved by It is now in the queue for this repository. |
☔ The latest upstream changes (presumably #102691) made this pull request unmergeable. Please resolve the merge conflicts. |
Since a7c25b2 removed `in-band` from code headers, the only remaining uses of the `in-band` class are: https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/render/write_shared.rs#L520-L521 https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/templates/print_item.html#L2-L3 https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/render/context.rs#L637-L638 https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/render/mod.rs#L368-L369 https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/render/mod.rs#L401-L402 https://github.com/rust-lang/rust/blob/02cd79afb8080fce8c8ce35533c54d8ecf8f390e/src/librustdoc/html/static/js/main.js#L525 Since all of these uses are nested below `h1.fqn`, we can get rid of it, and the support code that was used for when `in-band` was part of item rendering.
f7a1d69
to
3cb03cb
Compare
@bors r=GuillaumeGomez rollup |
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#102672 (rustdoc: remove unused CSS class `in-band`) - rust-lang#102693 (Revert "Use getentropy when possible on all Apple platforms") - rust-lang#102694 (Suggest calling method if fn does not exist) - rust-lang#102708 (Suggest `==` to wrong assign expr) - rust-lang#102710 (Add test for issue 82633) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Since a7c25b2 removed
in-band
from code headers, the only remaining uses of thein-band
class are:rust/src/librustdoc/html/render/write_shared.rs
Lines 520 to 521 in 02cd79a
rust/src/librustdoc/html/templates/print_item.html
Lines 2 to 3 in 02cd79a
rust/src/librustdoc/html/render/context.rs
Lines 637 to 638 in 02cd79a
rust/src/librustdoc/html/render/mod.rs
Lines 368 to 369 in 02cd79a
rust/src/librustdoc/html/render/mod.rs
Lines 401 to 402 in 02cd79a
rust/src/librustdoc/html/static/js/main.js
Line 525 in 02cd79a
Since all of these uses are nested below
h1.fqn
, we can get rid of it, and the support code that was used for whenin-band
was part of item rendering.