Reintroduce spotlight / "important traits" feature #74370

Manishearth · 2020-07-15T16:05:14Z

(Reopened version of #74111 because Github is broken, see discussion there)

This PR reintroduces the "spotlight" ("important traits") feature.

A couple changes have been made:

As there were concerns about its visibility, it has been moved to be next to the return type, as opposed to being on the side.

It also no longer produces a modal, it shows the traits on hover, and it can be clicked on to pin the hover bubble.

It also works fine on mobile:

rust-highfive · 2020-07-15T16:05:17Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

rust-highfive · 2020-07-15T16:05:19Z

r? @ollie27

(rust_highfive has picked a reviewer for you, use r? to override)

Manishearth · 2020-07-15T16:05:48Z

r? @GuillaumeGomez

src/librustdoc/html/static/rustdoc.css

ollie27

I've noticed a bit of a graphic glitch for free functions:

ollie27 · 2020-07-15T17:51:29Z

src/librustdoc/html/render.rs

+
+    if let Some(did) = decl.output.def_id() {
+        let c = cache();
+        if let Some(impls) = c.impls.get(&did) {


This isn't a good way to find applicable impls because it doesn't take into account things like generic parameters and mutability. For example:

and

I'd imagine this will need to use a mechanism similar to how auto trait and blankets impls are found.

Given that this is reintroducing an old feature that was previously accepted in this form, can we handle this as a followup?

Like, I want to ladn this so we can gather more community feedback on this feature and grow it. It's a feature that existed in this form for a while and was recently removed, my priority is getting it back in so we can improve it.

src/librustdoc/html/render.rs

Manishearth · 2020-07-15T20:23:30Z

@ollie27 Guillaume has said he's good with this once we have your approval. Would you be okay with landing this with followups filed? I would rather keep this PR scoped to mostly bringing the feature back as is, with some basic UI changes. I want to do improvements differently, with a tracking issue.

ollie27 · 2020-07-16T13:22:35Z

I'm really not a fan of reintroducing bugs but I guess it's not the end of the world. There's even an old issue still open (#55082) for the impls that aren't applicable.

Could the issue with free functions I pointed out in #74370 (review) be fixed though? That wasn't an issue with the old implementation. If it's too difficult to fix then the important traits feature could be disabled for those functions for now.

Manishearth · 2020-07-16T14:49:58Z

Yeah I can fix that, i didn't immediately understand what the bug was but I do now

Manishearth · 2020-07-16T16:03:01Z

@ollie27 It was the selector in #44671 not being specific enough, Fixed

Manishearth · 2020-07-16T16:13:45Z

Manishearth · 2020-07-16T16:17:07Z

@ollie27 Guillaume is holding off on r+ing until you confirm the fix above, can you do so?

bors · 2020-07-16T16:55:25Z

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

This reverts commit 1244ced.

This reverts commit 13c6d58.

This makes the spotlight show on hover instead of click. Clicks can be used to persist it, which is also what's used on mobile.

We need it for run buttons (rust-lang#44671), but not function defs

ollie27 · 2020-07-16T17:37:20Z

Looks good now, glad it was an easy fix.

Manishearth · 2020-07-16T17:41:35Z

@bors r=GuillaumeGomez

bors · 2020-07-16T17:41:36Z

📌 Commit c621a54 has been approved by GuillaumeGomez

bors · 2020-07-16T17:41:37Z

🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened

…meGomez Reintroduce spotlight / "important traits" feature (Reopened version of rust-lang#74111 because Github is broken, see discussion there) Fixes rust-lang#73785 This PR reintroduces the "spotlight" ("important traits") feature. A couple changes have been made: As there were concerns about its visibility, it has been moved to be next to the return type, as opposed to being on the side. It also no longer produces a modal, it shows the traits on hover, and it can be clicked on to pin the hover bubble. ![image](https://user-images.githubusercontent.com/1617736/86674555-a82d2600-bfad-11ea-9a4a-a1a9ffd66ae5.png) ![image](https://user-images.githubusercontent.com/1617736/86674533-a1061800-bfad-11ea-9e8a-c62ad86ed0d7.png) It also works fine on mobile: ![image](https://user-images.githubusercontent.com/1617736/86674638-bda25000-bfad-11ea-8d8d-1798b608923e.png)

@ghost

…arth Rollup of 21 pull requests Successful merges: - rust-lang#73566 (Don't run `everybody_loops` for rustdoc; instead ignore resolution errors) - rust-lang#73771 (Don't pollute docs/suggestions with libstd deps) - rust-lang#73794 (Small cleanup for E0705 explanation) - rust-lang#73807 (rustdoc: glue tokens before highlighting) - rust-lang#73835 (Clean up E0710 explanation) - rust-lang#73926 (Ignoring test case: [codegen] repr-transparent-aggregates-1.rs for aarch64) - rust-lang#73981 (Remove some `ignore-stage1` annotations.) - rust-lang#73998 (add regression test for rust-lang#61216) - rust-lang#74140 (Make hir ProjectionKind more precise) - rust-lang#74148 (Move #[doc(alias)] check in rustc) - rust-lang#74159 (forbid generic params in the type of const params) - rust-lang#74171 (Fix 44056 test with debug on macos.) - rust-lang#74221 (Don't panic if the lhs of a div by zero is not statically known) - rust-lang#74325 (Focus on the current file in the source file sidebar) - rust-lang#74359 (rustdoc: Rename internal API fns to `into_string`) - rust-lang#74370 (Reintroduce spotlight / "important traits" feature) - rust-lang#74390 (Fix typo in std::mem::transmute documentation) - rust-lang#74391 (BtreeMap: superficially refactor root access) - rust-lang#74392 (const generics triage) - rust-lang#74397 (Fix typo in the latest release note) - rust-lang#74406 (Set shell for github actions CI) Failed merges: r? @ghost

Manishearth · 2020-07-16T22:45:07Z

I'll do the follow-ups soonish

Manishearth · 2020-07-16T23:19:14Z

Followup: #74417

I'll open an internals thread to workshop the UI and name once it's on nightly

Manishearth · 2020-07-17T22:11:28Z

Internals thread: https://internals.rust-lang.org/t/feedback-on-important-traits-rustdoc-feature/12752

Manishearth requested a review from GuillaumeGomez July 15, 2020 16:05

rust-highfive assigned ollie27 Jul 15, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 15, 2020

Manishearth mentioned this pull request Jul 15, 2020

Reintroduce spotlight/"important traits" feature #74111

Closed

rust-highfive assigned GuillaumeGomez and unassigned ollie27 Jul 15, 2020

GuillaumeGomez reviewed Jul 15, 2020

View reviewed changes

src/librustdoc/html/static/rustdoc.css Outdated Show resolved Hide resolved

Manishearth force-pushed the re-spotlight branch 6 times, most recently from c35e99d to 1a6fe87 Compare July 15, 2020 17:17

ollie27 reviewed Jul 15, 2020

View reviewed changes

GuillaumeGomez reviewed Jul 15, 2020

View reviewed changes

src/librustdoc/html/render.rs Show resolved Hide resolved

Manishearth added 5 commits July 16, 2020 09:58

Revert "Remove "important traits" feature"

9845075

This reverts commit 1244ced.

Revert "Remove spotlight usage"

3cecd60

This reverts commit 13c6d58.

Move spotlight next to the return type

c90fb71

Make spotlight show on hover

734afb4

This makes the spotlight show on hover instead of click. Clicks can be used to persist it, which is also what's used on mobile.

Review comments for JS

ae6c7e6

Manishearth added 3 commits July 16, 2020 09:58

Add Ayu theme for spotlight

0625b29

Remove !important on border-color and background-color

a474b27

Don't position:relative on all pres

c621a54

We need it for run buttons (rust-lang#44671), but not function defs

Manishearth force-pushed the re-spotlight branch from a497fd3 to c621a54 Compare July 16, 2020 16:58

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 Jul 16, 2020

Manishearth mentioned this pull request Jul 16, 2020

Rollup of 17 pull requests #74407

Closed

Manishearth mentioned this pull request Jul 16, 2020

Rollup of 21 pull requests #74408

Merged

bors merged commit fc09817 into rust-lang:master Jul 16, 2020

Manishearth deleted the re-spotlight branch July 16, 2020 23:16

Manishearth mentioned this pull request Jul 16, 2020

Spotlight shows traits with generic impls that do not apply to the specific type #74417

Open

Manishearth mentioned this pull request Jul 19, 2020

Missing docs for non-first impl #74531

Closed

cuviper added this to the 1.47.0 milestone May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reintroduce spotlight / "important traits" feature #74370

Reintroduce spotlight / "important traits" feature #74370

Manishearth commented Jul 15, 2020

rust-highfive commented Jul 15, 2020

rust-highfive commented Jul 15, 2020

Manishearth commented Jul 15, 2020

ollie27 left a comment

ollie27 Jul 15, 2020

Manishearth Jul 15, 2020

Manishearth Jul 15, 2020

Manishearth commented Jul 15, 2020

ollie27 commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

bors commented Jul 16, 2020

ollie27 commented Jul 16, 2020

Manishearth commented Jul 16, 2020

bors commented Jul 16, 2020

bors commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 17, 2020

Reintroduce spotlight / "important traits" feature #74370

Reintroduce spotlight / "important traits" feature #74370

Conversation

Manishearth commented Jul 15, 2020

rust-highfive commented Jul 15, 2020

rust-highfive commented Jul 15, 2020

Manishearth commented Jul 15, 2020

ollie27 left a comment

Choose a reason for hiding this comment

ollie27 Jul 15, 2020

Choose a reason for hiding this comment

Manishearth Jul 15, 2020

Choose a reason for hiding this comment

Manishearth Jul 15, 2020

Choose a reason for hiding this comment

Manishearth commented Jul 15, 2020

ollie27 commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

bors commented Jul 16, 2020

ollie27 commented Jul 16, 2020

Manishearth commented Jul 16, 2020

bors commented Jul 16, 2020

bors commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 16, 2020

Manishearth commented Jul 17, 2020