search

willianjusten / nba-remix

A simple app to show NBA games and scores/details.
https://nba.willianjusten.com.br
210 stars 31 forks source link

Add favicon - closes #93 #97

Closed edisonferraz closed 2 years ago

edisonferraz commented 2 years ago

I decided to use the NBA icon as a favicon instead of the ball icon.

vercel[bot] commented 2 years ago

Someone is attempting to deploy a commit to a Personal Account owned by @willianjusten on Vercel.

@willianjusten first needs to authorize it.

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/willianjusten/nba-remix/Gy1xiWqbiJBRGKS8y2AWTUcGNNQZ
✅ Preview: https://nba-remix-git-fork-edisonferraz-add-favicon-willianjusten.vercel.app

adeonir commented 2 years ago

Hey @edisonferraz sorry but not quite what we are looking for.

You can use the designed icon from our Figma file.

willianjusten commented 2 years ago

Just giving my 20cents as well. @edisonferraz, my tip is to always start asking what the maintainers think about a change like that, instead of just saying that "you decided".

I'd start a conversation inside the issue like:

"Hey guys, I was thinking to use the NBA logo there, what do you guys think? Can I open a PR with it?"

edisonferraz commented 2 years ago

sorry guys, I used the wrong word really, what I would like to express was that I didn't find a ball favicon and my suggestion was this (knowing that it would go through review).

my other mistake was not looking at Figma, I will implement the icon that is present in Figma.

sorry again for the rude way I expressed myself earlier, thanks for the feedback <3

willianjusten commented 2 years ago

Nice to see that you took our feedback in a good direction! This is the way <3

Another tip that I can give to you is using a site like https://favicon.io/ to help you create the different versions of the favicon and the manifest.json, this will help us when creating the PWA on #84 =)

adeonir commented 2 years ago

I think that @helderburato already started #84, so for this PR is just to add the .ico file.

edisonferraz commented 2 years ago

@willianjusten @adeonir I just made the commit adding the .ico file, let me know if it is necessary to add the other formats please

willianjusten commented 2 years ago

Looks great to me =) image And since @helderburato will take care of the PWA part, he can create the manifest there.

Thanks for your contribution and your willingness to get our feedback and answer to it as well <3