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

feat(documentation): adds open graph tags to storybook header #2205

Merged
merged 17 commits into from
Jan 31, 2024

Conversation

b1aserlu
Copy link
Contributor

@b1aserlu b1aserlu commented Nov 2, 2023

There is an anomaly. On the frist page of the review the home--docs the open graph tags seem to have no effect.

@b1aserlu b1aserlu linked an issue Nov 2, 2023 that may be closed by this pull request
Copy link

changeset-bot bot commented Nov 2, 2023

⚠️ No Changeset found

Latest commit: 34d7a07

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@swisspost-bot
Copy link
Contributor

swisspost-bot commented Nov 2, 2023

Preview environment ready: https://preview-2205--swisspost-design-system-next.netlify.app

@b1aserlu b1aserlu marked this pull request as ready for review November 2, 2023 14:05
Copy link
Member

@gfellerph gfellerph 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 we should use a different image as the preview. The one used now has overflowing text: https://www.opengraph.xyz/url/https%3A%2F%2Fpreview-2205--swisspost-design-system-next.netlify.app%2F

Here's a collection of splash screen variants, recommended is an image in the format 1200x630.

Figma https://www.figma.com/file/MsZXoWnhUoV1tPg3HboSlm/Design-System-Splash-Screen?type=design&node-id=6439%3A734&mode=design&t=vXlCbLHZjMRdH4AE-1

@b1aserlu
Copy link
Contributor Author

b1aserlu commented Dec 5, 2023

I made the image be a reference to our github code repository becasue a relative path cannot be used in the url. This should work in theory. However it cannot yet work in this preview as there is no image in the location atm as it would be added by this PR as well.

I just realised It would probably better to convert the image to a datatype so it has a smaller file size an svg would be ideal but I do not know how to convert a png to a svg. I mean there are convertors out there but I do not know which ones I am allowed to use.

@gfellerph
Copy link
Member

I just realised It would probably better to convert the image to a datatype so it has a smaller file size an svg would be ideal but I do not know how to convert a png to a svg. I mean there are convertors out there but I do not know which ones I am allowed to use.

You can choose the file type when exporting from figma. I don't know if all services support svg, I'd stick to png. The png can be further optimised by using https://tinypng.com/.

checked the documentation for twitter cards and saw that they had fallbacks on the respective open graph tags.
Copy link
Member

@gfellerph gfellerph left a comment

Choose a reason for hiding this comment

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

Looks good so far, but the splash screen has been changed by Rouven to reflect the accent colors change. Can you update the splash screen from the figma file?

also adjusts the name of the .png file
Copy link

sonarcloud bot commented Jan 25, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@b1aserlu b1aserlu merged commit ae00688 into main Jan 31, 2024
8 checks passed
@b1aserlu b1aserlu deleted the 806-add-open-graph-meta-tags-in-storybook-header branch January 31, 2024 13:02
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.

Add open-graph meta tags in storybook header
4 participants