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 KTM (#2985) #2997

Merged
merged 7 commits into from Jun 2, 2020
Merged

Conversation

irgusite
Copy link
Contributor

@irgusite irgusite commented Apr 28, 2020

ktm
Issue: Closes #2985
Add KTM logo

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

KTM logo taken from the home page of https://ktm.com

Edit: Updated png for updated icon

@irgusite irgusite mentioned this pull request Apr 28, 2020
@PeterShaggyNoble PeterShaggyNoble added the new icon Issues or pull requests for adding a new icon label Apr 28, 2020
Copy link
Contributor

@phatbhoy67 phatbhoy67 left a comment

Choose a reason for hiding this comment

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

Hey @irgusite thanks for submitting this PR. 👏

Before I can merge this one in I need you to make a change.

Currently the icon is 26.984px wide, can you please update so the width is 24px wide and make sure it is centered both horizontally and vertically after you make this change.

Let me know if you need any help with this 👍

@irgusite
Copy link
Contributor Author

irgusite commented May 5, 2020

Hi @phatbhoy67,

It should be okay now :)

@PeterShaggyNoble
Copy link
Member

Thanks for the update, @irgusite. However, this is now coming in a 23.901 wide.

@irgusite
Copy link
Contributor Author

irgusite commented May 5, 2020

Okay so I tried, but inkskape is either making a 24.001 px image or a 23.901 one....

@PeterShaggyNoble
Copy link
Member

Hmm ... that's odd. Could you let us know the steps you're following to resize and optimise the path 'til we see if can help you out?

@irgusite
Copy link
Contributor Author

irgusite commented May 5, 2020

Okay, I found why. In the transform menu, I checked the Scale proportionally option, which rounded the two values to match.

So to avoid the problem, I first scaled it proportionally, then unchecked the option and put the correct width.

Updating atm and pushing again. Should be fine now :)

@PeterShaggyNoble
Copy link
Member

Thanks for the quick update, @irgusite.

I'm seeing a couple of very small hairline differences between your version and the source but they are so insignificant, I would put it down to my tired, under-caffeinated eyes seeing things that aren't there if one of the other @simple-icons/maintainers said they couldn't see them!

@PeterShaggyNoble PeterShaggyNoble added awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed and removed changes requested labels May 5, 2020
@phatbhoy67
Copy link
Contributor

So to avoid the problem, I first scaled it proportionally, then unchecked the option and put the correct width.

Hey @irgusite, using the technique you described above I think will end up with a change in the aspect ratio of the icon.

I'm not sure how familiar you are with inkscape, but the technique I would use is as follows, if you look between the W and H boxes there is a little padlock icon you can toggle on and off to lock the aspect ratio.

With this particular icon, width is the biggest value so I would replace the W value by typing 24px into the W value box and pressing enter.

That should scale the icon to the correct values, in this case I end up with 24px W - 8.470px H.

That technique works whether W or H is the larger value in the original icon you are working with and is simpler than using the transform menu imho.

As @PeterShaggyNoble points out there are a few hairline differences that need to be corrected.

I found that using a precision value of 4 when optimising with the inkscape plugin will give a result of 24px W - 8.470px H, path below for reference:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path d="M0 15.735h3.3539l.843-2.0604 1.5507 2.0605h7.2245l2.234-2.0814-.3722 2.0814h2.8309l2.335-2.0597-.3197 2.0597h3.0522L24 9.9894h-3.068L18.4458 12.18l.4802-2.1907h-2.9417l-3.209 3.2161 1.3414-3.9371h4.9077l.2248-1.0035H6.3815L6.003 9.2684h4.7322l-1.9939 5.054-1.5723-2.066 2.717-2.267H7.612l-2.7876 2.229.9385-2.229H2.44L0 15.735"/></svg>

Hope that helps you correct the small differences, and thanks again for your contribution. 👍

@PeterShaggyNoble PeterShaggyNoble added changes requested and removed awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed labels May 6, 2020
@irgusite
Copy link
Contributor Author

Thank you for you input,

I re-downloaded the original icon and used the top W and H boxes, finished with a 24.000 by 7.470 image.

and used a precision of 4 this time

Hope this time everything is okay :)

@PeterShaggyNoble
Copy link
Member

The version in this commit with precision 3 actually looked pretty spot-on to me, @irgusite but I'll let @phatbhoy67 double check that before asking you to revert. Not that there's a significant difference between the file sizes of each option.

icons/ktm.svg Outdated
@@ -0,0 +1 @@
<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>KTM icon</title><path d="M0 15.735h3.3539l.843-2.0605 1.5507 2.0606h7.2245l2.234-2.0814-.3722 2.0814h2.8309l2.335-2.0597-.3197 2.0597h3.0522L24 9.9894h-3.068L18.4458 12.18l.4802-2.1907h-2.9417l-3.209 3.2161 1.3414-3.9371h4.9077l.2248-1.0035H6.3815L6.003 9.2684h4.7322l-1.9939 5.054-1.5723-2.066 2.717-2.267H7.612l-2.7876 2.229.9385-2.229H2.44L0 15.735" /></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

My vote also goes to the precision 3 version, could you revert the change @irgusite (or use the suggestion below)

Suggested change
<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>KTM icon</title><path d="M0 15.735h3.3539l.843-2.0605 1.5507 2.0606h7.2245l2.234-2.0814-.3722 2.0814h2.8309l2.335-2.0597-.3197 2.0597h3.0522L24 9.9894h-3.068L18.4458 12.18l.4802-2.1907h-2.9417l-3.209 3.2161 1.3414-3.9371h4.9077l.2248-1.0035H6.3815L6.003 9.2684h4.7322l-1.9939 5.054-1.5723-2.066 2.717-2.267H7.612l-2.7876 2.229.9385-2.229H2.44L0 15.735" /></svg>
<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>KTM icon</title><path d="M0 15.735h3.354l.843-2.06 1.55 2.06h7.225l2.234-2.081-.372 2.081h2.83L20 13.675l-.32 2.06h3.052L24 9.99h-3.068l-2.486 2.191.48-2.19h-2.942l-3.209 3.216 1.342-3.938h4.907l.225-1.003H6.381l-.378 1.003h4.732l-1.994 5.054-1.572-2.066L9.886 9.99H7.612l-2.787 2.23.938-2.23H2.44L0 15.735"/></svg>

@PeterShaggyNoble
Copy link
Member

I hope you don't mind but, in the interest of clearing out some PRs, I've updated your branch with the requested changes.

@PeterShaggyNoble PeterShaggyNoble merged commit c7bf078 into simple-icons:develop Jun 2, 2020
ericcornelissen pushed a commit that referenced this pull request Jun 7, 2020
# New Icons

- GeeksforGeeks (#3072)
- Jasmine (#3152)
- KTM (#2997)
- Metro de la Ciudad de México (#3156)
- Metro de Madrid (#3153)
- N26 (#3133)
- Prime Video (#3025)
- Scratch (#2872)
- Shenzhen Metro (#3158)
- Tokyo Metro (#3157)
- TuneIn (#3115)
- VirusTotal (#3182)

# Updated Icons

- Bitwarden (#3150)
- SpaceX (#3159)
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.

KTM
4 participants