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.

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

Fix positioning for detail toggles on mobile. #84602

Closed
wants to merge 4 commits into from

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Apr 26, 2021

The rule that was supposed to put these in the right place on mobile had
an extra class (item-list) in its selector, but that class isn't
actually part of the DOM for everything we care about.

After removing that extra class, I found the rule wasn't firing. That's
because @media doesn't increase specificity
(https://css-tricks.com/how-much-specificity-do-rules-have-like-keyframes-and-media/),
and the rule was being overridden by the later rule with the exact same
selector.

I moved all the details-related rules above the @media queries because
in general I think we expect rules inside @media to override others.
However, I didn't move the rules that I haven't touched, since I don't
know all the places to check that they are working as expected.

Demo at https://hoffman-andrews.com/rust/reorder-details-css/std/string/struct.String.html#trait-implementations

r? @GuillaumeGomez

The rule that was supposed to put these in the right place on mobile had
an extra class (item-list) in its selector, but that class isn't
actually part of the DOM for everything we care about.

After removing that extra class, I found the rule wasn't firing. That's
because @media doesn't increase specificity
(https://css-tricks.com/how-much-specificity-do-rules-have-like-keyframes-and-media/),
and the rule was being overridden by the later rule with the exact same
selector.

I moved all the details-related rules above the @media queries because
in general I think we expect rules inside @media to override others.
However, I didn't move the rules that I haven't touched, since I don't
know all the places to check that they are working as expected.
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2021
/* The hideme class is used on summary tags that contain a span with
placeholder text shown only when the toggle is closed. For instance,
"Expand description" or "Show methods". */
details.rustdoc-toggle > summary.hideme {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "hideme", what do you think about "hide-me"? Easier to read.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for all below.

cursor: pointer;
}

details.rustdoc-toggle > summary::-webkit-details-marker {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an equivalent on firefox?

@jyn514 jyn514 added the A-rustdoc-ui Area: Rustdoc UI (generated HTML) label Apr 28, 2021
@bors
Copy link
Contributor

bors commented May 2, 2021

☔ The latest upstream changes (presumably #84754) made this pull request unmergeable. Please resolve the merge conflicts.

@jsha
Copy link
Contributor Author

jsha commented May 14, 2021

Closing in favor of #85289.

@jsha jsha closed this May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants