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.

Sign up for GitHub

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

Jump to bottom

rustdoc: make icons more consistent #91735

Closed
wants to merge 1 commit into from
Closed

rustdoc: make icons more consistent #91735

wants to merge 1 commit into from

Conversation

pitaj
Copy link
Contributor

@pitaj pitaj commented Dec 10, 2021 •
edited
Loading

Close #91309

  • Change to solid Font Awesome icons to match the style of docs.rs and crates.io
Description Old New Before After
arrow for <selects> down-arrow.svg caret-down.svg image image
settings button gear.svg cog.svg image image
unsafe ⚠ exclamation-triangle.svg image image
experimental 🔬 flask.svg image image
info tooltips ⓘ info-circle.svg image image
deprecated 👎 thumbs-down.svg image image
  • All SVG icons were bundled into a custom font icons
    (those listed above plus clipboard.svg, toggle-minus.svg, toggle-plus.svg)
    This allows for easily setting the icon color, saves network requests, improves fallback behavior,
    and removes some root_path/suffix handling in templates
  • Move information tooltip into example-wrap for pure CSS hover effects
  • Modify toggle-plus.svg and toggle-minus.svg: minified and empty margin removed
    to be compatible with icon font
  • Pure CSS code example hover effects

clipboard.svg was unchanged, besides being bundled into the icon font.
Included instruction for recreating the icon font should that be necessary in the future.

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 10, 2021
@GuillaumeGomez
Copy link
Member

Thanks, looks nice! Please fix the conflict and then ping me once done so I can review. ;)

@camelid
Copy link
Member

camelid commented Dec 10, 2021

This looks nice! Two comments:

  • I prefer the old rendering of ⚠️. The new one looks really faint.
  • The arrow next to "All crates" seems a little too thick for the text font.

@pitaj

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Dec 11, 2021
@pitaj
Copy link
Contributor Author

pitaj commented Dec 11, 2021

@GuillaumeGomez the ping your requested

@pitaj
Copy link
Contributor Author

pitaj commented Dec 11, 2021

@GuillaumeGomez I have refactored the icons to use a custom webfont. Works very well.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez mentioned this pull request Dec 12, 2021
Closed
@pitaj
rustdoc: more consistent icons
6c6adfe 
- Change to solid Font Awesome icons to match the style of docs.rs and crates.io
  + `down-arrow.svg` (`<select>`s) -> `caret-down.svg`
  + `gear.svg` (settings button) -> `cog.svg`
  + warning triangle ⚠ (unsafe) -> `exclamation-triangle.svg`
  + microscope 🔬 (experimental) -> `flask.svg`
  + info ⓘ (information tooltips) -> `info-circle.svg`
  + `brush.svg` (theme button) -> `paint-brush.svg` [these turned out to be the same icon]
  + thumbs down 👎 (deprecated) -> `thumbs-down.svg`
- All SVG icons were bundled into a custom font `icons`
  (those listed above plus `clipboard.svg`, `toggle-minus.svg`, `toggle-plus.svg`)
  This allows for easily setting the icon color, saves network requests, improves fallback behavior,
  and removes some `root_path`/`suffix` handling in templates
- Move information tooltip into example-wrap for pure CSS hover effects
- Modify `toggle-plus.svg` and `toggle-minus.svg`: minified and empty margin removed
  to be compatible with icon font
- Pure CSS code example hover effects
@pitaj pitaj marked this pull request as ready for review December 14, 2021 04:10
@pitaj
Copy link
Contributor Author

pitaj commented Dec 14, 2021

@GuillaumeGomez ready for review

@GuillaumeGomez
Copy link
Member

Code looks good to me. I'll test it locally as soon as possible. Having everything in a font directly instead of downloading multiple images seems like a great improvement!

In the meantime: what do you think about this @jsha?

@GuillaumeGomez
Copy link
Member

For anyone interested, I uploaded the generated std docs here.

@GuillaumeGomez
Copy link
Member

Looks great to me, thanks!

Considering it's a big PR, I just want to ensure I didn't miss something. cc @camelid @jsha

@camelid
Copy link
Member

camelid commented Dec 15, 2021

Hmm, I think the new (i) icon attracts too much attention:

image

It also seems somewhat non-optimal to rely on a third-party web service to generate the icon font file. Is there precedent for this?

@GuillaumeGomez
Copy link
Member

I didn't see it as a hard blocker but it could be problematic if we don't have an (opensource) alternative.

@pitaj
Copy link
Contributor Author

pitaj commented Dec 16, 2021

Fontello is open source licensed MIT: https://github.com/fontello/fontello

And various alternatives exist. I'm sure the creation of the font could even be automated (using fontello API or some other utility), but given how rarely I expect the font to be modified, I doubt it's worth the maintenance burden.

@GuillaumeGomez
Copy link
Member

Still something we need to approve as a team. I opened a discussion on zulip here.

@camelid camelid added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 17, 2021
@bors
Copy link
Contributor

bors commented Dec 19, 2021

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

@camelid camelid mentioned this pull request Mar 23, 2022
Merged
@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jul 24, 2022
@JohnCSimon
Copy link
Member

@pitaj
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Nov 27, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make rustdoc icons more consistent
9 participants
@pitaj @rust-highfive @GuillaumeGomez @camelid @rust-log-analyzer @bors @JohnCSimon @Mark-Simulacrum @rustbot