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 new logo #752

Merged
merged 24 commits into from
Oct 2, 2023
Merged

add twitter new logo #752

merged 24 commits into from
Oct 2, 2023

Conversation

apinet
Copy link
Contributor

@apinet apinet commented Sep 24, 2023

What kind of changes does this PR include?

  • Add an icon

Description

  • Add the new twitter svg logo next to the older one.

@changeset-bot
Copy link

changeset-bot bot commented Sep 24, 2023

🦋 Changeset detected

Latest commit: 3ac828d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Sep 24, 2023

Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit 3ac828d
🔍 Latest deploy log https://app.netlify.com/sites/astro-starlight/deploys/651b4336df80d3000843f81c
😎 Deploy Preview https://deploy-preview-752--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the 🌟 core Changes to Starlight’s main package label Sep 24, 2023
@HiDeoo
Copy link
Member

HiDeoo commented Sep 24, 2023

Thanks for your contribution. 🙌

This changes makes me wonder a few things:

  • Should this be added as a new logo or should it replace the current twitter one?
  • Should this have an impact on the social media icons displayed in the site header?
  • If yes, should twitter be renamed to x or both be kept?

Not sure what to think as x.com links redirects to twitter.com but twitter.com uses the new x logo on the official Twitter website, this is a bit confusing ^^ 🤔

@apinet
Copy link
Contributor Author

apinet commented Sep 24, 2023

If we replace twitter for x, we risk to break references to the old icon if some dev buddies used it directly. Maybe we could keep both for a few months and remove twitter icon after that?

Regarding social media icons I updated the pull request for supporting x as social media with the new icon and keeping twitter as is it.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for this @apinet! I think where you and @HiDeoo ended up makes sense for now, even if it is a bit odd having both, it lets people decide how much they want to humour the rebranding shenanigans 😅

One thing: I wonder if we should use x.com as the key here? We’ve discussed expanding our built-in icon support to include the full set of Unicons (the icon set Starlight uses currently) and it already includes a generic x icon that could clash. What do you think?

@HiDeoo
Copy link
Member

HiDeoo commented Sep 25, 2023

One thing: I wonder if we should use x.com as the key here? We’ve discussed expanding our built-in icon support to include the full set of Unicons (the icon set Starlight uses currently) and it already includes a generic x icon that could clash. What do you think?

Sounds fine by me.

I guess the remaining missing changes after updating the key would be updating docs/src/content/docs/guides/customization.mdx & docs/src/content/docs/reference/configuration.md to document it and adding a changeset.

@github-actions github-actions bot added i18n Anything to do with internationalization & translation efforts 📚 docs Documentation website changes labels Sep 30, 2023
@apinet
Copy link
Contributor Author

apinet commented Sep 30, 2023

Hi @HiDeoo. I have updated customization & configuration files in all available languages. Let me know if I miss something but I think w're good :)

Copy link
Member

@HiDeoo HiDeoo left a comment

Choose a reason for hiding this comment

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

Thanks for the update, this PR may requires a few changes:

  • Following Chris' comment, we should use x.com for the icon key to avoid a potential clash with the x icon already existing in the Unicons set.
  • Would you mind reverting the changes in d715b5f and only keep the changes for the English version (docs/src/content/docs/guides/customization.mdx & docs/src/content/docs/reference/configuration.md)? We usually only update the English pages and leave changes to the translations to our translators as this makes it easier for the Starlight translation tracker to keep track of the changes.

@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Oct 2, 2023 10:27pm

@github-actions github-actions bot removed the i18n Anything to do with internationalization & translation efforts label Oct 2, 2023
@delucis delucis added the 🌟 patch Change that triggers a patch release label Oct 2, 2023
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution @apinet! Hope you don’t mind I took the liberty of updating this PR with the last few changes so we can get this out in our next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 📚 docs Documentation website changes 🌟 patch Change that triggers a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants