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.

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

Inline more svg images into CSS #123734

Merged
merged 3 commits into from
Apr 28, 2024
Merged

Conversation

GuillaumeGomez
Copy link
Member

Follow-up of #123706.

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 Apr 10, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 10, 2024

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rust-log-analyzer

This comment has been minimized.

Comment on lines 1613 to 1642
content: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="22" height="22" \
enable-background="new 0 0 22 22" viewBox="0 0 27.434 29.5">\
<path d="M27.316 18.39a2.696 2.696 0 0 0-.98-1.46 1.62 1.62 0 0 1-.016-.762l.035-.176v-1.191c0-1.246-\
.003-1.278-.046-1.473a1.717 1.717 0 0 1 .007-.805c.477-.343.829-.859.997-1.472.257-.957.074-2.094-.508-\
3.117l-.594-1.032c-.746-1.304-1.965-2.117-3.18-2.117-.379 0-.75.078-1.086.235a1.958 1.958 0 0 1-.855-\
.391l-.102-.082-.117-.063-1.855-1.07-.094-.055-.106-.043c-.378-.156-.66-.41-.77-.554C17.919 1.172 16.349 \
0 14.297 0h-1.155c-2.043 0-3.61 1.152-3.75 2.723-.114.14-.391.382-.758.527l-.102.04-.094.05-1.94 1.066-\
.134.074-.117.094a2.019 2.019 0 0 1-.832.403 2.518 2.518 0 0 0-1.008-.211c-1.199 0-2.414.82-3.168 2.14l-\
.59 1.032c-.41.718-.64 1.523-.64 2.257-.004.953.36 1.758 1.012 2.258.035.152.058.445-.016.785-.04.168-\
.063.282-.063 1.563 0 1.148 0 1.148.016 1.261l.008.075.015.074c.075.344.047.64.012.8-.644.5-1.004 1.302-\
.992 2.259.008.726.238 1.52.648 2.242l.59 1.027c.758 1.332 1.965 2.16 3.149 2.16.324 0 .644-.062.937-\
.187.168.039.492.156.813.418l.11.086.124.07 2.047 1.156.102.059.105.043c.363.144.648.379.766.52.164 1.519 \
1.718 2.632 3.746 2.632h1.156c2.035 0 3.598-1.133 3.746-2.672.117-.144.402-.394.773-.55l.114-.047.101-.063 \
1.961-1.156.106-.063.097-.078c.309-.246.653-.37.832-.398.313.136.66.21 1.016.21 1.2 0 2.41-.82 3.164-2.14l\
.594-1.031c.59-1.028.777-2.164.52-3.117Zm-2.043 2.247-.59 1.031c-.437.766-1.105 1.25-1.636 1.25a.7.7 0 0 1-\
.371-.094 1.146 1.146 0 0 0-.567-.129c-.593 0-1.382.297-2.007.797l-1.961 1.156c-1.016.426-1.848 1.293-1.848 \
1.93 0 .64-.898 1.16-1.996 1.16H13.14c-1.102 0-2-.515-2-1.14 0-.63-.832-1.477-1.852-1.887l-2.047-1.16c-.637-\
.512-1.426-.813-2.008-.813-.199 0-.379.035-.515.114a.648.648 0 0 1-.332.085c-.52 0-1.18-.5-1.621-1.273l-.59-\
1.031c-.543-.953-.555-1.98-.024-2.285.532-.305.782-1.434.551-2.504V14.8c0-1.09.02-1.18.02-1.18.238-1.074-\
.008-2.203-.551-2.516-.54-.304-.54-1.34.008-2.293l.59-1.03c.437-.766 1.101-1.255 1.636-1.255a.73.73 0 0 1 \
.364.094c.152.086.343.125.566.125.594 0 1.379-.297 2.004-.793l1.945-1.066c1.02-.407 1.856-1.278 1.856-1.934 \
0-.656.898-1.191 2-1.191h1.156c1.098 0 1.996.543 1.996 1.21 0 .669.832 1.555 1.848 1.973L20 6.012c.617.492 \
1.402.777 2.012.777.242 0 .453-.047.62-.14a.79.79 0 0 1 .403-.102c.55 0 1.223.476 1.652 1.23l.59 1.032c\
.543.953.52 2.004-.062 2.336-.574.332-.86 1.48-.625 2.554 0 0 .008.04.008 1.102v1.011c-.215 1.051.07 2.176.636 \
2.5.567.325.586 1.368.04 2.325Zm0 0"/><path d="M13.61 7.61a7.084 7.084 0 0 0-7.083 7.085 7.085 7.085 0 1 0 \
14.168 0A7.088 7.088 0 0 0 13.61 7.61Zm0 12.41a5.33 5.33 0 0 1-5.325-5.325 5.33 5.33 0 0 1 5.324-5.32 5.327 \
5.327 0 0 1 5.325 5.32 5.328 5.328 0 0 1-5.325 5.325Zm0 0"/>\
<path d="M13.684 9.906a4.722 4.722 0 0 0-4.72 4.719 4.722 4.722 0 0 0 4.72 4.719 4.724 4.724 0 0 0 4.714-4.719 \
4.724 4.724 0 0 0-4.714-4.719Zm0 7.676a2.954 2.954 0 1 1 0-5.91 2.953 2.953 0 0 1 2.953 2.953 2.957 2.957 0 \
0 1-2.953 2.957Zm0 0"/></svg>');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When writing #123706, I considered inlining these SVGs, but decided not to because of this.

