-
Notifications
You must be signed in to change notification settings - Fork 62
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
Fixes #139: Use SVG to draw the checkmark. #142
Conversation
Why not use the |
@abdonrd good idea! I rather not make paper-checkbox depend on iron-icon but it's definitely a better source for the SVG than trying to roll my own. |
@bicknellr why not use the |
@cdata is it ok if I assign this to you? |
yo, what's the status on this? |
When merge this PR? |
This visual change will cause quite a bit of jank between the old and new versions which would break a lot of visual tests. I'm for SVGification, but on the fence about the recentering (as it's non-standard as is). Thoughts @notwaldorf @rictic? |
Would like to know how this PR fares in our internal tests. |
6b66c1b
to
213807e
Compare
a437745
to
dcb8ee4
Compare
(going to remove myself as a reviewer since I think Elliott has this covered!) |
@bicknellr any updates on internal testing? |
Last I checked, it breaks a bunch of screenshot tests - as expected. I don't remember there being any obviously broken non-screenshot tests but I should run it again. |
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.
Waiting for update / resolution on this
Go to either demo below and resize the window to see the difference:
before: http://jsbin.com/fepide/1/edit?html,output
after: http://jsbin.com/lisubu/1/edit?html,output
(Fixes #139.)