-
-
Notifications
You must be signed in to change notification settings - Fork 79.1k
Replace PNG with SVG and compress SVGs in custom form controls #17222
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
Conversation
before: https://pp.vk.me/c624325/v624325775/44408/FabMA25FDDs.jpg before(image-rendering: pixelated): https://pp.vk.me/c624325/v624325775/44416/Dzb6tO4Mmys.jpg after: https://pp.vk.me/c624325/v624325775/4440f/imd6KcYBgGI.jpg
Well compressed, but I'm sure it is possible to compress more
It sure feels dirty but also makes changing the color much easier. |
Could it be done with help from glyphicons? |
@YaroslavShapoval We no longer include/use Glyphicons in v4. |
Oh holy shit, no kidding? I missed this when I took a casual glance at this PR the other day. |
@mdo Well, @thekondrashov didn't implement it in the PR yet, he gave this example: input:checked ~ .c-indicator {
background-image: url("data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-4 -4 8 8'%3E%3Ccircle fill='%23fff' r='3'/%3E%3C/svg%3E");
} And since that is just the plain SVG source with a few symbols encoded, it'd be possible to change the |
So what do we do with this PR? SVGs are much better now without old IE support. |
I'm game! |
@thekondrashov: please fetch and rebase against v4-dev. |
Conflicts: scss/_custom-forms.scss scss/_variables.scss
Need to figure out how to balance this with #17476. |
I converted the icons from this PR to ASCII:
and I compared it with the #17476 PR by @efender and here are results:
Also all a hex colors are translated to lowercase (improves compression) in my PR. |
Rad! If all those changes are already part of this pull request, that's great! If not, you can push the changes right to the same branch here and they'll automatically show up here on GitHub. There's also a merge conflict in part of your code here—let me know if you are unsure how to clear that up. |
No, I didn't implement it in the PR yet. It's possible to do it via the web interface of GitHub? |
Commit c972592 is unwanted. |
And more optimizations - http://sassmeister.com/gist/a02447e3bed566e3068c |
How can I fix it? |
I redrew
and I counted the table with the new values by @efender:
Here are results:
Here the best results at the moment:
|
We can fix it on our end when we decide to merge this. |
@cvrebert , now merge conflict fixed? I need something else to do? |
No. This is awaiting further review from @mdo. |
I think I broke something in |
@@ -646,4 +646,4 @@ $kbd-bg: #333 !default; | |||
$pre-bg: #f7f7f9 !default; | |||
$pre-color: $gray-dark !default; | |||
$pre-border-color: #ccc !default; | |||
$pre-scrollable-max-height: 340px !default; | |||
$pre-scrollable-max-height: 340px !default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files should end with a trailing newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an empty line at the end of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I fixed it.
(Sorry, I deleted previous comment because I decided that @houndci is a bot and will not be able to answer me.)
I apologize for my mistakes, I'm new to git. |
Oh, I'm sorry. I have not tested icons from @efender after optimizations. |
I think it is better to specify the size of icons in styles using "background-size", rather than relying on the dimensions svg background-size: auto 1rem; |
Yes, I agree. Need to return changes of the value of the <svg xmlns="http://www.w3.org/2000/svg" viewBox="-1 -1 5 5" fill="#d9534f">
<path stroke="#d9534f" d="M 0 0 l 3 3 M 3 0 l -3 3"/>
<circle r=".5"/>
<circle r=".5" cx="3"/>
<circle r=".5" cy="3"/>
<circle r=".5" cx="3" cy="3"/>
</svg> Compressed to (282 bytes in ascii -vs- 526 bytes of original in base64, the difference is even more with gzip):
|
@efender, I created JSFiddle preview. |
Hi @thekondrashov! You appear to have posted a live example (https://fiddle.jshell.net/kondrashov/ztcLovor/show/light/), which is always a good first step. However, according to the HTML5 validator, your example has some validation errors, which might potentially be causing your issue:
You'll need to fix these errors and post a revised example before we can proceed further. (Please note that this is a fully automated comment.) |
Fixed JSFiddle preview. |
Hi @thekondrashov! You appear to have posted a live example (https://fiddle.jshell.net/kondrashov/ztcLovor/1/show/light/), which is always a good first step. However, according to Bootlint, your example has some Bootstrap usage errors, which might potentially be causing your issue:
You'll need to fix these errors and post a revised example before we can proceed further. (Please note that this is a fully automated comment.) |
So, where are we with this? |
@XhmikosR, sorry for the delay. |
@thekondrashov Wow, that's awesome work! I'm a fan of the |
Replace PNG with SVG
Compress SVGs
Well compressed with SVGO, but I'm sure it is possible to compress more:
(re)drawn by me (with maximum compression, is impossible to compress better 💪):
What do you think about this hack?
Looks dirty, but still smaller. Read more: https://codepen.io/Tigt/blog/optimizing-svgs-in-data-uris