Skip to content

Comments

refactor: Use render_globe_dropdown for phone and desktop layouts 🗺️#667

Open
darcywong00 wants to merge 4 commits intomasterfrom
fix/globe-phone
Open

refactor: Use render_globe_dropdown for phone and desktop layouts 🗺️#667
darcywong00 wants to merge 4 commits intomasterfrom
fix/globe-phone

Conversation

@darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Feb 24, 2026

For #384

This refactors the globe key (keyboard search UI) so it's also available on the phone layout menu.

Screenshot of phone menu

phone-globe

Test-bot: skip

@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Feb 24, 2026

User Test Results

Test specification and instructions

User tests are not required

echo <<<END
<li><a href="{$id[0]}">{$id[1]}</a></li>
END;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we missing </ul> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! I dropped that in 77c39a0

* Render the globe dropdown for changing the UI language
* Limitation: Currently only visible on pages that use localized strings
* @param number - Div number, default 0.
* @param divClass - Div class, default 0, or 1 (for desktop); or "phone".
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should use "desktop" instead of 1 and "default" instead of 0. That would make it consistent and IMO easier to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameter was originally for keeping unique <div> id's on the page for the UI list to hover on the globe.
Desktop layout used ui-language and ui-language1

Not applicable for phone layout. I'll just rename the parameter to divID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants