Skip to content

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 14, 2023

Currently when we hover the crate name, the background is stuck to the version and to the logo (when there is one):

Screenshot from 2023-11-14 11-42-39
Screenshot from 2023-11-14 11-43-19

I find it very unpleasant so I reduced the padding size and increased the margin (left and top) to keep the same positioning but not making it stuck anymore:

Screenshot from 2024-01-29 20-40-11
Screenshot from 2024-01-29 20-48-01

online docs

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 14, 2023
@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2023

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@notriddle
Copy link
Contributor

It was done that way so that it would align with the search bar.

#115948 (comment)

How does this commit address that?

@GuillaumeGomez
Copy link
Member Author

Including the search bar:

Screenshot from 2023-11-14 14-20-34
Screenshot from 2023-11-14 14-20-45

Although I'm curious about why we want it to be aligned (should be GUI tested too ;) ).

@GuillaumeGomez
Copy link
Member Author

Also if using more screen space is okay, I can simply increase margin-top on the version (but still keep the left padding/margin update on the crate name).

@GuillaumeGomez
Copy link
Member Author

So what do you want to do here @notriddle ?

@notriddle
Copy link
Contributor

I’ll approve if @fmease thinks it looks good.

@fmease
Copy link
Member

fmease commented Jan 22, 2024

Oddly enough, having built rustdoc locally with this patch applied, it looks totally different from Guillaume's screenshots here: #117906 (comment). The backdrop still aligns with the the search bar:

rustdoc_padded_but_still_aligned

And I've found a slight debatable regression (which you can hopefully reproduce):

master #117906
rustdoc_regression_master rustdoc_regression_patch
symmetric padding asymmetric padding

Not blocking though.

So I can neither approve nor disapprove atm ^^'


Mozilla Firefox 121.0
Linux Arch 6.6.10

@GuillaumeGomez
Copy link
Member Author

This PR is a bit old. I'm going to rebase and update whatever needs to be updated then I'll post new screenshots and an online demo.

@GuillaumeGomez GuillaumeGomez force-pushed the improve-crate-name-hover branch from 1a9544e to 96e6cfa Compare January 29, 2024 19:41
@GuillaumeGomez
Copy link
Member Author

Updated screenshots, code and added link to online docs so you can test it yourself. :)

@fmease
Copy link
Member

fmease commented Jan 30, 2024

CC #120471, could be addressed in this PR, too, or in a follow-up.

In the latter case, r=notriddle,fmease on this PR I suppose.

@GuillaumeGomez
Copy link
Member Author

Different case. I'll take a look later on. Thank you both for the reviews!

@bors r=fmease,notriddle

@bors
Copy link
Collaborator

bors commented Jan 30, 2024

📌 Commit 96e6cfa has been approved by fmease,notriddle

It is now in the queue for this repository.

@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 Jan 30, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2024
…llaumeGomez

Rollup of 11 pull requests

Successful merges:

 - rust-lang#117906 (Improve display of crate name when hovered)
 - rust-lang#118533 (Suppress unhelpful diagnostics for unresolved top level attributes)
 - rust-lang#120293 (Deduplicate more sized errors on call exprs)
 - rust-lang#120295 (Remove `raw_os_nonzero` feature.)
 - rust-lang#120310 (adapt test for v0 symbol mangling)
 - rust-lang#120342 (Remove various `has_errors` or `err_count` uses)
 - rust-lang#120434 (Revert outdated version of "Add the wasm32-wasi-preview2 target")
 - rust-lang#120445 (Fix some `Arc` allocator leaks)
 - rust-lang#120475 (Improve error message when `cargo build` is used to build the compiler)
 - rust-lang#120476 (Remove some unnecessary check logic for lang items in HIR typeck)
 - rust-lang#120485 (add missing potential_query_instability for keys and values in hashmap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit efff267 into rust-lang:master Jan 30, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2024
Rollup merge of rust-lang#117906 - GuillaumeGomez:improve-crate-name-hover, r=fmease,notriddle

Improve display of crate name when hovered

Currently when we hover the crate name, the background is stuck to the version and to the logo (when there is one):

![Screenshot from 2023-11-14 11-42-39](https://github.com/rust-lang/rust/assets/3050060/717190cd-483d-45a1-a462-e9ba342d4376)
![Screenshot from 2023-11-14 11-43-19](https://github.com/rust-lang/rust/assets/3050060/23f8bc9b-1304-4f91-ae1e-96bc508e9da6)

I find it very unpleasant so I reduced the padding size and increased the margin (left and top) to keep the same positioning but not making it stuck anymore:

![Screenshot from 2024-01-29 20-40-11](https://github.com/rust-lang/rust/assets/3050060/82cf266c-7f99-4a46-a62b-ebe2445f52be)
![Screenshot from 2024-01-29 20-48-01](https://github.com/rust-lang/rust/assets/3050060/e7097c10-6e09-4bdc-a37f-070b6dac671d)

[online docs](https://rustdoc.crud.net/imperio/improve-crate-name-hover/std/index.html)

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez deleted the improve-crate-name-hover branch August 19, 2024 12:20
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.

5 participants