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

Updated Twitter (X) icon #74666

Closed
wants to merge 2 commits into from
Closed

Updated Twitter (X) icon #74666

wants to merge 2 commits into from

Conversation

blindaks
Copy link
Contributor

@blindaks blindaks commented Oct 7, 2023

Closes #74494

@blindaks
Copy link
Contributor Author

blindaks commented Oct 8, 2023

@Esh07 Please review this PR.

Copy link
Contributor

@Esh07 Esh07 left a comment

Choose a reason for hiding this comment

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

Please update the link to this and it will resolve the problem.

https://edent.github.io/SuperTinyIcons/images/svg/x.svg

@@ -105,7 +105,7 @@ jobs:
// social logo
const repo_logo = "https://avatars0.githubusercontent.com/u/65761570?s=88&u=640f39b808c75c6b86460aa907dd030bcca2f3c7&v=4"
const slack_logo = "https://edent.github.io/SuperTinyIcons/images/svg/slack.svg"
const twitter_logo = "https://edent.github.io/SuperTinyIcons/images/svg/twitter.svg"
const twitter_logo = "https://fontawesome.com/icons/x-twitter?f=brands&s=solid&pc=%23ffffff"
Copy link
Contributor

Choose a reason for hiding this comment

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

The link you added is not an icon, but a webpage that shows the icon. We need an icon that is a media file, like this one.

For example, compare these two links:

Do you see the difference? The first one is a webpage and the second one is a media file. We need a media file.

The link we are using for the icon is from SuperTinyIcons and there is a PR pending and ready to merge:

We can update the link once that PR gets merged, as it follows the guidelines for the resolution and aspect of the media. Otherwise, linking an SVG from another website could break the layout of the content.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for explaining.

Copy link
Contributor

@Esh07 Esh07 left a comment

Choose a reason for hiding this comment

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

@blindaks thank you for making changes and your contributions.

Looking good 👍🏼 . Ready to merge.

Thanks

@blindaks
Copy link
Contributor Author

@Esh07 @tmsagarofficial Please merge this PR

@Esh07
Copy link
Contributor

Esh07 commented Oct 13, 2023

Hi @blindaks,

Apologies for the delay. The repository has limited people with write access, so it might take some time for your PR to get merged. Once the maintainers are free and have time to go through the PRs, yours will be reviewed and merged.

Happy coding!

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.

Change the twitter logo to X
4 participants