-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature: added calculator icon #612
Conversation
Codecov Report
@@ Coverage Diff @@
## master #612 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 34 34
Branches 3 3
=====================================
Hits 34 34 Continue to review full report at Codecov.
|
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.
since you're not using fill, I think you could remove fill rule.
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.
Your "grouping" of paths seems weird. Could try something more logical, like a compound path for the first row buttons, one for the second, etc ? Also, you should work on spacing, things are just too close to the border and that looks really bad. Guidelines (#171) says that things should have at least 2px of empty space.
Yeah looking at it now in the preview it’s not the same as when I preview it in the browser. Also the grouping came from SVGOMG I guess, I’ll get the original from Figma and see what’s the difference.
Will resubmit in the morning.
…On Jun 4, 2019, 10:52 PM -0700, Locness ***@***.***>, wrote:
@locness3 requested changes on this pull request.
Your "grouping" of paths seems weird. Could try something more logical, like a compound path for the first row buttons, one for the second, etc ? Also, you should work on spacing, things are just too close to the border and that looks really bad. Guidelines (#171) says that things should have at least 2px of empty space.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Maybe it should be looks like this: Code: <svg
xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
stroke-width="2"
stroke-linecap="round"
stroke-linejoin="round"
>
<rect x="6" y="3" width="12" height="18" rx="2" />
<path d="M9 18V18" />
<path d="M9 15V15" />
<path d="M9 12V12" />
<path d="M12 18V18" />
<path d="M12 15V15" />
<path d="M12 12V12" />
<path d="M15 18V18" />
<path d="M15 15V15" />
<path d="M15 12V12" />
</svg> |
@ahtohbi4 I really like the icon that you designed ... one note, maybe add a "screen" to the icon (above the numbers)? |
@johnletey let's try :) But I had to do this version of calculator wider. |
Although it is a wider icon @ahtohbi4 ... I love it with the screen! |
I think it looks to detailed for small sizes. you could you try removing some buttons. |
Why not ? I think it looks weird with the keys not being circles though. Maybe have only four keys, and make them circles ? |
That's great now! |
@btn0s you can take the code if you wish: <svg
xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
stroke-width="2"
stroke-linecap="round"
stroke-linejoin="round"
>
<rect x="5" y="2" width="14" height="20" rx="2" />
<rect x="9" y="6" width="6" height="4" />
<path d="M10 14V14M14 14V14M10 18V18M14 18V18z" />
</svg> |
I liked the non-circle buttons more imo, iOS is the only calc I know of with circle buttons -- most have rounded squares. That said if everyone else is happy with this I'll update the code. |
Oh |
@ahtohbi4 Can I get the code for this one? This is the one I'd like to submit. |
@btn0s This one seems too cluttered. Maybe doing only four buttons and making the screen less to the edges. |
As long as it keeps the width, we don't want it to look like a cellphone. |
The more simpler the more Fetaher-ish though. @colebemis ? |
|
Since I needed a calculator icon, I edited ahtohbi4's one to: <rect x="3" y="1" width="18" height="22" rx="2" />
<rect x="7" y="5" width="10" height="2" />
<path d="M8 20V19" />
<path d="M8 16V15" />
<path d="M8 12V11" />
<path d="M12 20V19" />
<path d="M12 16V15" />
<path d="M12 12V11" />
<path d="M16 20V19" />
<path d="M16 16V15" />
<path d="M16 12V11" />
|
You could also use @lucide-icons (community fork of Feather), we have a calculator icon. |
Added calculator icon to close #462