useful-forks / useful-forks.github.io

Improving GitHub's Forks list discoverability through automatic filtering. The project offers an online tool and a Chrome extension.
https://useful-forks.github.io/
MIT License
1.18k stars 61 forks source link

Extension: Change button icon, fix popup and double quote #64

Closed WarningImHack3r closed 1 year ago

WarningImHack3r commented 1 year ago

Bring UI tweaks to the Chrome extension's in-GitHub button:

Also, change the remaining simple quotes to double quotes for consistency

payne911 commented 1 year ago

Thanks for your contribution!

Would you mind including a little screenshot in your PR's summary? :)

Also, change the remaining simple quotes to double quotes for consistency

Ideally, I would have preferred that being done as a separate commit, but it's not really a big deal.

WarningImHack3r commented 1 year ago

Thanks for your contribution!

No problem :)

Would you mind including a little screenshot in your PR's summary? :)

Here you are!

CleanShot 2023-07-31 at 14 17 42@2x

Note I know it's not perfectly vertically centered, but none of the original GitHub buttons really are, so I guess it's a styling issue on GitHub's side they probably would fix by themselves

Also, change the remaining simple quotes to double quotes for consistency

Ideally, I would have preferred that being done as a separate commit, but it's not really a big deal.

Sorry!

payne911 commented 1 year ago

Thanks! I hear you on the "GitHub might fix this eventually", but I think it'd be nice to just have it centered right now, and if they ever do fix this we adjust accordingly. Would you mind trying to center it vertically a bit more?

WarningImHack3r commented 1 year ago

The thing is it would very likely require (inline) CSS and that would be the first "hardcoded" snippet on that injected HTML, which I'm not a fan of. I can possibly try to find an already available (= compiled) Tailwind class somewhere (what all this button is already styled with) to make it "cleaner", but even there we can't be ensured it will stay available forever... A second reason not to do so is that the current magnifying glass icon is not centered either, so it's not a regression, just a better look but still uncentered.

TL;DR: I can do it with inlined CSS if you want but that's not an idea I like much

payne911 commented 1 year ago

Wouldn't another approach be to simply change the values of the SVG?

To me, this icon looks properly centered: image

Whereas this one somehow seems a bit off-centered: image

Honestly, it's no big deal. But if there's a quick hack for it, I think it'd be nice. Let me know if you'd rather me simply merge this as-is.

payne911 commented 1 year ago

Alright, tested it locally and it looks good enough. Let's not bother you further with this.

image

Thanks again for your contribution! 👍 And while I'm here: if you ever feel like it and have some free time, know that #59 is definitely the highest priority issue right now.

WarningImHack3r commented 1 year ago

Thank you! Oh I didn't look at issues at all, gonna take a quick look