rustdoc: merge doctest tooltip with notable traits tooltip #107340
Conversation
rustbot
added
S-waiting-on-review
|
A change occurred in the Ayu theme. cc @Cldfire Some changes occurred in HTML/CSS themes. Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @Folyd, @jsha |
This comment has been minimized.
This comment has been minimized.
notriddle
force-pushed
the
notriddle/simplify-doctest-tooltip
branch
from
afad8a3
to
fe696a4
Compare
| @@ -1093,44 +1093,8 @@ pre.rust .doccomment { | |||
| display: block; | |||
| left: -25px; | |||
| top: 5px; | |||
| } | |||
|
|
|||
| .example-wrap .tooltip:hover::after { | |||
There was a problem hiding this comment.
It seems like a regression to me to have it positioned using JS now.
There was a problem hiding this comment.
It still works without JavaScript.
If JS is disabled, the title attribute causes your browser’s built-in tooltip to appear. It’s only actually needed for click handling, which the old setup couldn’t do at all.
There was a problem hiding this comment.
Without JS it gives:
Which seems completely fine to me?
There was a problem hiding this comment.
Also: do we want to keep the text around? It makes sense for notable tooltips but what about these tooltips?
There was a problem hiding this comment.
Yeah,
The only reason the JS needs to exist at all is because Mobile Safari provides no way to see the title tooltip, and Mobile Chrome didn’t used to even provide a way to see the CSS tooltip (it was completely inaccessible on there, AFAIK).
There was a problem hiding this comment.
Oh I see. I forgot about mobile once again... Then can you add GUI test to confirm that the click maintain the tooltip display and then hide the tooltip please?
There was a problem hiding this comment.
Actually, never mind about Mobile Chrome. Now that I test it again, it actually does allow getting at the :hover tooltip. It doesn't provide any reasonable way to see the title= attribute, though.
| ) | ||
| click: ".docblock .example-wrap.ignore .tooltip" | ||
| assert-false: ".popover.tooltip" |
There was a problem hiding this comment.
What do I need to add to the test to finish it? @GuillaumeGomez
There was a problem hiding this comment.
Sorry, I missed this check. Let's just keep the font and background colors checks and ten it'll be all good to me.
There was a problem hiding this comment.
Okay, I pushed a change that adds back the background, text color, and border color checks.
notriddle
force-pushed
the
notriddle/simplify-doctest-tooltip
branch
from
fe696a4
to
7aa4a20
Compare
|
Thanks! @bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#107340 (rustdoc: merge doctest tooltip with notable traits tooltip) - rust-lang#107838 (Introduce `-Zterminal-urls` to use OSC8 for error codes) - rust-lang#107922 (Print disk usage in PGO CI script) - rust-lang#107931 (Intern span when length is MAX_LEN with parent.) - rust-lang#107935 (rustc_ast: Merge impls and reorder methods for attributes and meta items) - rust-lang#107986 (layout: deal with placeholders, ICE on bound types) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes https://discord.com/channels/442252698964721669/443150878111694848/1066420140167680000
a user report where the tooltip arrow overlaps the text
Fixes #91100
Preview: https://notriddle.com/notriddle-rustdoc-demos/simplify-doctest-tooltip/std/vec/struct.Vec.html#indexing
Screenshot: