Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add language selection dropdown #994
Add language selection dropdown #994
Changes from 14 commits
18a7af9
0c930bb
13f9024
e4691de
18b4a9b
8330b98
bb86c3c
f7606cb
511d4f7
aade0e0
4527696
0178a3b
15510fd
5d83779
6d642e9
5e0fda6
98766c2
88c17bb
03646ac
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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.
@gilgongo I'm not sure if/how something has to be added here for the canonical links.
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.
As long as the markup shows any link that goes to a
site.active_lang
page ascanonical
and (I think less importantly) any other langs asalternate
, we're OK.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.
Whoops, @ignotus666 sorry I meant
site.default_lang
(if that's a thing?)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.
Only just got round to looking properly.
The markup seems to be coming out as
lang="canonical"
andlang="alternate"
here when I view it with Jekyll. They need to behreflang="[lang]"
with the actual lang code of the URL.When the currently selected language isn't
site.default_lang
, URLs need to haverel="alternate"
, otherwise they arerel="canonical"
(which I think is happening?)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.
@gilgongo did you test what happens with JS disabled? Does it work as intended 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.
@ann0see Ok, it's simple enough for non-JS mode, but seems to be tricky for JS mode without a script. Is JS mode good as it is or do we also want the currently selected language to be non-clickable in the dropdown?
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.
Yes. I think it's only needed for non JS mode
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.
Ok done. Artifact is at the bottom.
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.
@ann0see I can't for the life of me figure out how to stop it doing that. Nothing I try changes that behaviour.
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 is fixed now.