rhbz#1888697 list important major languages #2935

sdp5 · 2020-10-19T15:49:57Z

Listing the most important major languages make them super-easy to find for international users. (lesser scroll)
Identification of those languages is a challenge, however we may refer gnome-control-center.

Implementation

One option is to join (prepend) those identified languages to locales in pyanaconda/ui/gui/spokes/welcome.py. Here, we may have separators to avoid confusion: separate local/national languages to common/major ones. So, major languages | national languages | other languages. However, in a many cases the count of national languages remain less than 4 which may leave the separators very close! And may not look nice.
Another option could be major languages + national languages | other languages. I feel this may create confusion.
As we are identifying local/national languages already to help ease user's search (for their languages), it makes sense to populate the first list with identified common languages in the event when territory lookup fails instead of just en_US.

Should we try moving get_common_languages to langtable instead?

Please feel free, to suggest better implementation way.

pep8speaks · 2020-10-19T15:50:04Z

Hello @sundeep-anand! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-13 15:18:13 UTC

jkonecny12 · 2020-10-21T07:58:23Z

/packit build

jkonecny12 · 2020-10-21T08:02:58Z

Hello @sundeep-anand thanks for your PR. Yes, this seems something which should be part of the langtable. What do you think @mike-fabian ? Is it appropriate to have list of these languages in langtable or is there a better solution?

mike-fabian · 2020-10-21T12:08:16Z

Yes, we can add this to langtable.

I must say that I don't like such a list of common languages very much, it always annoys me a bit when I open gnome-control-center. My own native language (German) is in the list of common languages, but I feel that it might annoy people that their language isn't.

But anyway, I am not against adding something like that to langtable, it is probably better to have it there than in anaconda.

juhp · 2020-10-21T12:28:01Z

Just to comment, I believe Gnome used number of speakers as the main criteria for the top listed languages.

VladimirSlavik · 2020-10-21T12:57:38Z

As to the technical matters:

I tested this and it works, probably as intended. Thank you!
The language support spoke does not seem to be affected, just as it should be.
Yes, langtable is a better place for such a list. Thank you Mike for considering this.
This happens only if geolocation is off. I think it's an important distinction for the rest of the discussion. I am also not sure if that was the intention?
The discussion in first comment seems for more than the one commit. Is this part of some larger plan? Will more be added? Having a full picture would help.

Apart from that, there is the "soft" side of this. It's a potentially loaded decision / change.

Someone (@juhp?) asked for this and someone else (@sundeep-anand) implemented this. Bug has no links to anywhere else. So two people definitely think this is a good idea. Who else?
Do we need to run this by some sort of Fedora board?
In the Gnome thing at least there is some balance: The list is initially very short and lists only the most common stuff, so that it fits on the screen space. So many benefit from having the choice "above the fold". Others can expand the list for everything. Here, we already do have a "full" list in some sense, there is no clear split between these important items and the rest. Also note the Gnome list changes to alphabetical after expanding.
Should we also order these? How? Alphabetically? By estimated amount of speakers? By expected % of speakers using Fedora? By expected share of computer users among speakers of these languages?

At any rate, I am told @M4rtinK already had some discussions about this, so switching to him...

sdp5 · 2020-10-22T06:29:38Z

depends mike-fabian/langtable#12
edit: common languages are being appended to the priority languages. (determined by geolocation)
a discussion around the use case is here.

Zokormazo · 2020-10-22T06:58:25Z

pyanaconda/ui/gui/spokes/welcome.py

@@ -162,6 +162,10 @@ def initialize(self):
        # get language codes for the locales
        langs = [localization.get_language_id(locale) for locale in locales]

+        # get common languages and append them here
+        common_locales = localization.get_common_locales()
+        langs += [localization.get_language_id(locale) for locale in common_locales]


Will this duplicate languages on territories where a common language is a territory language too?

no.. the language will appear once.. in territory and be omitted from common.

poncovka

Otherwise, the code looks good to me. Thank you!

poncovka · 2020-10-22T08:42:49Z

pyanaconda/localization.py

@@ -72,6 +72,11 @@ def get_language_id(locale):
    return langtable.parse_locale(locale).language


+def get_common_locales():
+    """Return common locales to prioritize them"""
+    return langtable.list_common_locales()


Please, update the required langtable version in the anaconda.spec.in file.

jkonecny12 · 2020-10-23T09:16:49Z

/packit build

jkonecny12 · 2020-10-23T09:18:19Z

Failure of the tests is expected here:

E1101(no-member):/anaconda/pyanaconda/localization.py:77,11: get_common_locales: Module 'langtable' has no 'list_common_locales' member

sdp5 · 2020-11-02T13:27:22Z

/packit build

packit-as-a-service · 2020-11-02T13:27:26Z

Only users with write or admin permissions to the repository can trigger Packit-as-a-Service

VladimirSlavik · 2020-11-02T13:29:21Z

/packit build

Sorry about that.

jkonecny12 · 2020-11-03T12:55:52Z

/packit build

jkonecny12 · 2020-11-03T12:56:42Z

Seems that the langtable is still not in the Fedora yet.

M4rtinK · 2020-11-13T14:09:04Z

/packit build

M4rtinK

Looks good to me, thanks a lot! :)

M4rtinK · 2020-11-13T14:46:50Z

Sorry for taking me so long to review this. :P
The code looks fine & the updated langtable seems to be in place as well. :)

The RPM test is failing, but this might be fixed by rebasing the PR on latest master. Cold you please do that ? :)

I also did some manual testing and everything seems to work fine:

