-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
bugfix/fix: add default colors to Font versions #2115
base: develop
Are you sure you want to change the base?
Conversation
@lunatic-fox I find it really hard to review this PR because the diff says the whole file changed. Do you know of any easy way to see the exact differences, or do you have any other tips for reviewing this PR? |
Maybe slicing it into around 10 - 20 icons per pull request, that way I think GitHub will be able to render it properly. |
@lunatic-fox I think the issue is that GitHub has trouble rendering single-line diffs. I've seen this issue on multiple new icon PRs before too, so I don't think it's because of the amount of svg's |
Hmm, so in that case, maybe I can prettify the output svgs. However, we should test if the web vscode shows the diffs first. |
@lunatic-fox good idea. I created a codespace here: https://musical-space-invention-ppj4wj4vwj4c9rgj.github.dev/ |
@lunatic-fox |
@Snailedlt
|
@lunatic-fox yeah me neither :/ |
This commit also modifies The icons listed below have to be off-colored since white has no contrast on itself.
|
Well... It seems that I can't upload any image, so here's a zip file containing the screenshots. |
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.
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.
LGTM! ✅
Excellent work lunatic!
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.
@canaleal I think that's just visually on icomoon. Iirc from reviewing the notion PR, the icons are sized and centered correctly, but icomoon has some sort of negative padding issue, which is why it looks like it goes outside the viewbox. |
Double check these details before you open a PR
Features
Add default colors to the icons listed below. And some adjustments on other icons of the same technology.
This PR closes #2105 - [LP] Add default color
Notes
The code to add colors can be found here: https://gist.github.com/lunatic-fox/fdb53e596701fe12cbe38030ac4e5f6b