Skip to content

Use an SVG image for clipboard instead of unicode character - #85118

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:clipboard-svg
May 10, 2021
Merged

Use an SVG image for clipboard instead of unicode character#85118
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:clipboard-svg

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented May 9, 2021

Copy link
Copy Markdown
Member

@GuillaumeGomezGuillaumeGomez 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 May 9, 2021
@rust-highfive

rust-highfive commented May 9, 2021

Copy link
Copy Markdown
Contributor

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 9, 2021
@notriddle

notriddle commented May 9, 2021

Copy link
Copy Markdown
Contributor

fill=currentColor isn’t going to work correctly with the IMG tag.

We should really switch to using an SVG sprite sheet for rustdoc icons, so all three icons can be loaded from one file, and so that we can inherit theme colors.

@GuillaumeGomez

GuillaumeGomez commented May 9, 2021

Copy link
Copy Markdown
MemberAuthor

They currently keep their color, whatever the theme.

@Nemo157

Nemo157 commented May 10, 2021

Copy link
Copy Markdown
Contributor

They currently keep their color, whatever the theme.

That's a problem

image

@Nemo157

Nemo157 commented May 10, 2021

Copy link
Copy Markdown
Contributor

It currently goes back to the old text clipboard icon after clicking (pretend this is a gif, I don't have a gif recorder on this pc 😀):

image
image
image

@GuillaumeGomez

GuillaumeGomez commented May 10, 2021

Copy link
Copy Markdown
MemberAuthor

Oh right! Good catch!

@GuillaumeGomez

GuillaumeGomez commented May 10, 2021

Copy link
Copy Markdown
MemberAuthor

Fixed all the mentionned issues.

@Nemo157

Nemo157 commented May 10, 2021

Copy link
Copy Markdown
Contributor

Looks good enough to start, @notriddle do you wanna open an issue about maybe moving to an SVG sprite sheet? I guess the major thing that would need checking is browser compatibility.

@bors r+

@bors

bors commented May 10, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 7450f70 has been approved by Nemo157

@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 May 10, 2021
@GuillaumeGomezGuillaumeGomez mentioned this pull request May 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2021
@bors
Auto merge ofrust-lang#85156- GuillaumeGomez:rollup-8u4h34g, r=Guil…
6fd7a6d
…laumeGomez
Rollup of 4 pull requests
Successful merges:
- rust-lang#84465 (rustdoc: Implement `is_primitive` in terms of `primitive_type()`)
- rust-lang#85118 (Use an SVG image for clipboard instead of unicode character)
- rust-lang#85148 (Fix source code line number display and make it clickable again)
- rust-lang#85152 (Adjust target search algorithm for rustlib path)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit c7e7de4 into rust-lang:masterMay 10, 2021
@rustbotrustbot added this to the 1.54.0 milestone May 10, 2021
@GuillaumeGomez
GuillaumeGomez deleted the clipboard-svg branch May 10, 2021 21:16
@camelid

camelid commented May 11, 2021

Copy link
Copy Markdown
Member

This made the clipboard icon much larger and more distracting:

before

image

after

image

I felt concerned about it being too conspicuous before, but now it's even more so. Is there a way to make it smaller or lighter?

The docs.rs one is only shown for the crate root, not for every single item in the crate.

@GuillaumeGomez

GuillaumeGomez commented May 11, 2021

Copy link
Copy Markdown
MemberAuthor

I voluntarily made it bigger actually. XD

@camelid

camelid commented May 11, 2021

Copy link
Copy Markdown
Member

So could you make it smaller again? ;)

@Nemo157

Nemo157 commented May 11, 2021

Copy link
Copy Markdown
Contributor

Hmm, I don't see such a small icon on older docs, it looks like it's using system fonts so depends on what font is selected.

image

That said the new icon definitely is a bit bigger, I kinda like something like dropping the border and reducing the opacity (with some kind of hover state). Having an inline "button" border looks quite weird to me.

image

image

@GuillaumeGomez

GuillaumeGomez commented May 11, 2021

Copy link
Copy Markdown
MemberAuthor

Hum... Maybe @jsha has an opinion here? I personally don't mind changing to any of what was suggested so I'm definitely not the good person to pick one.

@jsha

jsha commented May 12, 2021

Copy link
Copy Markdown
Contributor

I like @Nemo157's proposal with the smallish, unobtrusive icon.

@GuillaumeGomez

GuillaumeGomez commented May 17, 2021

Copy link
Copy Markdown
MemberAuthor

Giving it a try then!

@GuillaumeGomezGuillaumeGomez mentioned this pull request May 17, 2021
Merged
RalfJung added a commit to RalfJung/rust that referenced this pull request May 18, 2021
@RalfJung
Rollup merge ofrust-lang#85407- GuillaumeGomez:copy-path-display-im…
6e05a71
…provement, r=jsha
Improve display for "copy-path" button, making it more discreet
As suggested by `@Nemo157` [here](rust-lang#85118 (comment)):
![Screenshot from 2021-05-18 11-18-49](https://user-images.githubusercontent.com/3050060/118626882-899ddb00-b7cb-11eb-84c5-ef68dee5e351.png)
![Screenshot from 2021-05-18 11-15-06](https://user-images.githubusercontent.com/3050060/118626880-899ddb00-b7cb-11eb-95c1-2f3bacd22374.png)
![Screenshot from 2021-05-18 11-18-54](https://user-images.githubusercontent.com/3050060/118626885-8a367180-b7cb-11eb-870c-7acdf4259bef.png)
![Screenshot from 2021-05-18 11-15-00](https://user-images.githubusercontent.com/3050060/118626877-89054480-b7cb-11eb-9efc-9a0f153964ce.png)
![Screenshot from 2021-05-18 11-18-58](https://user-images.githubusercontent.com/3050060/118626888-8a367180-b7cb-11eb-91bb-47867cd0f2cc.png)
![Screenshot from 2021-05-18 11-19-00](https://user-images.githubusercontent.com/3050060/118626891-8acf0800-b7cb-11eb-9e3d-d2937b361524.png)
r? `@jsha`
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Nemo157Nemo157

Labels

A-rustdoc-uiArea: Rustdoc UI (generated HTML)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.54.0

Development

Successfully merging this pull request may close these issues.

8 participants

@GuillaumeGomez@rust-highfive@notriddle@Nemo157@bors@camelid@jsha@rustbot