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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace PNG with SVG and compress SVGs in custom form controls #17222

Merged
merged 8 commits into from Feb 7, 2016

Conversation

Projects
None yet
10 participants
@thekondrashov
Contributor

thekondrashov commented Aug 22, 2015

Replace PNG with SVG

Preview (scale 500%)
before / before (pixelated) / after Preview Preview Preview

Compress SVGs

Well compressed with SVGO, but I'm sure it is possible to compress more:

  • form-icon-success
  • form-icon-warning
  • form-icon-error
  • input:checked icon

(re)drawn by me (with maximum compression, is impossible to compress better 馃挭):

  • c-select
  • input:indeterminate icon
  • input:checked icon

What do you think about this hack?

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");
}

Looks dirty, but still smaller. Read more: https://codepen.io/Tigt/blog/optimizing-svgs-in-data-uris

@cvrebert cvrebert added css v4 labels Aug 22, 2015

@cvrebert cvrebert changed the title from Replace PNG to SVG and compress SVGs to Replace PNG with SVG and compress SVGs in custom form controls Aug 22, 2015

@hnrch02

This comment has been minimized.

Show comment
Hide comment
@hnrch02

hnrch02 Aug 22, 2015

Member

What do you think about this hack?

It sure feels dirty but also makes changing the color much easier.

Member

hnrch02 commented Aug 22, 2015

What do you think about this hack?

It sure feels dirty but also makes changing the color much easier.

@YaroslavShapoval

This comment has been minimized.

Show comment
Hide comment
@YaroslavShapoval

YaroslavShapoval Aug 22, 2015

Could it be done with help from glyphicons?

Could it be done with help from glyphicons?

@cvrebert

This comment has been minimized.

Show comment
Hide comment
@cvrebert

cvrebert Aug 22, 2015

Member

@YaroslavShapoval We no longer include/use Glyphicons in v4.

Member

cvrebert commented Aug 22, 2015

@YaroslavShapoval We no longer include/use Glyphicons in v4.

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Aug 23, 2015

Member

It sure feels dirty but also makes changing the color much easier.

Oh holy shit, no kidding? I missed this when I took a casual glance at this PR the other day.

Member

mdo commented Aug 23, 2015

It sure feels dirty but also makes changing the color much easier.

Oh holy shit, no kidding? I missed this when I took a casual glance at this PR the other day.

@hnrch02

This comment has been minimized.

Show comment
Hide comment
@hnrch02

hnrch02 Aug 23, 2015

Member

@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 fill property programmatically - not at run time though but at compile time - with a Sass variable.

Member

hnrch02 commented Aug 23, 2015

@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 fill property programmatically - not at run time though but at compile time - with a Sass variable.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 5, 2015

Member

So what do we do with this PR? SVGs are much better now without old IE support.

Member

XhmikosR commented Sep 5, 2015

So what do we do with this PR? SVGs are much better now without old IE support.

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Sep 6, 2015

Member

I'm game!

Member

mdo commented Sep 6, 2015

I'm game!

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 6, 2015

Member

@thekondrashov: please fetch and rebase against v4-dev.

Member

XhmikosR commented Sep 6, 2015

@thekondrashov: please fetch and rebase against v4-dev.

Merge remote-tracking branch 'twbs/v4-dev' into patch-1
Conflicts:
	scss/_custom-forms.scss
	scss/_variables.scss

@mdo mdo referenced this pull request Sep 22, 2015

Closed

Issues adjusting colors of SVG #60

@mdo mdo added this to the v4.0.0-alpha.2 milestone Nov 13, 2015

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Nov 13, 2015

Member

Need to figure out how to balance this with #17476.

Member

mdo commented Nov 13, 2015

Need to figure out how to balance this with #17476.

@mdo mdo added the on-hold label Nov 13, 2015

@mdo mdo referenced this pull request Nov 13, 2015

Closed

inline svg: base64 > ASCII85 #17476

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Nov 15, 2015

Contributor

