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 Passport (PassportJS) #5054

Merged
merged 7 commits into from Apr 12, 2021

Conversation

aadelgrossi
Copy link
Contributor

passport preview

Issue: n/a
Github stars: 18.5k passport

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

Hex color is one of the primary colors in the logo. I have joined the colored sections into one creating a single path, since the icon must be monochromatic.

@github-actions github-actions bot added the new icon Issues or pull requests for adding a new icon label Feb 18, 2021
@aadelgrossi aadelgrossi changed the title Add PassportJS icon and entry in _data Add Passport (PassportJS) Feb 18, 2021
@PeterShaggyNoble
Copy link
Member

Welcome to Simple Icons, @aadelgrossi and thanks for the contribution.

Often, with icons of this nature, we'll add cut-outs between the different coloured sections (see below) to maintain some sort of differentiation between them but I think, in this case, your approach also works just as well. For the colour, there doesn't look to be a clear cut choice as they use all 3 on their site but I would lean towards the green simply because that's the one used on their homepage.

Going to ping @jaredhanson for feedback here to see if they have an existing or preferred monochrome treatment of the icon, or even allow a monochrome version at all, and also to provide input on the colour choice.

Alexa rank: ~72.7k

@PeterShaggyNoble PeterShaggyNoble added the awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed label Feb 18, 2021
@aadelgrossi
Copy link
Contributor Author

Welcome to Simple Icons, @aadelgrossi and thanks for the contribution.

Often, with icons of this nature, we'll add cut-outs between the different coloured sections (see below) to maintain some sort of differentiation between them but I think, in this case, your approach also works just as well. For the colour, there doesn't look to be a clear cut choice as they use all 3 on their site but I would lean towards the green simply because that's the one used on their homepage.

Going to ping @jaredhanson for feedback here to see if they have an existing or preferred monochrome treatment of the icon, or even allow a monochrome version at all, and also to provide input on the colour choice.

Alexa rank: ~72.7k

You're right, the green may suit the icon better since it ends up more pervasive on other sections of the home page, the remaining colors are sprinkled throughout additional pages but the green definitely stands out.

As for the cut between the colors, that was my initial approach in an attempt to differentiate colored sections, but when I reduced it to the 24 24 viewbox the cut becomes so minimal it's barely visible. Yours is a little more evident since the gap is a bit wider than what I had done, but it's still quite tiny when applied on the smallest size.

Thanks for the feedback and suggestions 👏

@PeterShaggyNoble
Copy link
Member

We try to stick to a 0.5dp cut-out if we do need one as anything larger than that and we begin to lose too much of the icon and anything smaller is barely visible at our native size - my version above was just a very quick mock-up.

@fbernhart fbernhart added changes requested and removed awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed labels Feb 27, 2021
@adamrusted
Copy link
Member

@PeterShaggyNoble I've applied your hex and styling to the icon. Could you or @fbernhart review this one so we can get it merged?

@PeterShaggyNoble
Copy link
Member

Thanks, @adamrusted 🙂 Looks like we're having the same problem here as we had with the Namebase icon, though (e.g.) but they might be fixable just by dragging the points across to meet the curve of the original. There are also a couple of duplicate points at the top of the "L" shape and very top of the icon.

No mention of the logo or artwork in their LICENSE.md so I think, to be safe, we should omit it here.

@adamrusted
Copy link
Member

Weird - no clue what's going on with those paths again. Should be fixed now though.

@jaredhanson
Copy link

Just chiming in here to say the icon looks great, and thanks for including it!

@PeterShaggyNoble
Copy link
Member

Thanks for the seal of approval, @jaredhanson 🙂

Given that, I'm happy to merge this in as-is, if you are @adamrusted. Although, I'm still seeing some minor differences, most notably down the left edge of the inner part of the icon and in the vertical line at the bottom. Gave it a try myself and came up with this (unoptimised) path if you want to run with it instead:

M 11.875,0 C 6.6714098,0.06703894 2.4670256,4.2709337 2.4,9.4746094 H 7.2 C 7.2667969,6.9266223 9.3273115,4.866872 11.875,4.8 Z M 12.125,0 V 4.8 C 14.672987,4.8667969 16.733067,6.9269829 16.8,9.4746094 H 21.6 C 21.532961,4.2710192 17.328676,0.06702552 12.125,0 Z M 2.4003906,9.7246094 V 24 H 12 V 19.199219 H 7.1992188 V 9.7246094 Z M 12,19.199219 C 17.261827,19.199219 21.532433,14.970328 21.6,9.7246094 H 16.8 C 16.732089,12.315131 14.606809,14.400391 12,14.400391Z

adamrusted and others added 2 commits April 11, 2021 14:47
Co-Authored-By: Peter Noble <15157491+PeterShaggyNoble@users.noreply.github.com>
@adamrusted
Copy link
Member

Thanks for the updated path @PeterShaggyNoble - I've optimized it down to 3 decimals and pushed to this PR. Could another @simple-icons/maintainers take a look over this before we merge it?

Copy link
Member

@service-paradis service-paradis left a comment

Choose a reason for hiding this comment

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

LGTM 💯
Thanks everyone!

@service-paradis service-paradis merged commit 3cf1f11 into simple-icons:develop Apr 12, 2021
ericcornelissen added a commit that referenced this pull request Apr 18, 2021
# New Icons

- CLion (#5447)
- EditorConfig (#5237)
- FontBase (#5402)
- Hasura (#5452)
- Imou (#5458)
- Klarna (#5441)
- MediaTek (#5171)
- Mumble (#4874)
- Passport (#5054)
- PurgeCSS (#5400)
- Shelly (#5216)
- Storyblok (#5399)
- UpCloud (#5444)
- Verdaccio (#5450)
- Vite (#5401)

# Updated Icons

- Dior (#5314)
- Gitpod (#5425)
- GNU Bash (#5460)
- GNU Emacs (#5460)
- GNU IceCat (#5460)
- GNU Privacy Guard (#5460)
- Loom (#5378)
- Raspberry Pi (#5339)
- Trello (#5459)
- Xiaomi (#5426)
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.

None yet

6 participants