-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add method-toggle to <details> for methods #85169
Conversation
Some changes occurred in HTML/CSS/JS. |
This comment has been minimized.
This comment has been minimized.
Looks good to me! Just need to update the test failure and it's good to go. |
src/librustdoc/html/render/mod.rs
Outdated
let method_toggle_class = | ||
if item_type == ItemType::Method { " method-toggle" } else { "" }; | ||
write!( | ||
w, | ||
"<details class=\"rustdoc-toggle{}\" open><summary class=\"{}{}\">", | ||
method_toggle_class, item_type, in_trait_class | ||
); |
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.
Better move space from " method-toggle"
into format string?
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 because then we might have an unwanted extra whitespace in case it's not a method.
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.
js\html minificator remove extra spaces, isn't it?
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.
There is no html minifier.
☔ The latest upstream changes (presumably #85199) made this pull request unmergeable. Please resolve the merge conflicts. |
Started work on fixing the XPath tests. I can't really fix them properly until #85167 is fixed, because that one actually involves improper nesting of the HTML. In order to fix the XPath tests reasonably for this change I need the |
I realized the XPath tests were telling me about a real problem: When there's no documentation on a method, we don't wrap it in |
The makes the code for handling "auto-hide" settings more consistent.
I see, it's unfortunate but it makes sense. Well, that's already an improvement so let's go forward with it! Thanks! @bors: r+ rollup |
📌 Commit 24480de has been approved by |
Add method-toggle to <details> for methods The makes the code for handling "auto-hide" settings more consistent. Demo at https://hoffman-andrews.com/rust/hoist-classes/std/string/struct.String.html Fixes rust-lang#84829
Add method-toggle to <details> for methods The makes the code for handling "auto-hide" settings more consistent. Demo at https://hoffman-andrews.com/rust/hoist-classes/std/string/struct.String.html Fixes rust-lang#84829
Rollup of 8 pull requests Successful merges: - rust-lang#84717 (impl FromStr for proc_macro::Literal) - rust-lang#85169 (Add method-toggle to <details> for methods) - rust-lang#85287 (Expose `Concurrent` (private type in public i'face)) - rust-lang#85315 (adding time complexity for partition_in_place iter method) - rust-lang#85439 (Add diagnostic item to `CStr`) - rust-lang#85464 (Fix UB in documented example for `ptr::swap`) - rust-lang#85470 (Fix invalid CSS rules for a:hover) - rust-lang#85472 (CTFE Machine: do not expose Allocation) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The makes the code for handling "auto-hide" settings more consistent.
Demo at https://hoffman-andrews.com/rust/hoist-classes/std/string/struct.String.html
Fixes #84829