This graphic is infeasible to edit by hand. The only way to modify it is to copy it out of the CSS and open it in inkscape, which is a terrible devex because you also have to remove the backslashes and fix any escaped characters. Inlined into the CSS, this "path soup" might as well be a binary blob. That's why every other inlined SVG has been five lines or less.

If you really want to inline these SVGs, could you use a geometrically simpler graphic?

Copy link
Contributor

@notriddle notriddle Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.svgrepo.com/svg/421570/cog-gear claims to be CC0, and is a good example of what I'm thinking. It's still uncomfortably long, but shorter than the one being used now.

Suggested change
content: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="22" height="22" \
enable-background="new 0 0 22 22" viewBox="0 0 27.434 29.5">\
<path d="M27.316 18.39a2.696 2.696 0 0 0-.98-1.46 1.62 1.62 0 0 1-.016-.762l.035-.176v-1.191c0-1.246-\
.003-1.278-.046-1.473a1.717 1.717 0 0 1 .007-.805c.477-.343.829-.859.997-1.472.257-.957.074-2.094-.508-\
3.117l-.594-1.032c-.746-1.304-1.965-2.117-3.18-2.117-.379 0-.75.078-1.086.235a1.958 1.958 0 0 1-.855-\
.391l-.102-.082-.117-.063-1.855-1.07-.094-.055-.106-.043c-.378-.156-.66-.41-.77-.554C17.919 1.172 16.349 \
0 14.297 0h-1.155c-2.043 0-3.61 1.152-3.75 2.723-.114.14-.391.382-.758.527l-.102.04-.094.05-1.94 1.066-\
.134.074-.117.094a2.019 2.019 0 0 1-.832.403 2.518 2.518 0 0 0-1.008-.211c-1.199 0-2.414.82-3.168 2.14l-\
.59 1.032c-.41.718-.64 1.523-.64 2.257-.004.953.36 1.758 1.012 2.258.035.152.058.445-.016.785-.04.168-\
.063.282-.063 1.563 0 1.148 0 1.148.016 1.261l.008.075.015.074c.075.344.047.64.012.8-.644.5-1.004 1.302-\
.992 2.259.008.726.238 1.52.648 2.242l.59 1.027c.758 1.332 1.965 2.16 3.149 2.16.324 0 .644-.062.937-\
.187.168.039.492.156.813.418l.11.086.124.07 2.047 1.156.102.059.105.043c.363.144.648.379.766.52.164 1.519 \
1.718 2.632 3.746 2.632h1.156c2.035 0 3.598-1.133 3.746-2.672.117-.144.402-.394.773-.55l.114-.047.101-.063 \
1.961-1.156.106-.063.097-.078c.309-.246.653-.37.832-.398.313.136.66.21 1.016.21 1.2 0 2.41-.82 3.164-2.14l\
.594-1.031c.59-1.028.777-2.164.52-3.117Zm-2.043 2.247-.59 1.031c-.437.766-1.105 1.25-1.636 1.25a.7.7 0 0 1-\
.371-.094 1.146 1.146 0 0 0-.567-.129c-.593 0-1.382.297-2.007.797l-1.961 1.156c-1.016.426-1.848 1.293-1.848 \
1.93 0 .64-.898 1.16-1.996 1.16H13.14c-1.102 0-2-.515-2-1.14 0-.63-.832-1.477-1.852-1.887l-2.047-1.16c-.637-\
.512-1.426-.813-2.008-.813-.199 0-.379.035-.515.114a.648.648 0 0 1-.332.085c-.52 0-1.18-.5-1.621-1.273l-.59-\
1.031c-.543-.953-.555-1.98-.024-2.285.532-.305.782-1.434.551-2.504V14.8c0-1.09.02-1.18.02-1.18.238-1.074-\
.008-2.203-.551-2.516-.54-.304-.54-1.34.008-2.293l.59-1.03c.437-.766 1.101-1.255 1.636-1.255a.73.73 0 0 1 \
.364.094c.152.086.343.125.566.125.594 0 1.379-.297 2.004-.793l1.945-1.066c1.02-.407 1.856-1.278 1.856-1.934 \
0-.656.898-1.191 2-1.191h1.156c1.098 0 1.996.543 1.996 1.21 0 .669.832 1.555 1.848 1.973L20 6.012c.617.492 \
1.402.777 2.012.777.242 0 .453-.047.62-.14a.79.79 0 0 1 .403-.102c.55 0 1.223.476 1.652 1.23l.59 1.032c\
.543.953.52 2.004-.062 2.336-.574.332-.86 1.48-.625 2.554 0 0 .008.04.008 1.102v1.011c-.215 1.051.07 2.176.636 \
2.5.567.325.586 1.368.04 2.325Zm0 0"/><path d="M13.61 7.61a7.084 7.084 0 0 0-7.083 7.085 7.085 7.085 0 1 0 \
14.168 0A7.088 7.088 0 0 0 13.61 7.61Zm0 12.41a5.33 5.33 0 0 1-5.325-5.325 5.33 5.33 0 0 1 5.324-5.32 5.327 \
5.327 0 0 1 5.325 5.32 5.328 5.328 0 0 1-5.325 5.325Zm0 0"/>\
<path d="M13.684 9.906a4.722 4.722 0 0 0-4.72 4.719 4.722 4.722 0 0 0 4.72 4.719 4.724 4.724 0 0 0 4.714-4.719 \
4.724 4.724 0 0 0-4.714-4.719Zm0 7.676a2.954 2.954 0 1 1 0-5.91 2.953 2.953 0 0 1 2.953 2.953 2.957 2.957 0 \
0 1-2.953 2.957Zm0 0"/></svg>');
content: url('data:image/svg+xml,<svg width="22" height="22" viewBox="0 0 12 12" \
enable-background="new 0 0 12 12" xmlns="http://www.w3.org/2000/svg">\
<path d="M10.25,6c0-0.1243286-0.0261841-0.241333-0.0366211-0.362915l1.6077881-1.5545654l\
-1.25-2.1650391 c0,0-1.2674561,0.3625488-2.1323853,0.6099854c-0.2034912-0.1431885-0.421875\
-0.2639771-0.6494751-0.3701782L7.25,0h-2.5 c0,0-0.3214111,1.2857666-0.5393066,2.1572876\
C3.9830933,2.2634888,3.7647095,2.3842773,3.5612183,2.5274658L1.428833,1.9174805 \
l-1.25,2.1650391c0,0,0.9641113,0.9321899,1.6077881,1.5545654C1.7761841,5.758667,\
1.75,5.8756714,1.75,6 s0.0261841,0.241333,0.0366211,0.362915L0.178833,7.9174805l1.25,\
2.1650391l2.1323853-0.6099854 c0.2034912,0.1432495,0.421875,0.2639771,0.6494751,0.3701782\
L4.75,12h2.5l0.5393066-2.1572876 c0.2276001-0.1062012,0.4459839-0.2269287,0.6494751\
-0.3701782l2.1323853,0.6099854l1.25-2.1650391L10.2133789,6.362915 C10.2238159,6.241333,\
10.25,6.1243286,10.25,6z M6,7.5C5.1715698,7.5,4.5,6.8284302,4.5,6S5.1715698,4.5,6,4.5S7.5\
,5.1715698,7.5,6 S6.8284302,7.5,6,7.5z" fill="black"/></svg>');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.svgrepo.com/svg/384069/settings-cog-gear is even shorter, but not as nice...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine as this point. Just one thought: even though we don't need to link back, should we still?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Put a comment in the CSS file pointing at the source. That way, nobody ever asks in the future whether we're complying with the copyright of the icons (or, at least, made a good-faith effort to do so).

