-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat : add new language picker component #2040
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 new language picker component #2040
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🚦 Lighthouse Results (Mobile & Desktop)
|
5deaa54
to
def57d6
Compare
…ar/expressjs.com into refactor-lang-picker
DELAY=10 | ||
|
||
echo "Checking Netlify preview: $PREVIEW_URL" | ||
for i in $(seq 1 $MAX_RETRIES); do | ||
if curl -s --head --max-time 5 "$PREVIEW_URL" | grep "200 OK" > /dev/null; then | ||
if curl -s -I "$PREVIEW_URL" | grep "HTTP/.* 200" > /dev/null; then |
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.
Correcting header status string. Now lighthouse audit is not running because of wrong status checking.
_includes/language-picker.html
Outdated
{% assign url_parts = page.url | split: '/' %} | ||
{% assign url_remainder = url_parts | slice: 2, url_parts.size | join: '/' %} | ||
{% assign current_lang = page.lang %} |
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 think this should stay, otherwise we won’t be able to know the current URL to keep the page where it was and only change the language.
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.
Right, thank you for pointing out.
Co-authored-by: bjohansebas 103585995+bjohansebas@users.noreply.github.com
Signed-off-by: Shubham Oulkar <oulkarshubhu@gmail.com>
@@ -12,11 +12,6 @@ | |||
<nav id="navbar" aria-label="primary"> | |||
<input id="q" placeholder="🔎 search"> | |||
<ul id="navmenu"> | |||
<li> | |||
<a href="/" id="home-menu" {% if page.menu=='home' %} class="active" {% endif %}> | |||
{{ site.data[page.lang].menu.home }} |
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.
Translations for this is removed. This link is redundant because express logo also points to homepage.
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.
LGTM, thanks
See #1933 (comment)
closes #1804
cc @bjohansebas