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 twitter x new logo #761

Merged
merged 2 commits into from
Oct 9, 2023
Merged

Add twitter x new logo #761

merged 2 commits into from
Oct 9, 2023

Conversation

0APOCALYPSE0
Copy link
Contributor

No description provided.

@jmb
Copy link
Contributor

jmb commented Oct 3, 2023

Hi, your submission doesn't quite follow the guidelines with the following layout and elements required:

<svg xmlns="http://www.w3.org/2000/svg"
aria-label="..." role="img"
viewBox="0 0 512 512"><rect
width="512" height="512"
fill="#fff"/>...</svg>

@0APOCALYPSE0
Copy link
Contributor Author

Will fix it

@0APOCALYPSE0
Copy link
Contributor Author

@jmb Hi, Sorry I was busy that is why not able to fix it on time. But now I have updated the SVG and it is following the above guidelines.
Now SVG is able to fit inside a view box of 0 0 512 512.
Could you please review and approve?
Thanks

@jmb
Copy link
Contributor

jmb commented Oct 6, 2023

Sorry, some more comments (and I'm not sure I have the permissions to review/approve)...

I don't think there is any need for width, height or version. Also it should follow the guideline template image and be sized to fit the green circle.

@0APOCALYPSE0
Copy link
Contributor Author

0APOCALYPSE0 commented Oct 7, 2023

@jmb Adjusted,
I am also attaching the SVG here

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512" role="img" aria-label="X"> <circle fill="#f43" r="256" cx="256" cy="256"/> <circle fill="#fd0" r="217.5" cx="256" cy="256"/> <circle fill="#5d6" r="192" cx="256" cy="256"/> <path d="m0 0H512V512H0" fill="none" /> <path transform="translate(121, 120)" style=" stroke:none;fill-rule:nonzero;fill:rgb(0%,0%,0%);fill-opacity:1;" d="M 160.6875 114.269531 L 261.199219 0 L 237.382812 0 L 150.105469 99.21875 L 80.398438 0 L 0 0 L 105.410156 150.035156 L 0 269.863281 L 23.820312 269.863281 L 115.984375 165.085938 L 189.601562 269.863281 L 270 269.863281 L 160.679688 114.269531 Z M 128.0625 151.355469 L 117.382812 136.414062 L 32.402344 17.535156 L 68.988281 17.535156 L 137.566406 113.476562 L 148.246094 128.414062 L 237.394531 253.121094 L 200.808594 253.121094 L 128.0625 151.363281 Z M 128.0625 151.355469 "/> </svg>

image

@jmb
Copy link
Contributor

jmb commented Oct 7, 2023

Looks good now! 👍🏻

Copy link
Contributor

@jmb jmb left a comment

Choose a reason for hiding this comment

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

I think this looks good now, but it could be reduced further by removing the style attribute completely and applying the transform to the coordinates (using: https://yqnn.github.io/svg-path-editor/ can help here!) You could also probably round the coordinates to 1 or 2 decimal places. 😃

@0APOCALYPSE0
Copy link
Contributor Author

0APOCALYPSE0 commented Oct 7, 2023

@jmb Thanks for suggesting the life-saving tool for SVG editing :)
I have updated the SVG.

Just one query:
Could you please guide me about the steps you follow for creating SVG icons? I mean how do you create SVG icons that can fit inside a viewbox of 0 0 512 512 and radius of 192px?

@jmb
Copy link
Contributor

jmb commented Oct 7, 2023

I generally open the guideline.svg in Inkscape and then paste the new icon in, adjust things to fit, delete the guideline bits and then use the online tool and a text editor to tweak.

Copy link
Contributor

@jmb jmb left a comment

Choose a reason for hiding this comment

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

This looks great now! I think @edent will need to merge.

@Eiim
Copy link
Collaborator

Eiim commented Oct 8, 2023

Just hopping in now. It looks like in the updates you broke the guidelines format again. Also, there were a couple more optimizations I wanted to do. How about this code?

<svg xmlns="http://www.w3.org/2000/svg"
aria-label="X" role="img"
viewBox="0 0 512 512"><rect
width="512" height="512"
fill="#fff"/><path d="M281.7 234.3 382.2 120h-23.8l-87.3 99.2-69.7-99.2H121l105.4 150L121 389.9h23.8L237 285.1l73.6 104.8H391ZM153.4 137.5H190l168.4 235.6h-36.6Z"/></svg>

@0APOCALYPSE0
Copy link
Contributor Author

@Eiim updated

@edent
Copy link
Owner

edent commented Oct 9, 2023

Looks good! Please can you regenerate the README then I'll merge.

updated

added white background

updated icon

adjusted icon

updated path

updated

updated readme
@edent edent merged commit 197751f into edent:master Oct 9, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants