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

Make [-] invisible in most cases #86154

Closed
wants to merge 2 commits into from
Closed

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jun 8, 2021

Many items on doc pages are collapsible, so they have [-] / [+] items. These icons take up critical space in the gutter, reducing the ability to scan for an item you're looking for. The [+] icon is very important, so you can open the item you're looking for once you find it. But for a group of toggles that are already open, having [-] is not very useful, since collapsing a single item doesn't give you a much clearer view of the page.

This PR changes the UI so that [-] doesn't show up on sections that are toggled open. Those sections can be toggled closed with the keyboard shortcuts or the button in the top right. Also, some sections (like methods) can be toggled closed by a setting.

For sections that start in a closed position, when you click the [+] to open them, they will get a [-] to close them again. Otherwise it would be frustrating to open a section and not be able to immediately change your mind and close it.

This change made the in stability output point into empty space, so I changed the display of stability information to not use (see comment on 770f7dd2d01054f64d8433034a8e8ee8ff53c26a).

Fixes #85372

Demo: https://jacob.hoffman-andrews.com/rust/invisible-toggles/std/string/struct.String.html#implementations
https://jacob.hoffman-andrews.com/rust/invisible-toggles/std/io/trait.Read.html#provided-methods

r? @GuillaumeGomez

This reduces clutter on the page, while still supporting the "auto hide
X" settings and the "collapse all docs" button.
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

A change occurred in the Ayu theme.

cc @Cldfire

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 8, 2021
@rust-log-analyzer

This comment has been minimized.

This was originally introduced in # 51387, to solve a UI problem: in a
list of collapsed methods, it was not clear whether the stability marker
attached to the item above it or the one below it.

That problem has now been solved in a couple of different ways:

 - The margin between the stability marker and the item it annotates is
   much smaller, indicating their relationship.
 - Stability markers are now collapsed along with the docblock.

Also, the arrow made our CSS fragile, since it was positioned absolutely
and needed to line up horizontally with the beginning of the item (or
with the link icon).

Aligning this with the rest of the docblock makes getting the CSS right
a lot easier.
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
  SCCACHE_BUCKET: rust-lang-ci-sccache2
  TOOLSTATE_REPO: https://github.com/rust-lang-nursery/rust-toolstate
  CACHE_DOMAIN: ci-caches.rust-lang.org
  EXTRA_VARIABLES: {
 "CI_ONLY_WHEN_SUBMODULES_CHANGED": 1
##[endgroup]
adding extra environment variable CI_ONLY_WHEN_SUBMODULES_CHANGED
linux builder detected, using docker to run the build
##[group]Run src/ci/scripts/should-skip-this.sh
---
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between ed597e7e19d0fe716d9f81b1e840a5abbfd7c28d and ca2f74a6a8dec84d0362621dc0702ed6010b18ca
Executing the job since rustdoc was updated
src/ci/scripts/verify-channel.sh
shell: /bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-tools
---
sidebar... ok
source-code-page... ok
theme-change... ok
toggle-docs-mobile... FAILED
[ERROR] (line 5) assert didn't fail: for command `assert-false: (".top-doc", "open", "")`
toggle-docs... ok
trait-sidebar-item-order... ok



command did not execute successfully: "/node-v14.4.0-linux-x64/bin/node" "/checkout/src/tools/rustdoc-gui/tester.js" "--doc-folder" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui" "--tests-folder" "/checkout/src/test/rustdoc-gui"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/test/rustdoc-gui --stage 2
Build completed unsuccessfully in 0:00:36

@jsha
Copy link
Contributor Author

jsha commented Jun 9, 2021

We have a failure in the rustdoc-gui tests that is presumably triggered by the real behavior change here. I've tried to debug it locally but I'm not sure how to get an interactive browser session to poke around after a test fails. Any recommendations @GuillaumeGomez ?

@Nemo157
Copy link
Member

Nemo157 commented Jun 9, 2021

Two places I would still leave the toggle is the per-impl-block headers and the description. The former are not toggled by the "collapse all docs" button so there is no way to collapse them now, the latter I at least very commonly collapse to get it out of the way and start skimming the method docs.

This also exacerbates an existing nightly issue that the text in trait-implementations is not indented far enough, previously that was sort of ok since the [-] acted as a bullet point

image

but now it's very hard to find the method headers inside the trait-implementation.

image

@GuillaumeGomez
Copy link
Member

Sorry but I really don't like it either... The disappearance of the toggle makes it look like you can't collapse it anymore. And just like @Nemo157 said, it actually makes the reading harder. This was a good visual element to "separate" elements.

@Cldfire
Copy link
Contributor

Cldfire commented Jun 10, 2021

One other anecdote: for me the [-] button next to methods is an important visual landmark that helps my eyes "feel" where the beginning and end of each method's docs are. With that button removed, this visual landmark is gone, meaning there's no differentiation in indentation or any other sort of prominent landmark via which to identify the beginning / end of a method in an impl block.

@jsha
Copy link
Contributor Author

jsha commented Jun 14, 2021

That's good feedback that people see the [-] as a visual separator between methods. That's actually part of my goal, to improve the visual separation between methods, and generally improve scanability. To me, the [-] do indeed serve as separators but I don't think they are the best tool for the job. They also wind up acting as visual noise and distracting from the most important thing: the method name.

I figured what I'm trying to get at here would be clearer if I put together a few of the changes I'm thinking of into one PR rather than doing this piecemeal. Take a look at #86283.

The disappearance of the toggle makes it look like you can't collapse it anymore.

Is the idea that people are likely to want to collapse toggles one-by-one? Or do you want the series of [-] down the side of the page to serve as a sort of advertisement for the "collapse all docs" feature?

@GuillaumeGomez
Copy link
Member

I can only use myself as example: I tend to collapse everything by default and then expand items to check if this is the one I'm looking for, and if not, to collapse it again.

@bors
Copy link
Contributor

bors commented Jun 17, 2021

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

@jsha
Copy link
Contributor Author

jsha commented Jun 25, 2021

Closing. Some of the same concepts are incorporated into #86283; when I pick that up again I'm interested to explore this more.

@jsha jsha closed this Jun 25, 2021
@jsha jsha deleted the invisible-toggles2 branch June 25, 2021 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

rustdoc: make [-] invisible in most cases
7 participants