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
Added Toshiba icon #1774
Added Toshiba icon #1774
Conversation
Hi @kaleeaswari thanks for your work on this PR, can you let me know what source you used for the .svg? Looking at the .svg you submitted you will need to make some changes. Did you follow the contributing guide instructions? The .svg should have a single path, without colour or other attributes and be sized at 24 x 24. It should also be optimised. If you need any help or tips on how to do that let me know. As far as the logo itself, you have used the full Toshiba wordmark. Perhaps it is a good idea to use just the "T" like the favicon on the Toshiba website, what do the @simple-icons/maintainers think? |
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.
@phatbhoy67 I think that in this case the brand themselves use their wordmark as their identifying mark - although they use the "T" as a favicon, this doesn't seem to be used elsewhere. All their products and websites use the full wordmark, so that's probably what we also want to use. Good call on bringing it up, though.
@kaleeaswari Could you please update the PR to meet our contribution guidelines (as linked by @phatbhoy67 above)?
Have committed new file created using Inkscape and optimized using SVGOMG. |
Hi @kaleeaswari thanks for making the changes, however there are a few changes you still need to make to your .svg file. The easiest thing to do would be to open it up again in Inkscape and edit it. Are you quite familiar with Inkscape? If not I can list some steps you need to take to achieve the result in detail, let me know. In summary the steps are:
That should fix some of the issues. Then you will need to optimise it and annotate it. You can annotate it easily using a normal text editor. Hope that helps, let me know if you need any further guidance 👍 |
Thanks for the detailed steps phatbhoy67. Am new to inkscape, have followed the steps listed and committed back. |
Thanks for updating the file @kaleeaswari It looks like we are nearly there. The icon is still too big, I think you may have set the height to 24px rather than the width. If you open the file back up in Inkscape, select the icon, make sure the aspect ratio is locked (click the padlock in between the W and H) and set the width to 24px. Then center the icon both vertically and horizontally on the page. Next, to ensure you have only one path, with the icon selected, go to the "Path" menu and click "union". Nothing should change visually when you do this. Save the file and upload it to svgomg check it looks ok , if so, click the "markup" button. There should be only one If that is the case press the "download" button. Open that file in a text editor. The properly annotated final file should look like this (I've removed the path data to shorten it in the example)
To be clear that means you will have to manually delete the That should be you good to go. Good luck and let me know if you get stuck 👍 |
Hey @kaleeaswari the .svg looks good, well done and thanks for your work making the requested changes 👍. The reason the checks are still failing is because you have used lowercase letters in the hex value in the _data/simple_icons.json file. If you change the |
Thanks much @phatbhoy67 for helping out with detailed explanations. |
Hey @kaleeaswari thanks for your patience, persistence and effort on this PR, really appreciated the contribution 👍. I will ask @birjolaxew to review it, hopefully it will be merged in soon. 👏 |
Thanks @phatbhoy67.. Looking forward for merge. |
👍 Looks good! Thanks a lot for all your work on updating this @kaleeaswari and @phatbhoy67! |
Issue: #1661
Checklist
_data/simple-icons.json
viewbox
is0 0 24 24
Description