Skip to content

Rustdoc: search: color item type and reduce size to avoid clashing - #112720

Merged
bors merged 2 commits into
rust-lang:masterfrom
poliorcetics:rustdoc-item-type-color-same-as-item-color
Jun 20, 2023
Merged

Rustdoc: search: color item type and reduce size to avoid clashing#112720
bors merged 2 commits into
rust-lang:masterfrom
poliorcetics:rustdoc-item-type-color-same-as-item-color

Conversation

@poliorcetics

@poliorceticspoliorcetics commented Jun 16, 2023

Copy link
Copy Markdown
Contributor
  • rustdoc: search: color item type same as item
  • rustdoc: search: reduce item type size to 0.875rem to avoid clashing with path and item

@rustbot

rustbot commented Jun 16, 2023

Copy link
Copy Markdown
Collaborator

r? @jsha

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

@rustbotrustbot 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 Jun 16, 2023
@rustbot

rustbot commented Jun 16, 2023

Copy link
Copy Markdown
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@poliorcetics

poliorcetics commented Jun 16, 2023

Copy link
Copy Markdown
ContributorAuthor

Current look:

Screenshot 2023-06-16 at 22 31 55

@poliorcetics

poliorcetics commented Jun 16, 2023

Copy link
Copy Markdown
ContributorAuthor

My proposition after #110688, which I find nice but a little too charged and uniform to be truly agreable to use

@GuillaumeGomez

GuillaumeGomez commented Jun 16, 2023

Copy link
Copy Markdown
Member

Having the color put on the type kind isn't great indeed. ^^'

Also, having the text smaller actually attracts the readers attention to the type kind rather than the item name. Not sure if it's the goal?

@poliorcetics

poliorcetics commented Jun 16, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor
Screenshot 2023-06-16 at 23 16 51

I like this much better, the uniform grey make it much less eye-catching and allows concentrating on the searched-for item

@poliorcetics
poliorceticsforce-pushed the rustdoc-item-type-color-same-as-item-color branch from fc6c82e to 3afbfc5CompareJune 16, 2023 21:19
@notriddle

notriddle commented Jun 17, 2023

Copy link
Copy Markdown
Contributor

I think grey is a lot nicer than color in this case.

@GuillaumeGomez

GuillaumeGomez commented Jun 17, 2023

Copy link
Copy Markdown
Member

Agreed.

@GuillaumeGomezGuillaumeGomez mentioned this pull request Jun 17, 2023
Closed
@dns2utf8

dns2utf8 commented Jun 17, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

I think it is still a bit visually noisy while looking clearly better.
Could we align the fully qualified type a bit? Maybe with spaces?

Edit: Or use word that are the same length?

@poliorcetics

poliorcetics commented Jun 17, 2023

Copy link
Copy Markdown
ContributorAuthor

If someone manages to find nice wording that have almost the same size for all items, I'm interested, but I have no idea for some, like trait method or re export

I suppose we could use Rust syntax, like fn instead of method, I'll try it when I'm on my computer

@GuillaumeGomez

GuillaumeGomez commented Jun 17, 2023

Copy link
Copy Markdown
Member

method != fn != trait method. ;)

@notriddle

notriddle commented Jun 17, 2023

Copy link
Copy Markdown
Contributor

fn ⊃ method ⊃ trait method

Calling a trait method a function is accurate. It’s just not maximally specific.

@GuillaumeGomez

GuillaumeGomez commented Jun 17, 2023

Copy link
Copy Markdown
Member

But in the docs we make the difference, hence why I commented it this way.

@Sp00ph

Sp00ph commented Jun 17, 2023

Copy link
Copy Markdown
Member

Would it be possible to add padding to vertically align the item names? I think the fact that the different types have different widths makes the list look more cluttered to me.

@poliorcetics

poliorcetics commented Jun 17, 2023

Copy link
Copy Markdown
ContributorAuthor

It was tried in the original PR and didn't look good, it makes it much harder to find the looked-for item

I personally like it as-is but I'll change it if more people want columns

@GuillaumeGomez

GuillaumeGomez commented Jun 18, 2023

Copy link
Copy Markdown
Member

I think the current display is good too. Can you add a screenshot with "mobile view" too please?

@poliorcetics

poliorcetics commented Jun 18, 2023

Copy link
Copy Markdown
ContributorAuthor

IMG_8710F5302837-1

@GuillaumeGomez

GuillaumeGomez commented Jun 18, 2023

Copy link
Copy Markdown
Member

Please add a GUI test for the color of the type name (in tests/rustdoc-gui). If you need help, don't hesitate to ask me. Once done, I'll r+ it.

@rustbot

rustbot commented Jun 18, 2023

Copy link
Copy Markdown
Collaborator

Some changes occurred in GUI tests.

cc @GuillaumeGomez

@poliorcetics

poliorcetics commented Jun 18, 2023

Copy link
Copy Markdown
ContributorAuthor

Added a test. I'm unsure if I'm correct with it, hopefully I am

GuillaumeGomez
GuillaumeGomez reviewed Jun 19, 2023
View reviewed changes
Comment threadtests/rustdoc-gui/search-result-color.goml Outdated
@poliorcetics
poliorceticsforce-pushed the rustdoc-item-type-color-same-as-item-color branch from cf10b9c to 612c3a1CompareJune 19, 2023 21:34
GuillaumeGomez
GuillaumeGomez reviewed Jun 19, 2023
View reviewed changes
Comment threadtests/rustdoc-gui/search-result-color.goml Outdated
@poliorcetics
poliorceticsforce-pushed the rustdoc-item-type-color-same-as-item-color branch from 612c3a1 to c3cdf85CompareJune 20, 2023 06:29
@GuillaumeGomez

GuillaumeGomez commented Jun 20, 2023

Copy link
Copy Markdown
Member

Thanks!

@bors r=notriddle,GuillaumeGomez rollup

@bors

bors commented Jun 20, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit c3cdf85 has been approved by notriddle,GuillaumeGomez

It is now in the queue for this repository.

@borsbors 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 Jun 20, 2023
@GuillaumeGomezGuillaumeGomez mentioned this pull request Jun 20, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2023
@bors
Auto merge ofrust-lang#112839- GuillaumeGomez:rollup-jc5nqug, r=Gui…
1b6d4cd
…llaumeGomez
Rollup of 6 pull requests
Successful merges:
- rust-lang#112464 (Fix windows `Socket::connect_timeout` overflow)
- rust-lang#112720 (Rustdoc: search: color item type and reduce size to avoid clashing)
- rust-lang#112762 (Sort the errors from arguments checking so that suggestions are handled properly)
- rust-lang#112786 (change binders from tuple structs to named fields)
- rust-lang#112794 (Fix linker failures when #[global_allocator] is used in a dependency)
- rust-lang#112819 (Disable feature(unboxed_closures, fn_traits) in weird-exprs)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit a318824 into rust-lang:masterJun 20, 2023
@rustbotrustbot added this to the 1.72.0 milestone Jun 20, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@notriddlenotriddlenotriddle approved these changes
@GuillaumeGomezGuillaumeGomezGuillaumeGomez approved these changes

Assignees

@jshajsha

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.72.0

Development

Successfully merging this pull request may close these issues.

8 participants

@poliorcetics@rustbot@GuillaumeGomez@notriddle@dns2utf8@Sp00ph@bors@jsha