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

Language dropdown #224

Merged
merged 6 commits into from
Sep 17, 2021
Merged

Language dropdown #224

merged 6 commits into from
Sep 17, 2021

Conversation

ericswpark
Copy link
Contributor

@ericswpark ericswpark commented Jun 29, 2021

Needs more CSS styling and maybe JS to handle dropdown

Closes #169

@ericswpark ericswpark changed the title Language dropdown (#169) Language dropdown Jun 29, 2021
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@lxndrblz lxndrblz self-requested a review July 1, 2021 19:10
@lxndrblz lxndrblz self-assigned this Jul 1, 2021
@lxndrblz lxndrblz added the enhancement New feature or request label Jul 1, 2021
@lxndrblz
Copy link
Owner

lxndrblz commented Jul 4, 2021

@ericswpark Please apologize the delay. I tried it and out confirmed it is working.

However, I have some suggestions on which I would like to have your insights.

What would you think about keeping the language links/buttons consistent and only replacing the links to the translated page or the default site as a backup? This would avoid that the options within the menu bar are constantly changing, which might be confusing to the reader of your website.

Secondly, I would like to keep the current setup of simply nesting the language choices as menu items (if possible), as this ensures the choices are also nicely presented within the mobile menu. Would you mind telling me about your intention of moving the items to a separate dropdown menu?

Thanks again for your PR. I am sure together; we can get this merged soon

@ericswpark
Copy link
Contributor Author

@lxndrblz Hi, thanks for reviewing the PR.

I think the PR's implementation of showing the currently available translations for a given page is better. I also considered listing all languages and redirecting to the home page if a given page doesn't have a translation, but I think that's even more confusing for users. By showing available translations on a given page we can avoid that.

As for moving the language toggle to a separate dropdown, this is because placing the language switcher with the menu items is very confusing. IMHO, navbar items are for navigating between pages - if a navbar item changes the language of the page then it should look distinct as to avoid confusion.

For making the language dropdown presentable on mobile, could we possibly implement a navbar item dropdown similar to how Bootstrap does it? https://getbootstrap.com/docs/5.0/components/navbar/ We can have a navbar item that is titled "Language" or something like that and have language options presented as dropdown items. That way the language menu items are distinct from the rest of the navbar/page items, and it solves the theming problem on mobile. What do you think?

@lxndrblz
Copy link
Owner

lxndrblz commented Jul 10, 2021

@ericswpark Thanks for the explanation. This really helps me to better understand your reasoning. I think your suggestion with the dropdown is also among my current favourites. I am currently trialling different options and will report back once I have something substantial to show.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ericswpark
Copy link
Contributor Author

@lxndrblz please check out the added commits. I added CSS styling so it actually looks like a dropdown now.

Here's a video of the dropdown working. Sorry for the terrible quality:

rec.mp4

I do have some more ideas for the dropdown, such as adding country flags for each language and the box shape around the dropdown options, but I think this PR would get too big. Can we merge this first and add other changes as a separate PR/commit batch?

@lxndrblz
Copy link
Owner

lxndrblz commented Aug 31, 2021

@ericswpark Thanks for the update and please apologize the wait. I'll try it out tomorrow - the video already looks promising. Hopefully, soon I can dedicate more time again to to this project. :)

I like the idea of the country flags. I'll see if I can incorporate these.

@lxndrblz
Copy link
Owner

lxndrblz commented Sep 6, 2021

@ericswpark I was playing around all day with the PR and tried different stylings. The best outcome I had so far was this one:

image

However, in mobile it looks quite odd and the hover usability is quite poor. I am currently leaning on placing the languages side by side as menu items and just adding the redirect functionality you had included. I will play around with that tomorrow.

On another note: I'd also tried including flags as visual indicators for the languages. However, after some research I found out that from a usability issue flags should be avoided. For example, English is spoken in the UK, US, Canada, Philippines, and more countries/territories. It might offend certain user groups if only a Union Jack was displayed. More on this issue can be found here.

@ericswpark
Copy link
Contributor Author

ericswpark commented Sep 6, 2021

@ericswpark I was playing around all day with the PR and tried different stylings. The best outcome I had so far was this one:

image

This is exactly what I had in mind! Looks great :D

Edit: IMO the list should not contain the currently selected language I think?

However, in mobile it looks quite odd and the hover usability is quite poor.

Instead of using hover, maybe we could make it work like a button? Sort of like the ellipses button on GitHub comments:

Screen_Recording_20210907-020318_Firefox_1.mp4

On another note: I'd also tried including flags as visual indicators for the languages. However, after some research I found out that from a usability issue flags should be avoided. For example, English is spoken in the UK, US, Canada, Philippines, and more countries/territories. It might offend certain user groups if only a Union Jack was displayed. More on this issue can be found here.

Didn't even consider this. Thank you for the heads-up!

@ericswpark
Copy link
Contributor Author

@lxndrblz if you have time could you please push up the dropdown implementation you posted in the last comment as a separate branch? That one looks great and it was exactly what I was hoping to create with the PR (but couldn't due to my horrible webdev skills haha). It should also work for mobile if we change the hover implementation to a tap implementation.

@lxndrblz
Copy link
Owner

lxndrblz commented Sep 16, 2021

Let me push the changes to this PR and we can work on it together.

@lxndrblz lxndrblz merged commit 276daf8 into lxndrblz:master Sep 17, 2021
@lxndrblz
Copy link
Owner

@ericswpark Sorry for the confusion. I've accidentally merged the PR, instead of pushing to it. 😰

As it has not been fully functional (mobile and Arabic are a bit wonky), I had to revert the commit which also closed this PR.

You can find the current version in the branch called languagedropdown. It should look like this:

image

@ericswpark
Copy link
Contributor Author

@lxndrblz No worries. Can you re-open #169 so we can track the progress of the branch there?

@lxndrblz
Copy link
Owner

@ericswpark I've just reopened it. Please see the original issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better language dropdown
2 participants