Skip to content

Conversation

@darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Dec 3, 2025

Addresses this point of #384

We should hide the icon on pages that don't have localization (to avoid disappointment for users)

Will limit visibility to /keyboards/ pages

Test-bot: skip

@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Dec 3, 2025

User Test Results

Test specification and instructions

User tests are not required

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

I'd prefer to bake this into the infrastructure. My proposal:

Add this to Locale class:

    /**
     * Defines a global variable for page locale strings and also
     * tells locale system that current page uses locales
     */
    public static function definePageLocale($define, $id) {
      global $page_is_using_locale;
      $page_is_using_locale = true;
      define($define, $id);
    }

Use this test in render_globe_dropdown:

        global $page_is_using_locale;
        if(!isset($page_is_using_locale) || !$page_is_using_locale) {
          // only render on pages that use localized strings
          return;
        }

And in each page, remove the define('LOCALE... line and replace it with the equivalent of:

  Locale::definePageLocale('LOCALE_KEYBOARDS', 'keyboards');

@darcywong00
Copy link
Contributor Author

I'd prefer to bake this into the infrastructure. My proposal:

Got it. I'll update #623 separately (install and share pages) once this gets merged

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

LGTM 😉

@darcywong00 darcywong00 merged commit ca3ce8e into master Dec 4, 2025
5 checks passed
@darcywong00 darcywong00 deleted the feat/localize/globe-on-keyboard branch December 4, 2025 06:22
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants