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

Settings GUI: Use language names rather than codes #13752

Merged
merged 2 commits into from Aug 23, 2023

Conversation

rubenwardy
Copy link
Member

@rubenwardy rubenwardy commented Aug 22, 2023

Part of #13476

Showing language codes isn't very user friendly. This PR makes it so the language dropdown shows the local name for the language.

This is also a prerequisite to making other drop downs translatable (for that, I'll need to update the settingtypes parser to create these labels and update translation templates. For a future PR).

image

image

To do

This PR is Ready for Review

How to test

  • Open settings GUI > user interfaces
  • See dropdown

@rubenwardy rubenwardy changed the title Settings GUI: Use language names rather than labels Settings GUI: Use language names rather than codes Aug 22, 2023
@grorp
Copy link
Member

grorp commented Aug 23, 2023

Slightly related: To help with #9141, we could add language as the first setting in the "Accessibility" category (as this category is the first thing you see when you open the settings).

diff --git a/builtin/mainmenu/settings/dlg_settings.lua b/builtin/mainmenu/settings/dlg_settings.lua
index b8d99a469..6049729a4 100644
--- a/builtin/mainmenu/settings/dlg_settings.lua
+++ b/builtin/mainmenu/settings/dlg_settings.lua
@@ -68,6 +68,8 @@ add_page({
        id = "accessibility",
        title = gettext("Accessibility"),
        content = {
+               "language",
+               { heading = gettext("General") },
                "font_size",
                "chat_font_size",
                "gui_scaling",

It even makes sense: One could argue that localization is an accessibility feature.

Screenshot screenshot

Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(See comments on lines for the requests)

PS grorp's suggestion is good, too.

builtin/mainmenu/settings/dlg_settings.lua Show resolved Hide resolved
builtin/mainmenu/settings/dlg_settings.lua Outdated Show resolved Hide resolved
builtin/mainmenu/settings/dlg_settings.lua Outdated Show resolved Hide resolved
builtin/mainmenu/settings/dlg_settings.lua Outdated Show resolved Hide resolved
builtin/mainmenu/settings/dlg_settings.lua Outdated Show resolved Hide resolved
builtin/mainmenu/settings/dlg_settings.lua Outdated Show resolved Hide resolved
Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works, code looks good.

Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks good to me. (If any) naming errors can be addressed later.

PS I really want to invite the Minetest translators for checking these local names, but that is highly unlikely and will just cause delays.

@rubenwardy rubenwardy merged commit a65cdbe into minetest:master Aug 23, 2023
2 checks passed
@rubenwardy rubenwardy deleted the settings_language_labels branch August 23, 2023 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants