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 currency and banknote icons #670

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jletey
Copy link

@jletey jletey commented Jul 8, 2019

Closes #182

@codecov
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (3c64e93) to head (bbdaf24).
Report is 64 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #670   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines           34        34           
  Branches         3         3           
=========================================
  Hits            34        34           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

moeenio
moeenio previously approved these changes Jul 8, 2019
@moeenio
Copy link

moeenio commented Jul 8, 2019

You shoudl rename your PR to "feat: Add currency and banknote icons"

@jletey jletey changed the title Add currency icons Add currency and banknote icons Jul 9, 2019
@moeenio moeenio mentioned this pull request Jul 19, 2019
@mittalyashu
Copy link
Contributor

Banknote

To me, it doesn't really look like a banknote (sorry)

Icon
image

Bitcoin

In bitcoin logo the second curve should not be far, instead, it should be of the same size and tilted to 10 deg of the angle.

Icon Original
Figma_1MStwoFT6u image

Swiss Franc

In the first of the F, there is a lot of empty space in between

Icon Example
image image

Euros

Add the ends of euro looks too close

Icon Example
image image

GBP

In GBP icon, instead of keeping it flat straight, I think there should be a curve

Icon Original
image image

Rub

Maybe try decreasing the height of this area

Icon Original
image image

@jletey
Copy link
Author

jletey commented Jul 22, 2019

@locness3 @mittalyashu

Banknote

To me, it doesn't really look like a banknote (sorry)

Icon
image

I'm open for suggestions!

Bitcoin

In bitcoin logo the second curve should not be far, instead, it should be of the same size and tilted to 10 deg of the angle.

Icon Original
Figma_1MStwoFT6u image

Like this?

Shot 2019-07-22 at 18 53 35

Swiss Franc

In the first of the F, there is a lot of empty space in between

Icon Example
image image

Have updated so that the second line of the F is exactly in the middle of the other two lines:

Shot 2019-07-22 at 18 44 47

Euros

Add the ends of euro looks too close

Icon Example
image image

Updated:

Shot 2019-07-22 at 18 47 56

GBP

In GBP icon, instead of keeping it flat straight, I think there should be a curve

Icon Original
image image

Updated:

Shot 2019-07-22 at 18 42 59

Rub

Maybe try decreasing the height of this area

Icon Original
image image

Updated:

Shot 2019-07-22 at 18 55 08

@mittalyashu
Copy link
Contributor

Bitcoin

The tile of the bitcoin looks good, can you also try a newer version of bitcoin in which size of both the bumps is same.

image

GBP

We need to work on that bumpy curve 😅

image

Swiss Franc, Euros, Rub

Now, it looks good.

@jletey
Copy link
Author

jletey commented Jul 23, 2019

Bitcoin

The tile of the bitcoin looks good, can you also try a newer version of bitcoin in which size of both the bumps is same.

image

@mittalyashu How is this?

Shot 2019-07-23 at 14 53 03

GBP

We need to work on that bumpy curve 😅

image

Maybe give me an example?

Swiss Franc, Euros, Rub

Now, it looks good.

Updated the PR

@mittalyashu
Copy link
Contributor

Bitcoin icon looks good now.

Let me find an example of the GBP icon.

moeenio
moeenio previously approved these changes Jul 24, 2019
@moeenio
Copy link

moeenio commented Jul 24, 2019

Maybe adding a bitcoin tilted icon too ?

@MarcelloTheArcane
Copy link
Contributor

I think the BTC icon should be straight by default - is that possible?

@mittalyashu
Copy link
Contributor

No @MarcelloTheArcane it's not straight, you can check the official bitcoin website.

@jletey
Copy link
Author

jletey commented Aug 3, 2019

@locness3 @mittalyashu @MarcelloTheArcane Sorry ... didn't ever update the btc icon

It's now updated ... just need to work on the GBP

@antony
Copy link
Contributor

antony commented Oct 21, 2019

Excited about the GBP icon!

@moeenio
Copy link

moeenio commented Feb 16, 2020

@colebemis We're still waiting for you...

@LoopsGod
Copy link

This repository badly needs some extra moderators / pr-reviewers.

@moeenio
Copy link

moeenio commented Apr 4, 2020

Could someone reach out to @colebemis on Twitter?

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.

Icon Request: Currency
6 participants