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

Add stop icons (transparent, filled). #543

Merged
merged 3 commits into from
Mar 20, 2018
Merged

Conversation

bevacqua
Copy link
Contributor

The filled icon needs to actually be filled, but I'm not sure what the best way of doing so is, since I copied the path for another icon to be consistent in terms of border radius.

screen shot 2018-03-20 at 13 47 09

@bevacqua bevacqua requested a review from snide March 20, 2018 16:47
@bevacqua
Copy link
Contributor Author

(Only requirement for the filled icon would be that it needs to be in the same spot as the unfilled one
so that if for some reason you want to toggle between them (on hover or something), they don't appear jittery

<path id="stop-a" d="M0,1.99406028 C0,0.892771196 0.894513756,0 1.99406028,0 L14.0059397,0 C15.1072288,0 16,0.894513756 16,1.99406028 L16,14.0059397 C16,15.1072288 15.1054862,16 14.0059397,16 L1.99406028,16 C0.892771196,16 0,15.1054862 0,14.0059397 L0,1.99406028 Z M1,1.99406028 L1,14.0059397 C1,14.5539384 1.44579254,15 1.99406028,15 L14.0059397,15 C14.5539384,15 15,14.5542075 15,14.0059397 L15,1.99406028 C15,1.44606163 14.5542075,1 14.0059397,1 L1.99406028,1 C1.44606163,1 1,1.44579254 1,1.99406028 Z"/>
</defs>
<g fill-rule="evenodd">
<use xlink:href="#stop-a"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

You're going to have an issue here because you use the same id (#stop-a) in both svg files. These files end up being compiled into one giant SVG. I'd suggest changing the ID or removing the <defs> entirely and just putting the path inside the <g>.

@bevacqua
Copy link
Contributor Author

bevacqua commented Mar 20, 2018 via email

@cchaos
Copy link
Contributor

cchaos commented Mar 20, 2018

@bevacqua I did some tweaking because I wanted them to align a bit better with the size of the pause and play icons so here is some code for the 2 icons if you'd like:

stop

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <path fill-rule="evenodd" d="M4,2 L12,2 C13.1045695,2 14,2.8954305 14,4 L14,12 C14,13.1045695 13.1045695,14 12,14 L4,14 C2.8954305,14 2,13.1045695 2,12 L2,4 C2,2.8954305 2.8954305,2 4,2 Z M4,3 C3.44771525,3 3,3.44771525 3,4 L3,12 C3,12.5522847 3.44771525,13 4,13 L12,13 C12.5522847,13 13,12.5522847 13,12 L13,4 C13,3.44771525 12.5522847,3 12,3 L4,3 Z"/>
</svg>

stop_filled

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <rect width="12" height="12" x="2" y="2" fill-rule="evenodd" rx="2"/>
</svg>

@snide snide removed their request for review March 20, 2018 18:59
@snide
Copy link
Contributor

snide commented Mar 20, 2018

Removing myself form review since @cchaos is already running through this.

@bevacqua
Copy link
Contributor Author

Thanks team, I took the icons from @cchaos and made them more similar by using rects in both cases. Is this okay?

stop

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <rect width="12" height="12" x="2" y="2" stroke="currentColor" fill="transparent" fill-rule="evenodd" rx="2"/>
</svg>

stop_filled

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <rect width="12" height="12" x="2" y="2" stroke="currentColor" fill-rule="evenodd" rx="2"/>
</svg>

@cchaos
Copy link
Contributor

cchaos commented Mar 20, 2018

@bevacqua Unfortunately that won't work because we use the CSS fill property to color our icons. If we started using strokes, we'd have to customize each icon's CSS to allow for that property. Also, by using fills instead of strokes, we maintain the scale and design of the icon as it gets resized.

@cchaos
Copy link
Contributor

cchaos commented Mar 20, 2018

Further, using <rect> vs <path> for the filled one is pretty interchangeable. However, SVGO (our SVG optimizer) tries to simplify as much as possible and I believe it at least "thinks" that using shape elements over paths will render faster. Whether that's true or not, I'm not sure and we could research.

@bevacqua
Copy link
Contributor Author

Oh, that's fine. Updated the PR with your code then

@bevacqua bevacqua force-pushed the stop-icon branch 2 times, most recently from 2ea2b75 to 5658374 Compare March 20, 2018 20:03
@bevacqua
Copy link
Contributor Author

Rebased

@bevacqua bevacqua changed the title [WIP] Add stop icons (transparent, filled). Add stop icons (transparent, filled). Mar 20, 2018
@bevacqua bevacqua merged commit a555d65 into elastic:master Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants