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

Linkify sidebar headings for sibling items #93673

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Feb 5, 2022

Also adjust CSS so this doesn't produce excess padding/margin.

Note: I tried and failed to write a test with browser-UI-test. First I tried to assert-property: (".block.mod h3 a", {"href": "index.html#macros"}). But the href that gets read out is the fully-quallified URL, starting with file:///. That URL will differ depending on what path the test is run from, so that doesn't work.

Next I tried clicking on the appropriate sidebar link, and verifying that the appropriate heading on the next page is highlighted with the right background color. However, that also didn't work: according to browser-UI-test, the targeted heading was plain white. However, running with no-headless, I could see that it actually was yellow. I suspect this is a bug in the older version of Chromium used with browser-UI-test's bundled puppeteer, since it doesn't reproduce on latest Chrome.

Fixes #92957

Demo: https://rustdoc.crud.net/jsha/linkify-sidebar-headings/std/string/trait.ToString.html

r? @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 5, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2022
@@ -39,4 +39,4 @@ assert-position: ("#method\.must_use", {"y": 45})
// Check that the bottom-most item on the sidebar menu can be scrolled fully into view.
click: ".sidebar-menu-toggle"
scroll-to: ".block.keyword li:nth-child(1)"
assert-position: (".block.keyword li:nth-child(1)", {"y": 542.96875})
assert-position: (".block.keyword li:nth-child(1)", {"y": 542.234375})
Copy link
Member

Choose a reason for hiding this comment

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

With latest version, you can just check with 542 (which is much better for reading).

Copy link
Member

Choose a reason for hiding this comment

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

(from #93597)

@GuillaumeGomez
Copy link
Member

Did you try with the latest browser-ui-test version for background-color bug? Also I wonder if it's linked to show-text: true. Can you try adding it too please? If it fixes the bug, I'll enforce it like I did for color.

Also adjust CSS so this doesn't produce excess padding/margin.
@jsha
Copy link
Contributor Author

jsha commented Feb 6, 2022

I tried the background-color test again, and it worked, even on the current version of browser-ui-test. I suspect I may have been typoing the anchor name before.

@GuillaumeGomez
Copy link
Member

Perfect then! Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Feb 7, 2022

@GuillaumeGomez: 🔑 Insufficient privileges: Not in reviewers

@GuillaumeGomez
Copy link
Member

Weird again...

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 7, 2022

📌 Commit e27ebb5 has been approved by GuillaumeGomez

@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 Feb 7, 2022
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 7, 2022
…illaumeGomez

Linkify sidebar headings for sibling items

Also adjust CSS so this doesn't produce excess padding/margin.

Note: I tried and failed to write a test with browser-UI-test. First I tried to `assert-property: (".block.mod h3 a", {"href": "index.html#macros"})`. But the `href` that gets read out is the fully-quallified URL, starting with `file:///`. That URL will differ depending on what path the test is run from, so that doesn't work.

Next I tried clicking on the appropriate sidebar link, and verifying that the appropriate heading on the next page is highlighted with the right background color. However, that also didn't work: according to browser-UI-test, the targeted heading was plain white. However, running with no-headless, I could see that it actually was yellow. I suspect this is a bug in the older version of Chromium used with browser-UI-test's bundled puppeteer, since it doesn't reproduce on latest Chrome.

Fixes rust-lang#92957

Demo: https://rustdoc.crud.net/jsha/linkify-sidebar-headings/std/string/trait.ToString.html

r? `@GuillaumeGomez`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2022
Rollup of 13 pull requests

Successful merges:

 - rust-lang#88313 (Make the pre-commit script pre-push instead)
 - rust-lang#91530 (Suggest 1-tuple parentheses on exprs without existing parens)
 - rust-lang#92724 (Cleanup c_str.rs)
 - rust-lang#93208 (Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$t> for rust 1.60.0)
 - rust-lang#93394 (Don't allow {} to refer to implicit captures in format_args.)
 - rust-lang#93416 (remove `allow_fail` test flag)
 - rust-lang#93487 (Fix linking stage1 toolchain in `./x.py setup`)
 - rust-lang#93673 (Linkify sidebar headings for sibling items)
 - rust-lang#93680 (Drop json::from_reader)
 - rust-lang#93682 (Update tracking issue for `const_fn_trait_bound`)
 - rust-lang#93722 (Use shallow clones for submodules managed by rustbuild, not just bootstrap.py)
 - rust-lang#93723 (Rerun bootstrap's build script when RUSTC changes)
 - rust-lang#93737 (bootstrap: prefer using '--config' over 'RUST_BOOTSTRAP_CONFIG')

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0b6c7fb into rust-lang:master Feb 7, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

rustdoc: linkify "Structs", "Traits", etc., in sidebar
5 participants