search

jamulussoftware / jamuluswebsite

This is the GitHub Pages repository for the Jamulus main website. For the jamulus application source code, please visit jamulussoftware/jamulus.
https://jamulus.io
GNU Lesser General Public License v2.1
15 stars 81 forks source link

Add language selection dropdown #994

Closed ignotus666 closed 2 months ago

ignotus666 commented 3 months ago

Short description of changes Adds a language selection dropdown to the website. The website also detects what language the user's browser is in and initially displays itself in that language.

Scripts automating the addition of new languages have been updated to support adding the full language name + language code.

It also includes the changes from https://github.com/jamulussoftware/jamuluswebsite/pull/988 and https://github.com/jamulussoftware/jamuluswebsite/pull/990 so as to avoid conflicts between PRs and include all the changes here.

TODO after merging this:

Context: Fixes an issue? Related issues Fixes https://github.com/jamulussoftware/jamuluswebsite/issues/977, https://github.com/jamulussoftware/jamuluswebsite/issues/986 and https://github.com/jamulussoftware/jamuluswebsite/issues/992

Status of this Pull Request Should be ready pending a review.

What is missing until this pull request can be merged? Would be good if some more people can test it.

IMPORTANT: Weblate stuff should be merged first.

Does this need translation?

No.

ignotus666 commented 3 months ago

As commented in #993, the check fails because of lang code mismatches that will be fixed after this PR is merged. To test the language dropdown version of the website, links to working artifacts will be posted in comments below.

ann0see commented 3 months ago

What would a classic html select/option box with some styling be as alternative? It would be much better integrated in mobile OS and be standard compliant: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select

ann0see commented 3 months ago

I'd like to be part of the final review on this one.

pljones commented 3 months ago

Regarding avoidance of reliance on javascript:

That should be pretty transparent to the user - the benefit of the auto-detect (and then the widget having local storage) would be there for those who are happy to have javascript and local storage enabled but the functionality wouldn't be lost for other users.

ignotus666 commented 3 months ago

@ann0see, @pljones: I have a new version that doesn't do language detection nor uses javascript, and the css styling is moved to its own langsel.css file. The code is way more simplified. Other than not detecting the language automatically, it behaves exactly the same. So subject of course to a review, would you be happier if I replaced the current version with that?

ann0see commented 3 months ago

Yes.

gilgongo commented 3 months ago

We'll need to verify the accessibility of this at some point too. Not sure how best to do that though (I see there's some Aria tags though).

ann0see commented 3 months ago

As I said above, I'd assume that a select box is more accessible. But IDK if clicking easily works without JS. We could use a form.

ann0see commented 3 months ago

You mentioned somewhere that you used ChatGPT. Does this have any implications?

ignotus666 commented 3 months ago

I used Chatgpt to give me pointers when stuck (admittedly fairly often). I doubt this has any implications at all.

I still don't believe in the manual approach. Usually, I'd expect a dropdown box from HTML. This follows system standards on mobile and is probably the most accessible

If anything other than an entirely different approach is ultimately going to be rejected, I'd rather stop wasting my time instead of rearranging the deckchairs on this Titanic of a PR.

ignotus666 commented 3 months ago

Last try. It now uses standard HTML and is much simpler. I couldn't get the icon to integrate in the button without things messing up so I got rid of it. Artifact.

lang-select_1

ignotus666 commented 3 months ago

Added non-JS option. Artifact. WIth JS disabled you get the horizontal list of language codes (like it is now) instead of the dropdown.

gilgongo commented 2 months ago

@ignotus666 I'm not able to run this locally as I'm getting No such file or directory on the 1-[lang]-index.md files when I run jekyll serve. Do I need to tweak something?

ignotus666 commented 2 months ago

@gilgongo are you using the artifact I linked to above? A clone of this branch won't work.

gilgongo commented 2 months ago

@gilgongo are you using the artifact I linked to above? A clone of this branch won't work.

Oh OK. How do I view it with jekyll?

ignotus666 commented 2 months ago

Unzip it (twice; a second zipped folder emerges from the first). You should then have a folder called _site. CD into that and then use the jekyll serve command.

ann0see commented 2 months ago

As validity check, upload the generated HTML to https://validator.w3.org/#validate_by_upload

