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

feat: Add google icon #114

Closed
wants to merge 1 commit into from
Closed

Conversation

fauxparse
Copy link

Add a Google icon for login using Google OAuth.

@wappsdotgr
Copy link

wappsdotgr commented Jun 15, 2017

@colebemis Since you included this google icon you can close mine in #93 or use it with a '+' for google plus

@tjkns94
Copy link

tjkns94 commented Mar 24, 2018

hi, what is the hold up here?

dev-ea
dev-ea previously approved these changes Aug 1, 2018
martinsvoboda
martinsvoboda previously approved these changes Aug 26, 2018
@adaniello
Copy link

No way to merge this?

@locness3
Copy link

@colebemis Since you included this google icon you can close mine in #93 or use it with a '+' for google plus

Google plus is closing

@locness3
Copy link

No way to merge this?

@colebemis have to review first

@colebemis
Copy link
Member

colebemis commented Mar 21, 2019

I'll take a look at this today 👍 Sorry for the delay 😅

@colebemis
Copy link
Member

@fauxparse It looks like this icon is located in a icons/logos directory. Could you move into the icons directory?

@@ -0,0 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="#000" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
<path d="M21.8,10h-2.6l0,0H12v4h5.7c-0.8,2.3-3,4-5.7,4c-3.3,0-6-2.7-6-6s2.7-6,6-6c1.7,0,3.2,0.7,4.2,1.8l2.8-2.8C17.3,3.1,14.8,2,12,2C6.5,2,2,6.5,2,12s4.5,10,10,10s10-4.5,10-10C22,11.3,21.9,10.6,21.8,10z"/>
</svg>
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the formatting of this SVG file is slightly different than the rest of the files. Could you run npm run build to fix it? There are instructions on how to do that at the bottom of this comment: #171 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

Fixed!

@fauxparse
Copy link
Author

@fauxparse It looks like this icon is located in a icons/logos directory. Could you move into the icons directory?

Done. I guess the logos directory existed when I made the pull request back in August... ;)

@fauxparse
Copy link
Author

@colebemis Should be good to go now.

@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #114 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #114   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          34     34           
  Branches        3      3           
=====================================
  Hits           34     34

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d42ac8...1ccdf79. Read the comment docs.

@jletey
Copy link

jletey commented Apr 19, 2019

What's the hold up on this?

@mxstbr
Copy link

mxstbr commented May 8, 2019

Would love to have this!

@tbouron
Copy link

tbouron commented Jul 7, 2019

Hey there, any reason this hasn't be merged yet? I too would love to have the Google logo for my Google AOuth button rather than relying on some PNG icon.

@locness3
Copy link

locness3 commented Jul 7, 2019

Hey there, any reason this hasn't be merged yet?

Waiting for @colebemis ...

@mittalyashu
Copy link
Contributor

There should be a minimum of 2px of spacing

image

@simonschaufi
Copy link

simonschaufi commented Aug 19, 2019

If I adjust the font awesome google icon (https://fontawesome.com/icons/google?style=brands) to have just the line, then it looks like this:
image

@fauxparse Could you adjust it to look similar like that?

@fauxparse
Copy link
Author

@fauxparse Could you adjust it to look similar like that?

This PR is over two years old, I'm really not sure I need to.

@locness3 locness3 mentioned this pull request Aug 26, 2019
@NickeManarin
Copy link

@colebemis Could you take a look at this? This PR is quite old...

@omnibrain
Copy link

@colebemis please? Looks like all it needs at this point is one last approval. I think a lot of people are waiting for this

@locness3
Copy link

Sadly it looks like @colebemis doesn't want to merge new icons...

@omnibrain
Copy link

@colebemis I'll send you $5 in bitcoins if you just click that merge button. Please, oh please mighty maintainer, let us have this. We do not ask for much. We are but some humble developers, trying to make a living, barely getting by, craving that beautiful G. We never questioned your authority, never even as much as glanced at the new font-awesome icons. Have mercy with us, o mighty maintainer.

@locness3
Copy link

@omnibrain not sure that's how things work. And what prevents you from downloading the svg from @fauxparse's fork or using his whole repo?

@omnibrain
Copy link

@locness3 using the official package is obviously nicer than using some fork but I guess that‘s what I‘ll do. I think what bothers me is that I don‘t see any reason for this not to be merged? Has feather icons reached the end of development? I‘m OK with any explanation, but no explanation at all is hard to accept.

@locness3
Copy link

locness3 commented Mar 1, 2020

@omnibrain Brand icons are deprecated in feather, read #763, tho it would be cool to merge the current brand icons requests and only reject further ones.
But more generally @colebemis doesn't give any sign of life concerning this project.

@NickeManarin
Copy link

NickeManarin commented Mar 1, 2020

@omnibrain Brand icons are deprecated in feather

That's a harsh decision. Deprecating all brand icons instead of accepting the ones that are simple enough?
That's like deprecating all icons because most are too complex and complicated to be drawn in a 24x24px grid.

@locness3
Copy link

locness3 commented Mar 8, 2020

The problem, i guess, is brand icons are unique, and not adaptable to any icon set.

And if all you need are brand icons SVGs, simpleicons.com dors the job.

@simonschaufi
Copy link

simonschaufi commented Jul 17, 2020

Everyone who wants to manually add the icon, here is the path from simpleicons a little bit adjusted:

<symbol id="google" viewBox="0 0 24 24">
    <path d="M12.219 10.43 12.219 14.199 18.457 14.199C18.207 15.816 16.574 18.941 12.219 18.941 8.465 18.941 5.402 15.836 5.402 12 5.402 8.164 8.469 5.059 12.219 5.059 14.355 5.059 15.785 5.965 16.605 6.754L19.59 3.875C17.672 2.086 15.188 1 12.219 1 6.137 1 1.219 5.918 1.219 12 1.219 18.082 6.137 23 12.219 23 18.57 23 22.781 18.535 22.781 12.25 22.781 11.527 22.703 10.977 22.605 10.43ZM12.219 10.43"/>
</symbol>

@emmahsax
Copy link

I'll echo @fauxparse @omnibrain and say that this would be soooo helpful!

@colebemis Please? 🙏🏼

@PeterShaggyNoble
Copy link

As per #763, brand icons have been deprecated here in favour of Simple Icons, where this is already available: https://simpleicons.org/?q=Google

Google

@emmahsax
Copy link

Ah yes, I saw that. I wasn't sure if that was the final decision yet or not. I found this substitution too, which sort of looks in the style of feather.

Copy link

@gianfrancolombardo gianfrancolombardo left a comment

Choose a reason for hiding this comment

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

Looks perfect

@vonec
Copy link

vonec commented Mar 5, 2021

when will this get merged ?

@locness3
Copy link

locness3 commented Mar 7, 2021

Never #763

@fauxparse fauxparse closed this Mar 7, 2021
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.

None yet