See the 7 languages under the selected language and Afrikaans - these are the major languages this PR is about. :)

In comparison this is how it looks like at the moment without the PR being applied:

As you can there is just an alphabetical list of all languages Anaconda knowns under the currently selected language.

I've also checked with geolocation enabled & all seems to be fine as well:

One thing I have noticed is, that the separators we use in the language listbox are really hard to see. I don't think it used to be like this & it's possibly due to some theming changes. In any case, this is not really something this PR needs to fix & I'll make a note for myself to look into fixing all the Anaconda separators to be more visible.

M4rtinK · 2020-11-13T14:48:11Z

Also one more thing - the Packit test failure seems to be related to ongoing packit test infra issues & not an issue with this PR.

sdp5 · 2020-11-13T15:19:40Z

sorry - not sure how the review got dismissed ..

poncovka · 2020-11-16T09:50:42Z

/packit build

jkonecny12 · 2020-11-16T18:17:09Z

sorry - not sure how the review got dismissed ..

This is expected it's change of behavior in the project. Reason is push to the PR.

jkonecny12

Looks good to me too

M4rtinK · 2020-11-25T19:08:33Z

I did a new Anaconda build yesterday, so this change should be now hitting the Fedora Rawhide nightly composes as part of Anaconda 34.14-1. :)

sdp5 · 2020-11-26T02:08:41Z

I did a new Anaconda build yesterday, so this change should be now hitting the Fedora Rawhide nightly composes as part of Anaconda 34.14-1. :)

thank you!

VladimirSlavik added the master Please, use the `f39` label instead. label Oct 20, 2020

jkonecny12 assigned VladimirSlavik Oct 21, 2020

VladimirSlavik assigned M4rtinK and unassigned VladimirSlavik Oct 21, 2020

sdp5 marked this pull request as draft October 21, 2020 13:43

Zokormazo reviewed Oct 22, 2020

View reviewed changes

poncovka reviewed Oct 22, 2020

View reviewed changes

sdp5 marked this pull request as ready for review October 29, 2020 11:14

M4rtinK previously approved these changes Nov 13, 2020

View reviewed changes

rhbz#1888697 list important major languages

ff200b5

sdp5 dismissed M4rtinK’s stale review via ff200b5 November 13, 2020 15:18

jkonecny12 approved these changes Nov 16, 2020

View reviewed changes

M4rtinK merged commit 80bef12 into rhinstaller:master Nov 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rhbz#1888697 list important major languages #2935

rhbz#1888697 list important major languages #2935

sdp5 commented Oct 19, 2020

pep8speaks commented Oct 19, 2020 •

edited

jkonecny12 commented Oct 21, 2020

jkonecny12 commented Oct 21, 2020

mike-fabian commented Oct 21, 2020

juhp commented Oct 21, 2020

VladimirSlavik commented Oct 21, 2020

sdp5 commented Oct 22, 2020

Zokormazo Oct 22, 2020 •

edited

sdp5 Oct 22, 2020

poncovka left a comment

poncovka Oct 22, 2020

jkonecny12 commented Oct 23, 2020

jkonecny12 commented Oct 23, 2020

sdp5 commented Nov 2, 2020

packit-as-a-service bot commented Nov 2, 2020

VladimirSlavik commented Nov 2, 2020

jkonecny12 commented Nov 3, 2020

jkonecny12 commented Nov 3, 2020

M4rtinK commented Nov 13, 2020

M4rtinK left a comment

M4rtinK commented Nov 13, 2020

M4rtinK commented Nov 13, 2020

sdp5 commented Nov 13, 2020

poncovka commented Nov 16, 2020

jkonecny12 commented Nov 16, 2020 •

edited

jkonecny12 left a comment

M4rtinK commented Nov 25, 2020

sdp5 commented Nov 26, 2020 via email

rhbz#1888697 list important major languages #2935

rhbz#1888697 list important major languages #2935

Conversation

sdp5 commented Oct 19, 2020

Implementation

pep8speaks commented Oct 19, 2020 • edited

Comment last updated at 2020-11-13 15:18:13 UTC

jkonecny12 commented Oct 21, 2020

jkonecny12 commented Oct 21, 2020

mike-fabian commented Oct 21, 2020

juhp commented Oct 21, 2020

VladimirSlavik commented Oct 21, 2020

sdp5 commented Oct 22, 2020

Zokormazo Oct 22, 2020 • edited

Choose a reason for hiding this comment

sdp5 Oct 22, 2020

Choose a reason for hiding this comment

poncovka left a comment

Choose a reason for hiding this comment

poncovka Oct 22, 2020

Choose a reason for hiding this comment

jkonecny12 commented Oct 23, 2020

jkonecny12 commented Oct 23, 2020

sdp5 commented Nov 2, 2020

packit-as-a-service bot commented Nov 2, 2020

VladimirSlavik commented Nov 2, 2020

jkonecny12 commented Nov 3, 2020

jkonecny12 commented Nov 3, 2020

M4rtinK commented Nov 13, 2020

M4rtinK left a comment

Choose a reason for hiding this comment

M4rtinK commented Nov 13, 2020

M4rtinK commented Nov 13, 2020

sdp5 commented Nov 13, 2020

poncovka commented Nov 16, 2020

jkonecny12 commented Nov 16, 2020 • edited

jkonecny12 left a comment

Choose a reason for hiding this comment

M4rtinK commented Nov 25, 2020

sdp5 commented Nov 26, 2020 via email

pep8speaks commented Oct 19, 2020 •

edited

Zokormazo Oct 22, 2020 •

edited

jkonecny12 commented Nov 16, 2020 •

edited