I converted the icons from this PR to ASCII:

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M6.4 1l-.7.7-2.8 2.8-.8-.8-.7-.7L0 4.4l.7.7 1.5 1.5.7.7.7-.7 3.5-3.5.7-.7L6.4 1z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M0 3v2h8V3z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-4 -4 8 8'%3E%3Ccircle r='3' fill='%23fff'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 5'%3E%3Cpath fill='%23333' d='M2 0L0 2h4zm0 5L0 3h4z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%235cb85c' d='M233.8 610c-13.3 0-26-6-34-16.8L90.5 448.8C76.3 430 80 403.3 98.8 389c18.8-14.2 45.5-10.4 59.8 8.4l72 95L451.3 242c12.5-20 38.8-26.2 58.8-13.7 20 12.4 26 38.7 13.7 58.8L270 590c-7.4 12-20.2 19.4-34.3 20h-2z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%23d9534f' d='M447 544.4c-14.4 14.4-37.6 14.4-52 0l-89-92.7-89 92.7c-14.5 14.4-37.7 14.4-52 0-14.4-14.4-14.4-37.6 0-52l92.4-96.3-92.4-96.3c-14.4-14.4-14.4-37.6 0-52s37.6-14.3 52 0l89 92.8 89.2-92.7c14.4-14.4 37.6-14.4 52 0 14.3 14.4 14.3 37.6 0 52L354.6 396l92.4 96.4c14.4 14.4 14.4 37.6 0 52z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%23f0ad4e' d='M603 640.2l-278.5-509c-3.8-6.6-10.8-10.6-18.5-10.6s-14.7 4-18.5 10.6L9 640.2c-3.7 6.5-3.6 14.4.2 20.8 3.8 6.5 10.8 10.4 18.3 10.4h557c7.6 0 14.6-4 18.4-10.4 3.5-6.4 3.6-14.4 0-20.8zm-266.4-30h-61.2V549h61.2v61.2zm0-107h-61.2V304h61.2v199z'/%3E%3C/svg%3E

and I compared it with the #17476 PR by @efender and here are results:

Icon name by @efender in plain bytes by me in plain bytes winner reduce size even more?
input:checked ~ .c-indicator (checkbox) 214 214 equally yes, need an svg expert
input:indeterminate ~ .c-indicator 168 145 i am (-13.7%) probably yes, but it's hard
input:checked ~ .c-indicator (radio) 185 139 i am (-24.7%) no, 139 is the absolute minimum
.c-select (png icon) not represented 156 i am no, 156 is the absolute minimum
form-icon-success 365 347 i am (-4.9%) yes, need an svg expert
form-icon-error 441 420 i am (-4.8%) yes, need an svg expert
form-icon-warning 391 379 i am (-3.1%) yes, need an svg expert

Also all a hex colors are translated to lowercase (improves compression) in my PR.
How can I help? Sorry, I'm new to git and I don't know how to use git for this.

Contributor

thekondrashov commented Nov 15, 2015

I converted the icons from this PR to ASCII:

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M6.4 1l-.7.7-2.8 2.8-.8-.8-.7-.7L0 4.4l.7.7 1.5 1.5.7.7.7-.7 3.5-3.5.7-.7L6.4 1z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M0 3v2h8V3z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-4 -4 8 8'%3E%3Ccircle r='3' fill='%23fff'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 5'%3E%3Cpath fill='%23333' d='M2 0L0 2h4zm0 5L0 3h4z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%235cb85c' d='M233.8 610c-13.3 0-26-6-34-16.8L90.5 448.8C76.3 430 80 403.3 98.8 389c18.8-14.2 45.5-10.4 59.8 8.4l72 95L451.3 242c12.5-20 38.8-26.2 58.8-13.7 20 12.4 26 38.7 13.7 58.8L270 590c-7.4 12-20.2 19.4-34.3 20h-2z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%23d9534f' d='M447 544.4c-14.4 14.4-37.6 14.4-52 0l-89-92.7-89 92.7c-14.5 14.4-37.7 14.4-52 0-14.4-14.4-14.4-37.6 0-52l92.4-96.3-92.4-96.3c-14.4-14.4-14.4-37.6 0-52s37.6-14.3 52 0l89 92.8 89.2-92.7c14.4-14.4 37.6-14.4 52 0 14.3 14.4 14.3 37.6 0 52L354.6 396l92.4 96.4c14.4 14.4 14.4 37.6 0 52z'/%3E%3C/svg%3E
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 612 792'%3E%3Cpath fill='%23f0ad4e' d='M603 640.2l-278.5-509c-3.8-6.6-10.8-10.6-18.5-10.6s-14.7 4-18.5 10.6L9 640.2c-3.7 6.5-3.6 14.4.2 20.8 3.8 6.5 10.8 10.4 18.3 10.4h557c7.6 0 14.6-4 18.4-10.4 3.5-6.4 3.6-14.4 0-20.8zm-266.4-30h-61.2V549h61.2v61.2zm0-107h-61.2V304h61.2v199z'/%3E%3C/svg%3E