For the record:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

I drew the hamburger-button, sidebar, topbar, folder, and checkmark icons myself (and license them the same way as any other contribution)

Wow, nice!

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the inline-svg-into-css branch 2 times, most recently from ad3ec01 to 38734be Compare April 11, 2024 08:56
@GuillaumeGomez
Copy link
Member Author

Added comments and fixed tidy.

@notriddle
Copy link
Contributor

Since it’s replacing the wheel icon, this should go through fcp?

@GuillaumeGomez
Copy link
Member Author

Let's go for an FCP then!

In this PR, we replace the cog image with a simpler one (in term of SVG content) so we can inline it into the CSS directly. The new cog looks like this.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Apr 16, 2024

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 16, 2024
@bors
Copy link
Contributor

bors commented Apr 18, 2024

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

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Apr 18, 2024
@rfcbot
Copy link

rfcbot commented Apr 18, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Apr 18, 2024
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 28, 2024
@rfcbot
Copy link

rfcbot commented Apr 28, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@GuillaumeGomez
Copy link
Member Author

@bors r=notriddle rollup

@bors
Copy link
Contributor

bors commented Apr 28, 2024

📌 Commit 5a3509c has been approved by 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 Apr 28, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2024
…iaskrgr

Rollup of 2 pull requests

Successful merges:

 - rust-lang#123734 (Inline more svg images into CSS)
 - rust-lang#124473 (Port `print-cfg` run-make test to Rust-based rmake.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1b567de into rust-lang:master Apr 28, 2024
10 checks passed
@rustbot rustbot added this to the 1.80.0 milestone Apr 28, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2024
Rollup merge of rust-lang#123734 - GuillaumeGomez:inline-svg-into-css, r=notriddle

Inline more svg images into CSS

Follow-up of rust-lang#123706.

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez deleted the inline-svg-into-css branch April 29, 2024 08:51
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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.

7 participants