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: add features list translations #2074

Merged
merged 4 commits into from
Aug 21, 2023
Merged

Conversation

anshgoyalevil
Copy link
Member

Description
This PR adds the translations for the homepage's features section.

  • Cypress tests are updated to pass

Related issue(s)

part of #2039

@netlify
Copy link

netlify bot commented Aug 17, 2023

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit dd0e9bf
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/64e3624172da770008c53728
😎 Deploy Preview https://deploy-preview-2074--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@anshgoyalevil anshgoyalevil changed the title Add features list translations feat: Add features list translations Aug 17, 2023
@anshgoyalevil
Copy link
Member Author

//cc @magicmatatjahu 🚀

@anshgoyalevil anshgoyalevil changed the title feat: Add features list translations feat: add features list translations Aug 17, 2023
@github-actions
Copy link

github-actions bot commented Aug 17, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 29
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-2074--asyncapi-website.netlify.app/

@@ -1,50 +1,56 @@
export const features = [
Copy link
Member

Choose a reason for hiding this comment

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

@anshgoyalevil

Do we need that file when we have this content inside locales? Except that question, everything is good :)

Copy link
Member Author

@anshgoyalevil anshgoyalevil Aug 21, 2023

Choose a reason for hiding this comment

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

Yes. Actually we are using the ids of those features to create unique translation keys to fetch those from the locales. It would also act like a first source of truth, and cypress testing, since parsing from the json would not be convenient in cypress tests

Copy link
Member Author

@anshgoyalevil anshgoyalevil Aug 21, 2023

Choose a reason for hiding this comment

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

Like here for example

{t(`features.${feature.id}.name`)}

Copy link
Member

Choose a reason for hiding this comment

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

So maybe put another texts as values to keys, not whole content, ok?

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't get that. Please clarify. Do you mean creating another array of keys inside the file itself rather than importing features.js and using it for keys?

Copy link
Member

@magicmatatjahu magicmatatjahu Aug 21, 2023

Choose a reason for hiding this comment

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

I see what you did :) Only remove not needed keys from /features map, like description etc, only leave id, label, href etc, what you exactly used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done with the changes 🚀

@magicmatatjahu
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit e11ab6e into asyncapi:master Aug 21, 2023
@anshgoyalevil anshgoyalevil deleted the tr2 branch August 21, 2023 13:13
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.

3 participants