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

Attempt at GeeksForGeeks logo #877

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

oOXxTNTxXOo
Copy link

@oOXxTNTxXOo oOXxTNTxXOo commented Oct 30, 2024

Thank you for your contribution! Before sending this Pull Request, please confirm the following:

  • You have read the contributing guidelines
  • The icon's size is under 1,024 Bytes
  • The layout of the SVG looks like this including newlines
<svg xmlns="http://www.w3.org/2000/svg"
aria-label="..." role="img"
viewBox="0 0 512 512"><path
d="m0 0H512V512H0"
fill="#fff"/> ... </svg>

If you have done the above, please send the Pull Request.

@oOXxTNTxXOo
Copy link
Author

this was my first attempt at creating a svg. hope i did ok

@Eiim
Copy link
Collaborator

Eiim commented Oct 30, 2024

This is a great start! Just a couple comments. First, fill="none" instead of fill="#ffffff00" is shorter and even slightly better-supported. Second, you have enough small deviations from the original logo to be noticable. Fortunately, it looks pretty easy to get a lot closer. Below is an overlay of your version in purple and the original in green.
image
Moving the left circle left and right circle right by .5px:
image
Moving top points of polygons down 2px:
image
Moving top of line down 3px:
image

I don't know that those were the optimal changes, but it looks a lot better. I haven't tried implementing this in an efficient way, just playing around in Inkscape. You might also consider combining your two polygons into one <path>. There might be more clever <path> options for the green parts as well, but it would certainly be harder.

Oh, and if you can remove the last line break so the </svg> is on the third line, that'd be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants