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 Modrinth icon #8101

Merged
merged 6 commits into from
Dec 21, 2022
Merged

Add Modrinth icon #8101

merged 6 commits into from
Dec 21, 2022

Conversation

magicus
Copy link
Contributor

@magicus magicus commented Dec 3, 2022

modrinth-preview

Issue: closes #7488

Similarweb rank: #48410

Checklist

  • I updated the JSON data in _data/simple-icons.json
  • I optimized the icon with SVGO or SVGOMG
  • The SVG viewbox is 0 0 24 24

Description

According to the two previously closed PRs (#7579 and #7688) trying to incorporate the Modrinth logo, the full logo was deemed too complex, and the simpler "favicon" (still svg based) should be used instead. Unfortunately this used design-wise complex objects and clipping, which made it tricky to convert into a single path as required by simple-icons.

I solved it by following a hint about using Inkscape's fill bucket.

The color is the official light mode branding color.

@github-actions github-actions bot added the new icon Issues or pull requests for adding a new icon label Dec 3, 2022
@magicus magicus marked this pull request as draft December 3, 2022 19:31
@magicus
Copy link
Contributor Author

magicus commented Dec 3, 2022

I apologize. I'm not that used to working with svg files. I had apparently not succeeded in getting the icon maximized in the viewbox. I thought Inkscape had done that for me when I asked it to resize it, but it was apparently a bit off. I need to read up a bit on how to fix this.

@magicus magicus marked this pull request as ready for review December 3, 2022 20:02
Copy link

@triphora triphora left a comment

Choose a reason for hiding this comment

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

Looks good, but could the color be changed to either #1bd96a (dark mode brand color) or #00af5c (light mode brand color)? Whichever looks better in this context.

_data/simple-icons.json Outdated Show resolved Hide resolved
@magicus
Copy link
Contributor Author

magicus commented Dec 7, 2022

@triphora Done. Also updated the preview.

@magicus
Copy link
Contributor Author

magicus commented Dec 12, 2022

What is the process for getting this merged?

@magicus
Copy link
Contributor Author

magicus commented Dec 12, 2022

Also, @triphora, I see you are associated with Modrinth. Can we consider your approval an official acknowledgement? According to the simple-icons documentation, having such an approval might speed up the process.

@triphora
Copy link

Yes, you may. It's been on my personal to-do list to PR this, but I wasn't able to figure out how to merge the paths without breaking it.

Copy link

@AshtakaOOf AshtakaOOf left a comment

Choose a reason for hiding this comment

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

This need to be merged asap

@magicus
Copy link
Contributor Author

magicus commented Dec 14, 2022

I wasn't able to figure out how to merge the paths without breaking it.

Yeah, that was tricky. I'm not an SVG guru myself either, but I found a hint about using the fill bucket tool in Inkscape, and that did the trick. It created a new shape based on what is visible "to the human eye", which was just what was needed here.

@magicus
Copy link
Contributor Author

magicus commented Dec 20, 2022

@dirien Sorry for the ping, but I don't know else how to get this PR to the attention of someone with merge permissions. I believe this PR is ready for merge. We have an official review from the Modrinth team (#8101 (comment)). The automatic tests pass. All official requirements are checked off.

@dirien
Copy link
Member

dirien commented Dec 20, 2022

Hi @magicus,

I will look tomorrow (CET time) as first thing into it!

Thanks for the contribution!

Copy link
Member

@dirien dirien left a comment

Choose a reason for hiding this comment

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

Ho @magicus,

i see some differences to the reference icon (favicon.svg) I will approve this PR as the Modrinth team gave green light for your icon!

Thanks for contributing.

image

@dirien dirien merged commit 941bef9 into simple-icons:develop Dec 21, 2022
@magicus magicus deleted the modrinth-icon branch December 21, 2022 10:59
@github-actions github-actions bot mentioned this pull request Dec 25, 2022
mondeja added a commit that referenced this pull request Dec 29, 2022
# New Icons

- Île-de-France Mobilités (#8062)
- Cultura (#8123)
- Infracost (#8135)
- Modrinth (#8101)
- Polars (#8128)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new icon Issues or pull requests for adding a new icon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icon Request: Modrinth
4 participants