-
Notifications
You must be signed in to change notification settings - Fork 69
feat(web): improve usability of installer L10n settings #2359
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
Conversation
Place a button in the top bar near the main action for opening the current "Installer options" dialog, making the display language and keyboard layout settings more discoverable. The button shows visual hints about the action and displays the current values for both settings. Further A11y improvements are needed to ensure non-visual users can easily access this information as well.
Introduce a checkbox in the installer options dialog to quickly apply the display language and keyboard layout to the selected product. Note: Still missing a link to the Localization page.
For not mounting the button to change the language and kebyoard when it does not make sense.
The Header component now requires the `{ withL10n: true }` option to
work properly in tests. This commit avoids updating the tests themselves by
mocking the component instead.
Adds a keyboard layout hint next to password fields to reduce the likelihood of users mistyping their passwords due to an unexpected input language or layout. This is a proof of concept: currently missing tests and polish, but demonstrates the intended behavior.
By using a hook borrowed from https://pietrobondioli.com.br/articles/how-to-get-keylock-state-on-react As previous commit, this is a proof of concept without tests and a lot of room for improvements. and currently is missing tests
To avoid application crashing with
> Error: useInstallerL10n must be used within a InstallerL10nContext
> at useInstallerL10n (http://localhost:8080/index.js:57706:11)
> at PasswordInput (http://localhost:8080/index.62413e0223d80064660e.hot-update.js:322:99)
due to the usage of the useInstallerL10n hook.
Moving the keyboard reminder into an isolated internal component prevents the hook from executing
when mounting PasswordInput with the `showKeyboardHint={false}` prop.
To make use of installerRender and withL10n: true option for these components using a PasswordInput.
For using an icon closer to https://languageicon.org/
Enhances the InstallerOptions component to support selective rendering of settings via a `variant` prop. Allows displaying only language, only keyboard layout, or both. Useful in scenarios where only a subset of localization settings needs to be configured (e.g., keyboard-only setup from a question with password input)
With the latest changes, it makes more sense for them to live together in the same module and component. Having them separated was forcing developers to duplicate logic and props for almost no reason.
The installer doesn't yet support persisting localization settings for later application when a product is selected. Therefore, it's safer to show an informative message that might help clarify or even state the difference between interface and product localization, especially useful when the installer options dialog is opened before a product has been chosen.
Since it is only needed to test that it mounts InstallerOptions, not when its its toggle its rendered or not.
Enables passing a custom toggle element for opening the installer settings panel, which can be helpful for inline toggles or external buttons. Note: Each toggle renders and manages its own instance of the settings panel. While multiple panel instances can coexist in the DOM, only one can be open at a time. This is generally fine but worth keeping in mind for UX and performance. Alternative approaches that were considered: - Assigning an `id` to the default toggle and triggering it via `document.getElementById(id)?.click()`. This is not idiomatic React and could become error-prone if used frequently. - Using a `ref` on the default toggle and storing it in global context. This looks more aligned with React principles, but we still need to explore the best practices around it. Overall, this change improves flexibility without breaking existing usage.
To help users understanding that L10n page settings are for the product to install, not for the installer interface. It provides a link for directly open the installer options panel from there, but also teachs them that such an option is available from the top bar.
Currently limited to password input questions only.
Specifically in the explanation text for the checkbox that allows users to apply the selected settings to both the installer and the product to install.
The PasswordInput component now supports a reminders prop to control
which keyboard-related hints are shown, such as the current keymap or a
Caps Lock warning.
- Introduced `reminders` prop to control which keyboard-related hints are shown
("keymap", "capslock"), replacing the previous `showKeyboardHint` boolean.
- Refactored reminder rendering into dedicated components: KeymapReminder and CapsLockReminder.
- Skipped keymap hint when running over remote connections.
- Updated LoginPage and PasswordAndConfirmationInput to use new `reminders` prop.
- Expanded test coverage for all reminder combinations and edge cases.
Adding a test it was detected and fixed a wrong state change.
95a0355 to
709bb28
Compare
| expect(screen.queryByText(/^CAPS LOCK/)).toBeNull(); | ||
| }); | ||
|
|
||
| it("allows pikcing only the keymap reminder", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: picking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we have a check for spelling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we did. But was deactivated for long time and I drop it. I 've, however, an experimental approach in my development setup to check spelling on commit message... Unfortunately, looks like I ignored it here 😓 Thanks for catching it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we did. But was deactivated for long time and I drop it.
See #2158
I 've, however, an experimental approach in my development setup to check spelling on commit message
Which allows not polluting files with cspell directive comments
| expect(screen.queryByText(/is on$/)).toBeNull(); | ||
| }); | ||
|
|
||
| it("allows pikcing only the caps locsk reminder", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: picking
|
The code looks good, but I found some "weird" behaviour while playing with it. It would be easier to arrange a meeting and I will show you it. BTW, I have run the code directly in my VM because the iso image fails (missing product?): |
|
The issues we talked about are not introduced by this PR (they are present in master too). I can approve it after fixing the typos and the the changelog conflict. |
|
BTW, should I open a bug/issue with the detected problems? |
And also uses the "installer interface" in a text shown to users instead of only "interface", which could be confusing. A comment for translators has been also added.
Thanks for checking them. Much appreciated.
Working on it now
As you wish. Open a bug/issue or directly create a card in our internal Trello board. I've the feeling that latest will be faster. |
joseivanlopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Part of #2359: render current keymap set for installer only in local connections.
Prepare to release Agama 15: * #2258 * #2270 * #2277 * #2279 * #2283 * #2284 * #2285 * #2286 * #2287 * #2288 * #2291 * #2292 * #2293 * #2295 * #2297 * #2299 * #2300 * #2301 * #2302 * #2303 * #2305 * #2306 * #2307 * #2308 * #2309 * #2313 * #2314 * #2315 * #2317 * #2318 * #2319 * #2320 * #2321 * #2322 * #2323 * #2324 * #2325 * #2328 * #2329 * #2330 * #2331 * #2335 * #2336 * #2337 * #2338 * #2339 * #2340 * #2342 * #2345 * #2346 * #2348 * #2349 * #2350 * #2351 * #2352 * #2353 * #2354 * #2355 * #2357 * #2358 * #2359 * #2360 * #2361 * #2362 * #2363 * #2364 * #2365 * #2366 * #2368 * #2369 * #2370 * #2371 * #2372 * #2374 * #2377 * #2378 * #2379 * #2380 * #2381 * #2382 * #2384 * #2385 * #2386 * #2388 * #2389 * #2390 * #2391 * #2392 * #2394 * #2397 * #2398 * #2401 * #2403
https://build.opensuse.org/request/show/1280475 by user IGonzalezSosa + anag_factory - Version 15 - Handle reused MD RAIDs that are already included at the storage configuration (gh/agama-project/agama#2346). - Allow to resolve in web UI software conflicts (gh#agama-project/agama#2348) - Do not crash when navigating to a Wi-Fi network (bsc#1243415) - Keep margin between sidebar and main content in all breakpoints (gh/agama-project/agama#2370). - Rework the installer l10n settings (gh#agama-project/agama#2359): - Improve discoverability of language and keyboard layout settings. - Add contextual messages to help users differentiate between installer and product localization settings. - Add the ability to reuse installer settings for the product to install. - In local connections, keyboard layout change is now available directly from modal dialog
Problem
Agama offers two different localization (l10n) configurations:
Despite being configured in different UI places, users often confuse these settings. This leads to complaints when changing the "product" language or keyboard layout, but still seeing the installer interface remain unchanged.
Situation is particularly problematic with keyboard layouts, especially when it comes to a password input.
Solution
After considering and rejecting the idea of unifying both settings since it could introduce more complexity and be harder to explain to users, this PR improves discoverability and clarity of installer localization settings:
Testing
Notes for Reviewers
Due to the wide range of variations and UI paths, screenshots would be impractical. Manual testing is encouraged as part of the review process, for which you can use the image available at https://build.opensuse.org/project/show/systemsmanagement:Agama:branches:lang-and-keyboard-improvements
Scenarios to Test
Local Connection
LuksActivationQuestionshows a keyboard layout button, with reusability depending on whether the product was already selected.QuestionWithPassworddialogs show a layout change button.Remote Connection
LuksActivationQuestionandQuestionWithPassworddo not show layout buttons.Screenshots
Click to show/hide a bunch of them
IMPORTANT: screenshots below might be outdated as a result of changes requested by reviewers before merging