Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

[WSL] Allows conditionally skipping the SelectLanguagePage #1270

Merged
merged 6 commits into from Jan 5, 2023

Conversation

CarlosNihelton
Copy link
Contributor

@CarlosNihelton CarlosNihelton commented Dec 14, 2022

Setting up WSL results in an Ubuntu environment with locale set to C.UTF-8 by default. So, if a GET request returns anything different from that value it means the server acknowledged some configuration data for the language, such as when the prefill information file is passed via command line. In such scenarios Subiquity configures the locale controller, so clients can skip the language selection page.

This PR leverages that to skip the SelectLanguagePage conditionally. Two consequences unfold:

  1. We can no longer assume the selectLanguage to be the default initial route and
  2. We no longer need to request the server for the language in the SelectLanguagePage because that's only shown if the server is set to C.UTF-8, for which the parseLocale sets the UI in English.

Those consequences are reflected in the changes herein submitted to the test code.

Skipping that page on the Windows entry point, where it has to pass through the InstallationSlidesPage was a bit more evolving, because the decision of whether the SelectLanguagePage could be skipped or not could only be taken close to the moment when the slides page kicks itself out of the stage. InstallationSlidesModel now requires knowing about the AppModel. Thus the AppModel is now passed via a ValueListenableProvider, which carefully exposes the current value, without exposing the listenable object itself. That combined with the ChangeNotifierProxyProvider allows the view model to be in sync with the AppModel without being able to trigger the listenable object by accident.

Asks the server whether the language is C.UTF-8 (the Ubuntu rootfs
default) or not.
@CarlosNihelton CarlosNihelton marked this pull request as ready for review December 14, 2022 17:24
Sets the appModel.value after the last future.
Ensures atomic delivery.
It now depends on whether Subiquity has any pre configured language.
To be 100% sure we need to set the initial route via CLI.
The case for integration test.
This screen is shown due the language not being configured in the
server.
Thus no reason for a GET. It will return C.UTF-8.
Downwards in the widget tree via Provider.
The value passed cannot alter the original ValueListenable.
Server can only be considered up if we have a server variant.
It would be waste to make a client.variant() call in here.
The AppModel has the answer.
Through a ChangeNotifierProxyProvider.
Updates in the value published from main triggers the view model.
@jpnurmi
Copy link
Contributor

jpnurmi commented Jan 3, 2023

Hmm, we should expose the auto-install HTTP headers for our SubiquityClient responses. Those headers would tell whether endpoints are configured.

@CarlosNihelton
Copy link
Contributor Author

Hmm, we should expose the auto-install HTTP headers for our SubiquityClient responses. Those headers would tell whether endpoints are configured.

I believe this would simplify a lot this use case and many others for the desktop installer in the future.

@jpnurmi
Copy link
Contributor

jpnurmi commented Jan 3, 2023

Should we wrap all return values with SubiquityResponse or something like that? Can you think of a nicer way to expose that information from subiquity_client?

@CarlosNihelton
Copy link
Contributor Author

CarlosNihelton commented Jan 3, 2023

Should we wrap all return values with SubiquityResponse or something like that? Can you think of a nicer way to expose that information from subiquity_client?

EIther SubiquityResponse<T> across the board or create new methods for the relevant GET requests that return the wrapped response instead of the value-only. For example:

Future<SubiquityResponse<String>> localeWithStatus() {
...
}

Future<String> locale() {
...
}

The alternative seems to be less intrusive and allows for a smoother transition. After all, not all GET requests will demand inspecting the x-status or x-updated headers. Yet, I'm not in love with any of the options.

Copy link
Contributor

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@CarlosNihelton CarlosNihelton merged commit f995d32 into canonical:main Jan 5, 2023
@CarlosNihelton CarlosNihelton deleted the skip-lang-wsl-349 branch January 5, 2023 16:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants