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

fix: i18n for smaller screen #3113

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

TenzDelek
Copy link
Contributor

Fixes #2708

As per mentioned by @anshgoyalevil Discuss, have put the i8n in the dropdown.

image

Copy link

netlify bot commented Jul 24, 2024

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 30f1f30
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/66d04feabbda9b0008da1243
😎 Deploy Preview https://deploy-preview-3113--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.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Jul 24, 2024

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 36
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

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

@sambhavgupta0705
Copy link
Member

Can we something like kubevela.io ??
The selector needs to be inside of the dropdown only but we need to improve its UI

@TenzDelek
Copy link
Contributor Author

Can we something like kubevela.io ?? The selector needs to be inside of the dropdown only but we need to improve its UI

sure, will try to do that !!

@TenzDelek
Copy link
Contributor Author

Screenshot 2024-07-24 214448
Screenshot 2024-07-24 214503

cc @sambhavgupta0705

@TenzDelek
Copy link
Contributor Author

@sambhavgupta0705 do the repo has a specific command for linting. the pr seems to be failing due to it. what might be the cause of it. am i missing a step. wouldn't it be better to have a precommit hook for lint staged when we do the commit 🤔 happens same at PR

@sambhavgupta0705
Copy link
Member

Can we change the icon in side of language to anything in deutsche

@sambhavgupta0705
Copy link
Member

And increase the right margin of en and de also in the drop-down

@TenzDelek
Copy link
Contributor Author

And increase the right margin of en and de also in the drop-down

image
i believe you mean margin left.. , have given a margin of 2

@sambhavgupta0705
Copy link
Member

i believe you mean margin left.. , have given a margin of 2

Yes this only

@sambhavgupta0705
Copy link
Member

@sambhavgupta0705 do the repo has a specific command for linting. the pr seems to be failing due to it. what might be the cause of it. am i missing a step. wouldn't it be better to have a precommit hook for lint staged when we do the commit 🤔 happens same at PR

Lint cheeck passed in that PR ,there is a check fail which is because of change in npm and node version .We need to resolve it @akshatnema has more idea about it

@sambhavgupta0705
Copy link
Member

sambhavgupta0705 commented Jul 24, 2024

image
can we decrease the font size of the EN DE here

@TenzDelek
Copy link
Contributor Author

@sambhavgupta0705 do the repo has a specific command for linting. the pr seems to be failing due to it. what might be the cause of it. am i missing a step. wouldn't it be better to have a precommit hook for lint staged when we do the commit 🤔 happens same at PR

Lint cheeck passed in that PR ,there is a check fail which is because of change in npm and node version .We need to resolve it @akshatnema has more idea about it

sure , will wait for the response from his side..

@TenzDelek
Copy link
Contributor Author

image can we decrease the font size of the EN DE here

currently it is set to text-base, should we go with sm or xs

@sambhavgupta0705
Copy link
Member

currently it is set to text-base, should we go with sm or xs

For smaller screen yes and
image
remove this whitespace also

@sambhavgupta0705
Copy link
Member

@Mayaleeeee Can you please take a look at this

@sambhavgupta0705
Copy link
Member

/update

@Mayaleeeee
Copy link
Member

@Mayaleeeee Can you please take a look at this

Hello @TenzDelek @sambhavgupta0705, Instead of using codes like 'en' and 'de', let's use terms people are more familiar with, such as 'English' and 'Deutsch', for a more user-friendly experience and to reach a broader audience.

P.S. The position looks good on both the web and mobile.

Is there anything else you'd like me to review on this issue? cc @TenzDelek @sambhavgupta0705

@sambhavgupta0705
Copy link
Member

@TenzDelek push a review with these changes then we can look at this

@TenzDelek
Copy link
Contributor Author

@TenzDelek push a review with these changes then we can look at this

@sambhavgupta0705 check it once. have made some adjustment to the space x due to layout shift in the nav after the long form addition

@sambhavgupta0705
Copy link
Member

@TenzDelek I liked the design but we have a setup of using en and de so we need to change them also by replacing them with deutsch and english
cc: @anshgoyalevil

@TenzDelek
Copy link
Contributor Author

@TenzDelek I liked the design but we have a setup of using en and de so we need to change them also by replacing them with deutsch and english

cc: @anshgoyalevil

You mean the params? Didn't quite get you🤔

@sambhavgupta0705
Copy link
Member

yes the params only
currently it is redirecting us to https://deploy-preview-3113--asyncapi-website.netlify.app/deutsch which is not valid

A good approach will be to change the params from de to deutsch and en to english

@sambhavgupta0705
Copy link
Member

@TenzDelek any update?

@TenzDelek
Copy link
Contributor Author

@TenzDelek any update?

was bit busy past few days ! will look into this today or tomorrow

@TenzDelek
Copy link
Contributor Author

yes the params only currently it is redirecting us to https://deploy-preview-3113--asyncapi-website.netlify.app/deutsch which is not valid

A good approach will be to change the params from de to deutsch and en to english

can you help me with that a little bit.. what exactly we need to perform here. little bit confuse here. is it only changing file name here or ?
image
cc @sambhavgupta0705

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.

Enabling i18n feature for smaller screens
4 participants