Ensure the settings page accurately represents theme choices

Fixes https://github.com/vector-im/riot-web/issues/11518

This also fixes a bug where the the theme logic can incorrectly get stuck in the light theme - the ThemeController was overriding *all* values, not just supposed themes. Null values aren't overridden so that the various theme logic bits don't assume the user has chosen the light theme explicitly.
This commit is contained in:
Travis Ralston 2019-11-28 13:40:28 -07:00
parent 25809c54a7
commit 37ec7e6f7e
3 changed files with 46 additions and 7 deletions

View file

@ -49,8 +49,6 @@ export default class GeneralUserSettingsTab extends React.Component {
this.state = { this.state = {
language: languageHandler.getCurrentLanguage(), language: languageHandler.getCurrentLanguage(),
theme: SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme"),
useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"),
haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl()), haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl()),
serverSupportsSeparateAddAndBind: null, serverSupportsSeparateAddAndBind: null,
idServerHasUnsignedTerms: false, idServerHasUnsignedTerms: false,
@ -62,6 +60,7 @@ export default class GeneralUserSettingsTab extends React.Component {
}, },
emails: [], emails: [],
msisdns: [], msisdns: [],
...this._calculateThemeState(),
}; };
this.dispatcherRef = dis.register(this._onAction); this.dispatcherRef = dis.register(this._onAction);
@ -80,6 +79,39 @@ export default class GeneralUserSettingsTab extends React.Component {
dis.unregister(this.dispatcherRef); dis.unregister(this.dispatcherRef);
} }
_calculateThemeState() {
// We have to mirror the logic from ThemeWatcher.getEffectiveTheme so we
// show the right values for things.
const themeChoice = SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme");
const systemThemeExplicit = SettingsStore.getValueAt(
SettingLevel.DEVICE, "use_system_theme", null, false, true);
const themeExplicit = SettingsStore.getValueAt(
SettingLevel.DEVICE, "theme", null, false, true);
// If the user has enabled system theme matching, use that.
if (systemThemeExplicit) {
return {
theme: themeChoice,
useSystemTheme: true,
};
}
// If the user has set a theme explicitly, use that (no system theme matching)
if (themeExplicit) {
return {
theme: themeChoice,
useSystemTheme: false,
};
}
// Otherwise assume the defaults for the settings
return {
theme: themeChoice,
useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"),
};
}
_onAction = (payload) => { _onAction = (payload) => {
if (payload.action === 'id_server_changed') { if (payload.action === 'id_server_changed') {
this.setState({haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl())}); this.setState({haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl())});
@ -89,11 +121,11 @@ export default class GeneralUserSettingsTab extends React.Component {
_onEmailsChange = (emails) => { _onEmailsChange = (emails) => {
this.setState({ emails }); this.setState({ emails });
} };
_onMsisdnsChange = (msisdns) => { _onMsisdnsChange = (msisdns) => {
this.setState({ msisdns }); this.setState({ msisdns });
} };
async _getThreepidState() { async _getThreepidState() {
const cli = MatrixClientPeg.get(); const cli = MatrixClientPeg.get();
@ -193,9 +225,9 @@ export default class GeneralUserSettingsTab extends React.Component {
_onUseSystemThemeChanged = (checked) => { _onUseSystemThemeChanged = (checked) => {
this.setState({useSystemTheme: checked}); this.setState({useSystemTheme: checked});
SettingsStore.setValue("use_system_theme", null, SettingLevel.DEVICE, checked);
dis.dispatch({action: 'recheck_theme'}); dis.dispatch({action: 'recheck_theme'});
} };
_onPasswordChangeError = (err) => { _onPasswordChangeError = (err) => {
// TODO: Figure out a design that doesn't involve replacing the current dialog // TODO: Figure out a design that doesn't involve replacing the current dialog
@ -307,12 +339,15 @@ export default class GeneralUserSettingsTab extends React.Component {
_renderThemeSection() { _renderThemeSection() {
const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag"); const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag");
const LabelledToggleSwitch = sdk.getComponent("views.elements.LabelledToggleSwitch");
const themeWatcher = new ThemeWatcher(); const themeWatcher = new ThemeWatcher();
let systemThemeSection; let systemThemeSection;
if (themeWatcher.isSystemThemeSupported()) { if (themeWatcher.isSystemThemeSupported()) {
systemThemeSection = <div> systemThemeSection = <div>
<SettingsFlag name="use_system_theme" level={SettingLevel.DEVICE} <LabelledToggleSwitch
value={this.state.useSystemTheme}
label={SettingsStore.getDisplayName("use_system_theme")}
onChange={this._onUseSystemThemeChanged} onChange={this._onUseSystemThemeChanged}
/> />
</div>; </div>;

View file

@ -20,6 +20,8 @@ import {DEFAULT_THEME, enumerateThemes} from "../../theme";
export default class ThemeController extends SettingController { export default class ThemeController extends SettingController {
getValueOverride(level, roomId, calculatedValue, calculatedAtLevel) { getValueOverride(level, roomId, calculatedValue, calculatedAtLevel) {
if (!calculatedValue) return null; // Don't override null themes
const themes = enumerateThemes(); const themes = enumerateThemes();
// Override in case some no longer supported theme is stored here // Override in case some no longer supported theme is stored here
if (!themes[calculatedValue]) { if (!themes[calculatedValue]) {

View file

@ -80,6 +80,8 @@ export class ThemeWatcher {
} }
getEffectiveTheme() { getEffectiveTheme() {
// Dev note: Much of this logic is replicated in the GeneralUserSettingsTab
// If the user has specifically enabled the system matching option (excluding default), // If the user has specifically enabled the system matching option (excluding default),
// then use that over anything else. We pick the lowest possible level for the setting // then use that over anything else. We pick the lowest possible level for the setting
// to ensure the ordering otherwise works. // to ensure the ordering otherwise works.