Add the custom themes to the themes page of the onboarding flow #6785
Conversation
| export class SetupThemePage implements WizardPageN<null> { | ||
| export class SetupThemePage implements WizardPageN<SetupThemePageAttrs> { | ||
| // The whitelabel themes formatted as `RadioSelectorOption`s. | ||
| private customThemes: Array<RadioSelectorOption<string>> = [] |
There was a problem hiding this comment.
you use two variables to represent the state: one for custom themes and one to indicate whether the themes are loaded. I am a big proponent of "make invalid states unrepresentable" so I would suggest pulling it into a single variable. It can be as simple as nullable array or it can be more explicit as {state: "loading"} | {state: "loaded", themes: readonly string[] } (I am not sure we need to prepare the options every time, to me it's part of view attributes that should be created on each render).
There was a problem hiding this comment.
This currently relies on ThemeController already having custom themes but if the timing is wrong we might not have them yet. I think it's okay to take this risk though, it's not the first page.
There was a problem hiding this comment.
you use two variables to represent the state: one for custom themes and one to indicate whether the themes are loaded. I am a big proponent of "make invalid states unrepresentable" so I would suggest pulling it into a single variable. It can be as simple as nullable array or it can be more explicit as
{state: "loading"} | {state: "loaded", themes: readonly string[] }(I am not sure we need to prepare the options every time, to me it's part of view attributes that should be created on each render).
That is a better way of doing it for sure. I have gone the nullable array route now.
| @@ -1659,7 +1659,6 @@ export type TranslationKeyType = | |||
| | "whitelabelDomain_label" | |||
| | "whitelabelRegistrationCode_label" | |||
| | "whitelabelRegistrationEmailDomain_label" | |||
| | "whitelabelThemeDetected_msg" | |||
There was a problem hiding this comment.
the key will come back on the next translation update, we should mark it as deprecated in Phrase at least
rezbyte
force-pushed
the
onboarding-custom-theme-6770
branch
from
a5edbad
to
fd1e169
Compare
murilopereirame
force-pushed
the
onboarding-custom-theme-6770
branch
from
fd1e169
to
b420ae4
Compare
This appends the whitelabel themes to the bottom of the radio buttons on the themes page of the onboarding flow. When there is too many themes to display, it vertically expands the dialog. Which I did not program in but I can change it to scroll if desired.
The whitelabel popup and it's translation keys have also been removed as per acceptance criteria.
Closes #6770.