-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 language picker #1695
base: gh-pages
Are you sure you want to change the base?
feat: add language picker #1695
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't add the picker in the mobile, but I'll add it and remove the language part if you folks want. And I also like the picker. 😄 |
@cengizcmataraci |
Hello, sorry about the delay. Could you review again, please? I added the language picker for mobile. I tested for various devices in browser dev tools. @bjohansebas @carlosstenzel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this @cengizcmataraci, it really looks great
} | ||
|
||
#language-picker-menu #navmenu>li:first-child { | ||
display: flex !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try not to use !important
, please find a way to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the !important
Are you referring to the desktop version? Because we don’t display the language name in the mobile picker. And do we really think the Node.js website offers a better UX for the mobile picker? I find it a bit confusing. In our design, the user opens the picker solely to change the language, and once that’s done, it closes automatically. @bjohansebas |
I'm not an expert in UX/UI to determine if Node.js has the best mobile user experience, it was just a suggestion, so I'm okay with leaving it as it is, which I also think is great. |
Adding language picker to header
closes #1687