zonemaster / zonemaster-gui

The Zonemaster GUI - part of the Zonemaster project
Other
14 stars 23 forks source link

Use a drop down menu to select between languages #337

Closed ghost closed 1 year ago

ghost commented 2 years ago

Purpose

Zonemaster supports more and more languages, and the GUI displays all of them on a row. This could lead to eating a lot of space. This PR replaces this row with a drop down menu (a \<select> element) with an icon representing language selection (icon from fork awesome).

image

Context

Addresses a comment made when discussing the possibility to provide a new page for the "History" (PR #333) where it was pointed out that in some locale, the menu will span a lot of available space (almost eating on the language selection space).

Changes

Remove the horizontal language list in the header.

How to test this PR

Use the new drop down language selection menu to switch across languages. The page should be updated accordingly.

matsduf commented 2 years ago

What does then language icon add? Is the width of the language menu fixed or adjusted to the longest language name?

ghost commented 2 years ago

For this to be really good, the width of the field must adjust to the longest language name (and that is maybe already so).

This is already so, you can test applying the following diff:

diff --git a/src/environments/common.ts b/src/environments/common.ts
index 122ee52..66ce50a 100644
--- a/src/environments/common.ts
+++ b/src/environments/common.ts
@@ -12,9 +12,10 @@ export const common = {
     'fi': 'Suomi',
     'fr': 'Français',
     'nb': 'Norsk (bokmål)',
-    'sv': 'Svenska'
+    'sv': 'Svenska',
+    'aa': 'A very long language name to test field width'
   },
-  enabledLanguages: [ 'da', 'en', 'es', 'fi', 'fr', 'nb', 'sv' ],
+  enabledLanguages: [ 'aa', 'da', 'en', 'es', 'fi', 'fr', 'nb', 'sv' ],
   configUrl: 'assets/app.config.json',
   pollingInterval: 5 * 1000,
 }

I do not think that the language icon adds anything, on the contrary it distracts.

Hard to settle on the icon usage, I think some people might find it distracting as other might find it helpful (such as myself).

marc-vanderwal commented 2 years ago

I’m fine with the icon. I don’t find it distracting at all and as long as concerns about accessibility are addressed, I’m fine with it.

matsduf commented 2 years ago

Hard to settle on the icon usage, I think some people might find it distracting as other might find it helpful (such as myself).

The icon is just another graphical element in the menu. Nothing happens if you click on it. It is not obvious what it "means". What does it add?

ghost commented 2 years ago

Nothing happens if you click on it. It is not obvious what it "means".

I've just updated the code so that it's linked to the select (as suggested above). And I also provided a title, that is shown on hovering it.

matsduf commented 2 years ago

I still do not see the purpose of the icon. If I click on it, nothing meaningful happens. When hoover over it, I do get a tool-tip, that is partially covered by my mouse pointer (not seen in the screen shot). The text in the tool-tip is very small.

The icon could make sense if the menu is not shown until you click on the icon. And then the tool-tip should be clearer.

image

There is another place where tool-tip is used, but then it looks very different. I think they should look the same.

image
hannaeko commented 1 year ago

I think that with the growing number of language we should consider moving forward with this proposal. As for the comments raised:

I still do not see the purpose of the icon. If I click on it, nothing meaningful happens. When hoover over it, I do get a tool-tip, that is partially covered by my mouse pointer (not seen in the screen shot). The text in the tool-tip is very small.

The icon make sense as it provides a visual aid to indicate where to change the language when an other language is selected. We could also make the dropdown expend on click but I do not think that this should be blocking this PR.

There is another place where tool-tip is used, but then it looks very different. I think they should look the same.

This is not a tooltip but a "title". Titles provide accessibility and the style is dependent on the browser / operating system.

matsduf commented 1 year ago

I think that with the growing number of language we should consider moving forward with this proposal.

I agree that a drop-down menu with the language name is an improvement. Today's language codes are cryptic.

I still do not see the purpose of the icon. If I click on it, nothing meaningful happens. When hoover over it, I do get a tool-tip, that is partially covered by my mouse pointer (not seen in the screen shot). The text in the tool-tip is very small.

The icon make sense as it provides a visual aid to indicate where to change the language when an other language is selected. We could also make the dropdown expend on click but I do not think that this should be blocking this PR.

If the icon is self-explaining then it would be better not to show the menu until you have clicked on the icon. The title that comes up when hoovering on it is "Change interface language". The word "interface" does not add anything and can be hard to understand. Just "Change language" is shorter and easier to understand.

hannaeko commented 1 year ago

If the icon is self-explaining then it would be better not to show the menu until you have clicked on the icon.

I think showing the two of them is better. I see the icon as a label of the select here, indicating that this select is used for language.

The word "interface" does not add anything and can be hard to understand. Just "Change language" is shorter and easier to understand.

I don't mind if we change the wording. I also don't feel like "interface" adds much but I am fine either way.

matsduf commented 1 year ago

I will review this again, but I can see that the develop branch has changed since this was created. Can this be rebased on current develop?

ghost commented 1 year ago

Can this be rebased on current develop?

Done.

ghost commented 1 year ago

Rebased on top of develop to fix a conflict.

ghost commented 1 year ago

rebased, please re-review