pljones commented 2 months ago

@jamulussoftware/designers @jamulussoftware/translators @jamulussoftware/maindevelopers Are we okay to merge this? If so, please approve.

gilgongo commented 2 months ago

@jamulussoftware/designers @jamulussoftware/translators @jamulussoftware/maindevelopers Are we okay to merge this? If so, please approve.

Just need to fix the html tags to make it do the stuff in I think #988

ann0see commented 2 months ago

Yes. And I need to check it locally.

ignotus666 commented 2 months ago

@ann0see the latest version for checking locally is here.

softins commented 2 months ago

@ann0see the latest version for checking locally is here.

I downloaded the artifact and unzipped it, but it seems rather tricky to check it locally without setting up a local web server with its own local domain to put the site at the root of a domain. There are a LOT of links in both <head> and <body> that are absolute instead of relative, either beginning with / or even with https://jamulus.io/, which means it can't be viewed properly from a sub-folder.

How do others check the next-release branch or PRs for it?

gilgongo commented 2 months ago

@softins It's bit convoluted but once you've unzipped it (twice) you can view it locally by building it with jekyll. I outlined the process here but YMMV.

softins commented 2 months ago

Thanks, I'll give it a go.

ann0see commented 2 months ago

language duplication

The selection box shows up twice. And I don't think it looks nice in the footer.

ann0see commented 2 months ago

@ignotus666 also please check with https://github.com/jamulussoftware/jamuluswebsite/pull/994#issuecomment-2223833457 multiple issues are showing up in the w3c validator.

ignotus666 commented 2 months ago

The selection box shows up twice. And I don't think it looks nice in the footer.

Oops, a stray line got left in there (but not in this PR). Fixed version is here.

also please check with https://github.com/jamulussoftware/jamuluswebsite/pull/994#issuecomment-2223833457 multiple issues are showing up in the w3c validator.

What file am I supposed to upload there, langselect.html? It also spits out a bunch of warnings and errors with the current (next-release) version.

ann0see commented 2 months ago

Ok. No you should upload the compiled index.html from the artifact and check for new errors then.

ignotus666 commented 2 months ago

Right. So the main complaint (that differs from the next-release errors) seems to be that "canonical" in lang="canonical" is more than 8 characters long... How are we supposed to get around that?

gilgongo commented 2 months ago

My understanding is that lang= needs to be hreflang="[langcode]" (eg hreflang="ko-KR") and that for links outside of the <head> section we don't need to have rel="canonical" and rel="alternate" as you point out (but could have rel="[something else]" if needed I guess). But I don't know what the difference is here between lang and hreflang... There seems to be some Google preference for hreflang.

ignotus666 commented 2 months ago

I traced the problem back to nav.html. I think it's sorted now. Link to artifact. Tell me if this one seems ok and then I'll add the changes to this PR.

gilgongo commented 2 months ago

That looks good. I don't know whether the links in the nav to things like "/wiki/Getting-Started" also need to have hreflang= in them but perhaps that's not needed with the rel= stuff in the head area.

ignotus666 commented 2 months ago

Here's what I found:

Use rel="canonical" in navigation links for the primary version of a page. Reserve hreflang for <link>elements in the <head> to specify alternate language versions of pages.

ignotus666 commented 2 months ago

Latest version: artifact.

ann0see commented 2 months ago

Ok. Thanks! It was a long ride but now hopefully it's all good. We'll probably need to check the site before publication for e.g broken links and so.

ignotus666 commented 2 months ago

It was a long ride

You can say that again... ;) . Right, now before unlocking Weblate or merging anything else here there are a couple of things that need doing (see description of this PR). Hopefully I'll have some time tonight to put it together.

gilgongo commented 1 month ago

@ignotus666 I'm looking the the rel=canonical/alternate stuff and I'm not sure it's correct. Shall I open a PR with a couple of tweaks to headtags.html? If so, would that be against release or next-release?

I also notice a rel="canonical" in the 1-*-index.md files which probably shouldn't be there ~- how best to remove that?~ that's in wiki/en/misc/1-index.md now I take it?

ann0see commented 1 month ago

In theory next-release. But I'd rather wait until the translations have settled even though it shouldn't have conflicts