and I compared it with the #17476 PR by @efender and here are results:

Icon name by @efender in plain bytes by me in plain bytes winner reduce size even more?
input:checked ~ .c-indicator (checkbox) 214 214 equally yes, need an svg expert
input:indeterminate ~ .c-indicator 168 145 i am (-13.7%) probably yes, but it's hard
input:checked ~ .c-indicator (radio) 185 139 i am (-24.7%) no, 139 is the absolute minimum
.c-select (png icon) not represented 156 i am no, 156 is the absolute minimum
form-icon-success 365 347 i am (-4.9%) yes, need an svg expert
form-icon-error 441 420 i am (-4.8%) yes, need an svg expert
form-icon-warning 391 379 i am (-3.1%) yes, need an svg expert

Also all a hex colors are translated to lowercase (improves compression) in my PR.
How can I help? Sorry, I'm new to git and I don't know how to use git for this.

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Nov 15, 2015

Member

Also all a hex colors are translated to lowercase (improves compression) in my PR.
How can I help? Sorry, I'm new to git and I don't know how to use git for this.

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鈥攍et me know if you are unsure how to clear that up.

Member

mdo commented Nov 15, 2015

Also all a hex colors are translated to lowercase (improves compression) in my PR.
How can I help? Sorry, I'm new to git and I don't know how to use git for this.

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鈥攍et me know if you are unsure how to clear that up.

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Nov 15, 2015

Contributor

No, I didn't implement it in the PR yet. It's possible to do it via the web interface of GitHub?

Contributor

thekondrashov commented Nov 15, 2015

No, I didn't implement it in the PR yet. It's possible to do it via the web interface of GitHub?

@wolfy1339

This comment has been minimized.

Show comment
Hide comment
@wolfy1339

wolfy1339 Nov 15, 2015

Contributor

Commit c972592 is unwanted.

Contributor

wolfy1339 commented Nov 15, 2015

Commit c972592 is unwanted.

@efender

This comment has been minimized.

Show comment
Hide comment

efender commented Nov 15, 2015

And more optimizations - http://sassmeister.com/gist/a02447e3bed566e3068c
I redrew the icons (less instructions), sources here - https://drive.google.com/open?id=0B83vy7NwEQtkeE9DaWFSNkhxNTg

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Nov 15, 2015

Contributor

Commit c972592 is unwanted.

How can I fix it?

Contributor

thekondrashov commented Nov 15, 2015

Commit c972592 is unwanted.

How can I fix it?

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Nov 15, 2015

Contributor

I redrew input:indeterminate ~ .c-indicator icon:

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 4'%3E%3Cpath stroke='%23fff' d='M0 2h4'/%3E%3C/svg%3E

and I counted the table with the new values by @efender:

And more optimizations - http://sassmeister.com/gist/a02447e3bed566e3068c
I redrew the icons (less instructions), sources here - https://drive.google.com/open?id=0B83vy7NwEQtkeE9DaWFSNkhxNTg

Here are results:

Icon name by @efender in plain bytes by me in plain bytes winner reduce size even more?
input:checked ~ .c-indicator (checkbox) 193 214 @efender (-9.8%) no, design can be broken
input:indeterminate ~ .c-indicator 145 142 i am (-2.1%) no, 142 is the absolute minimum
input:checked ~ .c-indicator (radio) 151 139 i am (-7.9%) no, 139 is the absolute minimum
.c-select (png icon) not represented 156 i am no, 156 is the absolute minimum
form-icon-success 258 347 @efender (-25.6%) probably yes, but it's very hard
form-icon-error 329 420 @efender (-21.7%) probably yes, but it's very hard
form-icon-warning 310 379 @efender (-18.2%) probably yes, but it's very hard

Here the best results at the moment:

input:checked ~ .c-indicator (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M6.564.75l-3.59 3.612-1.538-1.55L0 4.26 2.974 7.25 8 2.193z'/%3E%3C/svg%3E

input:indeterminate ~ .c-indicator (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 4'%3E%3Cpath stroke='%23fff' d='M0 2h4'/%3E%3C/svg%3E

input:checked ~ .c-indicator (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-4 -4 8 8'%3E%3Ccircle r='3' fill='%23fff'/%3E%3C/svg%3E

.c-select icon (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 5'%3E%3Cpath fill='%23333' d='M2 0L0 2h4zm0 5L0 3h4z'/%3E%3C/svg%3E

form-icon-success (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath d='M2.31 6.726l-1.698-2.2c-.423-1.032.452-1.407 1.1-.8l1.1 1.4 3.4-3.8c.594-.63 1.59-.267 1.2.7l-4 4.6c-.443.514-.81.418-1.1.1z' fill='%235b5'/%3E%3C/svg%3E

form-icon-warning (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fa4' d='M4.4 5.324h-.8v-2.46h.8zm0 1.42h-.8V5.89h.8zM3.76.63L.04 7.075c-.115.2.016.425.26.426h7.397c.242 0 .372-.226.258-.426C6.726 4.924 5.47 2.79 4.253.63c-.113-.174-.39-.174-.494 0z'/%3E%3C/svg%3E

form-icon-danger (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' viewBox='0 0 8 8'%3E%3Cpath id='a' fill='%23d54' d='M.228 1.548l6.135 6.197c.833.77 2.183-.36 1.403-1.3L1.632.245C.725-.538-.518.735.228 1.55z'/%3E%3Cuse xlink:href='%23a' transform='rotate(90 4 4)'/%3E%3C/svg%3E
Contributor

thekondrashov commented Nov 15, 2015

I redrew input:indeterminate ~ .c-indicator icon:

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 4'%3E%3Cpath stroke='%23fff' d='M0 2h4'/%3E%3C/svg%3E

and I counted the table with the new values by @efender:

And more optimizations - http://sassmeister.com/gist/a02447e3bed566e3068c
I redrew the icons (less instructions), sources here - https://drive.google.com/open?id=0B83vy7NwEQtkeE9DaWFSNkhxNTg

Here are results:

Icon name by @efender in plain bytes by me in plain bytes winner reduce size even more?
input:checked ~ .c-indicator (checkbox) 193 214 @efender (-9.8%) no, design can be broken
input:indeterminate ~ .c-indicator 145 142 i am (-2.1%) no, 142 is the absolute minimum
input:checked ~ .c-indicator (radio) 151 139 i am (-7.9%) no, 139 is the absolute minimum
.c-select (png icon) not represented 156 i am no, 156 is the absolute minimum
form-icon-success 258 347 @efender (-25.6%) probably yes, but it's very hard
form-icon-error 329 420 @efender (-21.7%) probably yes, but it's very hard
form-icon-warning 310 379 @efender (-18.2%) probably yes, but it's very hard

Here the best results at the moment:

input:checked ~ .c-indicator (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fff' d='M6.564.75l-3.59 3.612-1.538-1.55L0 4.26 2.974 7.25 8 2.193z'/%3E%3C/svg%3E

input:indeterminate ~ .c-indicator (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 4'%3E%3Cpath stroke='%23fff' d='M0 2h4'/%3E%3C/svg%3E

input:checked ~ .c-indicator (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-4 -4 8 8'%3E%3Ccircle r='3' fill='%23fff'/%3E%3C/svg%3E

.c-select icon (by me)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 4 5'%3E%3Cpath fill='%23333' d='M2 0L0 2h4zm0 5L0 3h4z'/%3E%3C/svg%3E

form-icon-success (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath d='M2.31 6.726l-1.698-2.2c-.423-1.032.452-1.407 1.1-.8l1.1 1.4 3.4-3.8c.594-.63 1.59-.267 1.2.7l-4 4.6c-.443.514-.81.418-1.1.1z' fill='%235b5'/%3E%3C/svg%3E

form-icon-warning (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 8 8'%3E%3Cpath fill='%23fa4' d='M4.4 5.324h-.8v-2.46h.8zm0 1.42h-.8V5.89h.8zM3.76.63L.04 7.075c-.115.2.016.425.26.426h7.397c.242 0 .372-.226.258-.426C6.726 4.924 5.47 2.79 4.253.63c-.113-.174-.39-.174-.494 0z'/%3E%3C/svg%3E

form-icon-danger (by @efender)
data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' viewBox='0 0 8 8'%3E%3Cpath id='a' fill='%23d54' d='M.228 1.548l6.135 6.197c.833.77 2.183-.36 1.403-1.3L1.632.245C.725-.538-.518.735.228 1.55z'/%3E%3Cuse xlink:href='%23a' transform='rotate(90 4 4)'/%3E%3C/svg%3E
@cvrebert

This comment has been minimized.

Show comment
Hide comment
@cvrebert

cvrebert Nov 16, 2015

Member

Commit c972592 is unwanted.

How can I fix it?

We can fix it on our end when we decide to merge this.

Member

cvrebert commented Nov 16, 2015

Commit c972592 is unwanted.

How can I fix it?

We can fix it on our end when we decide to merge this.

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 8, 2015

Contributor

@cvrebert , now merge conflict fixed? I need something else to do?

Contributor

thekondrashov commented Dec 8, 2015

@cvrebert , now merge conflict fixed? I need something else to do?

@cvrebert

This comment has been minimized.

Show comment
Hide comment
@cvrebert

cvrebert Dec 8, 2015

Member

No. This is awaiting further review from @mdo.
(And once he approves, it'll need a rebase by someone on Core Team.)

Member

cvrebert commented Dec 8, 2015

No. This is awaiting further review from @mdo.
(And once he approves, it'll need a rebase by someone on Core Team.)

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 8, 2015

Contributor

I think I broke something in _variables.scss file. I will try to fix.

Contributor

thekondrashov commented Dec 8, 2015

I think I broke something in _variables.scss file. I will try to fix.

@houndci-bot

View changes

Show outdated Hide outdated scss/_variables.scss
@@ -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;

This comment has been minimized.

@houndci-bot

houndci-bot Dec 8, 2015

Files should end with a trailing newline

@houndci-bot

houndci-bot Dec 8, 2015

Files should end with a trailing newline

This comment has been minimized.

@cvrebert

cvrebert Dec 8, 2015

Member

Add an empty line at the end of the file.

@cvrebert

cvrebert Dec 8, 2015

Member

Add an empty line at the end of the file.

This comment has been minimized.

@thekondrashov

thekondrashov Dec 8, 2015

Contributor

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.)

@thekondrashov

thekondrashov Dec 8, 2015

Contributor

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.)

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 8, 2015

Contributor

I apologize for my mistakes, I'm new to git.

Contributor

thekondrashov commented Dec 8, 2015

I apologize for my mistakes, I'm new to git.

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Dec 8, 2015

Member

Validation states are too big now, and it looks like the warning and error ones have been swapped.

screen shot 2015-12-07 at 7 56 21 pm

Member

mdo commented Dec 8, 2015

Validation states are too big now, and it looks like the warning and error ones have been swapped.

screen shot 2015-12-07 at 7 56 21 pm

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 8, 2015

Contributor

Oh, I'm sorry. I have not tested icons from @efender after optimizations.
Now there are small differences new validation feedback icons from the original icons, but they are minimal.

Contributor

thekondrashov commented Dec 8, 2015

Oh, I'm sorry. I have not tested icons from @efender after optimizations.
Now there are small differences new validation feedback icons from the original icons, but they are minimal.

@mdo mdo modified the milestones: v4.0.0-alpha.2, v4.0.0-alpha.3 Dec 8, 2015

@efender

This comment has been minimized.

Show comment
Hide comment
@efender

efender Dec 8, 2015

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;

efender commented Dec 8, 2015

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;
@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 8, 2015

Contributor

Yes, I agree. Need to return changes of the value of the viewBox in svgs and manipulate the dimensions in other ways.
Now I'm trying to optimize icons even more.
At the moment, this is the best version of form-icon-danger icon:

<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):

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='%23d9534f' viewBox='-1 -1 5 5'%3E%3Cpath stroke='%23d9534f' d='M0 0l3 3m0-3L0 3'/%3E%3Ccircle r='.5'/%3E%3Ccircle cx='3' r='.5'/%3E%3Ccircle cy='3' r='.5'/%3E%3Ccircle cx='3' cy='3' r='.5'/%3E%3C/svg%3E
Contributor

thekondrashov commented Dec 8, 2015

Yes, I agree. Need to return changes of the value of the viewBox in svgs and manipulate the dimensions in other ways.
Now I'm trying to optimize icons even more.
At the moment, this is the best version of form-icon-danger icon:

<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):

data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='%23d9534f' viewBox='-1 -1 5 5'%3E%3Cpath stroke='%23d9534f' d='M0 0l3 3m0-3L0 3'/%3E%3Ccircle r='.5'/%3E%3Ccircle cx='3' r='.5'/%3E%3Ccircle cy='3' r='.5'/%3E%3Ccircle cx='3' cy='3' r='.5'/%3E%3C/svg%3E
@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 9, 2015

Contributor

@efender, I created JSFiddle preview.
Looks like it is better to use background-size: 1.5rem 1rem; instead background-size: auto 1rem; for validation feedback icons and background-size: .5rem .75rem; for select menus.
Checkboxes and radios work perfectly.

Contributor

thekondrashov commented Dec 9, 2015

@efender, I created JSFiddle preview.
Looks like it is better to use background-size: 1.5rem 1rem; instead background-size: auto 1rem; for validation feedback icons and background-size: .5rem .75rem; for select menus.
Checkboxes and radios work perfectly.

@twbs-lmvtfy

This comment has been minimized.

Show comment
Hide comment
@twbs-lmvtfy

twbs-lmvtfy Dec 9, 2015

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:

  • line 87, column 3 thru column 48: Element base not allowed as child of element body in this context. (Suppressing further errors from this subtree.)

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!

(Please note that this is a fully automated comment.)

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:

  • line 87, column 3 thru column 48: Element base not allowed as child of element body in this context. (Suppressing further errors from this subtree.)

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!

(Please note that this is a fully automated comment.)

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
Contributor

thekondrashov commented Dec 9, 2015

@twbs-lmvtfy

This comment has been minimized.

Show comment
Hide comment
@twbs-lmvtfy

twbs-lmvtfy Dec 9, 2015

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:

  • line 92, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 98, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 105, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 115, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 121, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 128, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 138, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 144, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 151, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 161, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 186, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 214, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 234, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 258, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 269, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 281, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!

(Please note that this is a fully automated comment.)

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:

  • line 92, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 98, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 105, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 115, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 121, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 128, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 138, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 144, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 151, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 161, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 186, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 214, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 234, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 258, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 269, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid
  • line 281, column 7: E003: Found one or more .rows that were not children of a grid column or descendants of a .container or .container-fluid

You'll need to fix these errors and post a revised example before we can proceed further.
Thanks!

(Please note that this is a fully automated comment.)

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Dec 24, 2015

Member

So, where are we with this?

Member

XhmikosR commented Dec 24, 2015

So, where are we with this?

@thekondrashov

This comment has been minimized.

Show comment
Hide comment
@thekondrashov

thekondrashov Dec 26, 2015

Contributor

@XhmikosR, sorry for the delay.
I rewrote my JSFiddle preview with scss. In this JSFiddle I'm provide two alternatives for form validation icons: ascii svgs with and without sizing.
Please, check it and if it's okay, I will create a commit with your favourite option.

Contributor

thekondrashov commented Dec 26, 2015

@XhmikosR, sorry for the delay.
I rewrote my JSFiddle preview with scss. In this JSFiddle I'm provide two alternatives for form validation icons: ascii svgs with and without sizing.
Please, check it and if it's okay, I will create a commit with your favourite option.

@mdo

This comment has been minimized.

Show comment
Hide comment
@mdo

mdo Jan 6, 2016

Member

@thekondrashov Wow, that's awesome work!

I'm a fan of the ascii #3 variations as the icons in the form controls are resized to compensate for the form control size changes.

Member

mdo commented Jan 6, 2016

@thekondrashov Wow, that's awesome work!

I'm a fan of the ascii #3 variations as the icons in the form controls are resized to compensate for the form control size changes.

@mdo mdo merged commit c4c9e19 into twbs:v4-dev Feb 7, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@thekondrashov thekondrashov deleted the thekondrashov:patch-1 branch Feb 10, 2016

@cvrebert cvrebert referenced this pull request Jul 20, 2016

Merged

V4 hamburger fix2 #20329

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment