From 5a700b518a5063a1484ee339daf2a4deb611d485 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 13:41:06 +0000 Subject: [PATCH 1/5] Get theme automatically from system setting Uses CSS `prefers-color-scheme` to get the user's preferred colour scheme. Also bundles up some theme logic into its own class. --- src/components/structures/MatrixChat.js | 17 ++--- .../tabs/user/GeneralUserSettingsTab.js | 28 ++++++-- src/i18n/strings/en_EN.json | 1 + src/settings/Settings.js | 5 ++ src/theme.js | 67 ++++++++++++++++++- 5 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index c6efb56a9d..661a0c7077 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -59,7 +59,7 @@ import { ValidatedServerConfig } from "../../utils/AutoDiscoveryUtils"; import AutoDiscoveryUtils from "../../utils/AutoDiscoveryUtils"; import DMRoomMap from '../../utils/DMRoomMap'; import { countRoomsWithNotif } from '../../RoomNotifs'; -import { setTheme } from "../../theme"; +import { ThemeWatcher } from "../../theme"; import { storeRoomAliasInCache } from '../../RoomAliasCache'; import { defer } from "../../utils/promise"; @@ -274,7 +274,8 @@ export default createReactClass({ componentDidMount: function() { this.dispatcherRef = dis.register(this.onAction); - this._themeWatchRef = SettingsStore.watchSetting("theme", null, this._onThemeChanged); + this._themeWatcher = new ThemeWatcher(); + this._themeWatcher.start(); this.focusComposer = false; @@ -361,7 +362,7 @@ export default createReactClass({ componentWillUnmount: function() { Lifecycle.stopMatrixClient(); dis.unregister(this.dispatcherRef); - SettingsStore.unwatchSetting(this._themeWatchRef); + this._themeWatcher.stop(); window.removeEventListener("focus", this.onFocus); window.removeEventListener('resize', this.handleResize); this.state.resizeNotifier.removeListener("middlePanelResized", this._dispatchTimelineResize); @@ -384,13 +385,6 @@ export default createReactClass({ } }, - _onThemeChanged: function(settingName, roomId, atLevel, newValue) { - dis.dispatch({ - action: 'set_theme', - value: newValue, - }); - }, - startPageChangeTimer() { // Tor doesn't support performance if (!performance || !performance.mark) return null; @@ -672,9 +666,6 @@ export default createReactClass({ }); break; } - case 'set_theme': - setTheme(payload.value); - break; case 'on_logging_in': // We are now logging in, so set the state to reflect that // NB. This does not touch 'ready' since if our dispatches diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index d400e7a839..50f37cea1f 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -27,7 +27,7 @@ import LanguageDropdown from "../../../elements/LanguageDropdown"; import AccessibleButton from "../../../elements/AccessibleButton"; import DeactivateAccountDialog from "../../../dialogs/DeactivateAccountDialog"; import PropTypes from "prop-types"; -import {enumerateThemes} from "../../../../../theme"; +import {enumerateThemes, ThemeWatcher} from "../../../../../theme"; import PlatformPeg from "../../../../../PlatformPeg"; import MatrixClientPeg from "../../../../../MatrixClientPeg"; import sdk from "../../../../.."; @@ -50,6 +50,7 @@ export default class GeneralUserSettingsTab extends React.Component { this.state = { language: languageHandler.getCurrentLanguage(), theme: SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme"), + useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"), haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl()), serverSupportsSeparateAddAndBind: null, idServerHasUnsignedTerms: false, @@ -177,16 +178,22 @@ export default class GeneralUserSettingsTab extends React.Component { // so remember what the value was before we tried to set it so we can revert const oldTheme = SettingsStore.getValue('theme'); SettingsStore.setValue("theme", null, SettingLevel.ACCOUNT, newTheme).catch(() => { - dis.dispatch({action: 'set_theme', value: oldTheme}); + dis.dispatch({action: 'recheck_theme'}); this.setState({theme: oldTheme}); }); this.setState({theme: newTheme}); // The settings watcher doesn't fire until the echo comes back from the // server, so to make the theme change immediately we need to manually // do the dispatch now - dis.dispatch({action: 'set_theme', value: newTheme}); + dis.dispatch({action: 'recheck_theme'}); }; + _onUseSystemThemeChanged = (checked) => { + this.setState({useSystemTheme: checked}); + dis.dispatch({action: 'recheck_theme'}); + } + + _onPasswordChangeError = (err) => { // TODO: Figure out a design that doesn't involve replacing the current dialog let errMsg = err.error || ""; @@ -297,11 +304,24 @@ export default class GeneralUserSettingsTab extends React.Component { _renderThemeSection() { const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag"); + + const themeWatcher = new ThemeWatcher(); + let systemThemeSection; + if (themeWatcher.isSystemThemeSupported()) { + systemThemeSection =
+ +
; + } return (
{_t("Theme")} + {systemThemeSection} + value={this.state.theme} onChange={this._onThemeChange} + disabled={this.state.useSystemTheme} + > {Object.entries(enumerateThemes()).map(([theme, text]) => { return ; })} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 473efdfb76..5dfcd038ec 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -363,6 +363,7 @@ "Automatically replace plain text Emoji": "Automatically replace plain text Emoji", "Mirror local video feed": "Mirror local video feed", "Enable Community Filter Panel": "Enable Community Filter Panel", + "Match system theme": "Match system theme", "Allow Peer-to-Peer for 1:1 calls": "Allow Peer-to-Peer for 1:1 calls", "Send analytics data": "Send analytics data", "Never send encrypted messages to unverified devices from this device": "Never send encrypted messages to unverified devices from this device", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 718a0daec3..8a3bc3ecbc 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -275,6 +275,11 @@ export const SETTINGS = { supportedLevels: LEVELS_ACCOUNT_SETTINGS, default: [], }, + "use_system_theme": { + supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, + default: true, + displayName: _td("Match system theme"), + }, "webRtcAllowPeerToPeer": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, displayName: _td('Allow Peer-to-Peer for 1:1 calls'), diff --git a/src/theme.js b/src/theme.js index 8a15c606d7..8996fe28fd 100644 --- a/src/theme.js +++ b/src/theme.js @@ -19,8 +19,72 @@ import {_t} from "./languageHandler"; export const DEFAULT_THEME = "light"; import Tinter from "./Tinter"; +import dis from "./dispatcher"; import SettingsStore from "./settings/SettingsStore"; +export class ThemeWatcher { + static _instance = null; + + constructor() { + this._themeWatchRef = null; + this._systemThemeWatchRef = null; + this._dispatcherRef = null; + + // we have both here as each may either match or not match, so by having both + // we can get the tristate of dark/light/unsupported + this._preferDark = global.matchMedia("(prefers-color-scheme: dark)"); + this._preferLight = global.matchMedia("(prefers-color-scheme: light)"); + + this._currentTheme = this.getEffectiveTheme(); + } + + start() { + this._themeWatchRef = SettingsStore.watchSetting("theme", null, this._onChange); + this._systemThemeWatchRef = SettingsStore.watchSetting("use_system_theme", null, this._onChange); + this._preferDark.addEventListener('change', this._onChange); + this._preferLight.addEventListener('change', this._onChange); + this._dispatcherRef = dis.register(this._onAction); + } + + stop() { + this._preferDark.removeEventListener('change', this._onChange); + this._preferLight.removeEventListener('change', this._onChange); + SettingsStore.unwatchSetting(this._systemThemeWatchRef); + SettingsStore.unwatchSetting(this._themeWatchRef); + dis.unregister(this._dispatcherRef); + } + + _onChange = () => { + this.recheck(); + } + + _onAction = (payload) => { + if (payload.action === 'recheck_theme') { + this.recheck(); + } + } + + recheck() { + const oldTheme = this._currentTheme; + this._currentTheme = this.getEffectiveTheme(); + if (oldTheme !== this._currentTheme) { + setTheme(this._currentTheme); + } + } + + getEffectiveTheme() { + if (SettingsStore.getValue('use_system_theme')) { + if (this._preferDark.matches) return 'dark'; + if (this._preferLight.matches) return 'light'; + } + return SettingsStore.getValue('theme'); + } + + isSystemThemeSupported() { + return this._preferDark || this._preferLight; + } +} + export function enumerateThemes() { const BUILTIN_THEMES = { "light": _t("Light theme"), @@ -83,7 +147,8 @@ export function getBaseTheme(theme) { */ export function setTheme(theme) { if (!theme) { - theme = SettingsStore.getValue("theme"); + const themeWatcher = new ThemeWatcher(); + theme = themeWatcher.getEffectiveTheme(); } let stylesheetName = theme; if (theme.startsWith("custom-")) { From 71f5c8b2b045a85beb563be5ef5483a6c0137723 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 13:47:54 +0000 Subject: [PATCH 2/5] Lint --- .../views/settings/tabs/user/GeneralUserSettingsTab.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index 50f37cea1f..dbe0a9a301 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -188,7 +188,7 @@ export default class GeneralUserSettingsTab extends React.Component { dis.dispatch({action: 'recheck_theme'}); }; - _onUseSystemThemeChanged = (checked) => { + _onUseSystemThemeChanged = (checked) => { this.setState({useSystemTheme: checked}); dis.dispatch({action: 'recheck_theme'}); } From a7444152213fc18e070e9e3ffca92f349c51fb47 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 15:34:32 +0000 Subject: [PATCH 3/5] Add hack to work around mystery settings bug --- .../views/settings/tabs/user/GeneralUserSettingsTab.js | 5 ++++- src/theme.js | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index dbe0a9a301..b518f7c81b 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -185,7 +185,10 @@ export default class GeneralUserSettingsTab extends React.Component { // The settings watcher doesn't fire until the echo comes back from the // server, so to make the theme change immediately we need to manually // do the dispatch now - dis.dispatch({action: 'recheck_theme'}); + // XXX: The local echoed value appears to be unreliable, in particular + // when settings custom themes(!) so adding forceTheme to override + // the value from settings. + dis.dispatch({action: 'recheck_theme', forceTheme: newTheme}); }; _onUseSystemThemeChanged = (checked) => { diff --git a/src/theme.js b/src/theme.js index 8996fe28fd..5e390bf2c8 100644 --- a/src/theme.js +++ b/src/theme.js @@ -60,13 +60,15 @@ export class ThemeWatcher { _onAction = (payload) => { if (payload.action === 'recheck_theme') { - this.recheck(); + // XXX forceTheme + this.recheck(payload.forceTheme); } } - recheck() { + // XXX: forceTheme param aded here as local echo appears to be unreliable + recheck(forceTheme) { const oldTheme = this._currentTheme; - this._currentTheme = this.getEffectiveTheme(); + this._currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme; if (oldTheme !== this._currentTheme) { setTheme(this._currentTheme); } From 518130c912dfd8cf196be96698fc4d342b79841e Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 15:37:48 +0000 Subject: [PATCH 4/5] add bug link --- src/theme.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/theme.js b/src/theme.js index 5e390bf2c8..43bc813d34 100644 --- a/src/theme.js +++ b/src/theme.js @@ -66,6 +66,7 @@ export class ThemeWatcher { } // XXX: forceTheme param aded here as local echo appears to be unreliable + // https://github.com/vector-im/riot-web/issues/11443 recheck(forceTheme) { const oldTheme = this._currentTheme; this._currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme; From e36f4375b0bdece26b729679c61e530ca17a26bf Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 16:12:14 +0000 Subject: [PATCH 5/5] Bugfix & clearer setting name --- src/i18n/strings/en_EN.json | 2 +- src/settings/Settings.js | 2 +- src/theme.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5dfcd038ec..c62b39cda0 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -363,7 +363,7 @@ "Automatically replace plain text Emoji": "Automatically replace plain text Emoji", "Mirror local video feed": "Mirror local video feed", "Enable Community Filter Panel": "Enable Community Filter Panel", - "Match system theme": "Match system theme", + "Match system dark mode setting": "Match system dark mode setting", "Allow Peer-to-Peer for 1:1 calls": "Allow Peer-to-Peer for 1:1 calls", "Send analytics data": "Send analytics data", "Never send encrypted messages to unverified devices from this device": "Never send encrypted messages to unverified devices from this device", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 8a3bc3ecbc..59e60353b8 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -278,7 +278,7 @@ export const SETTINGS = { "use_system_theme": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, default: true, - displayName: _td("Match system theme"), + displayName: _td("Match system dark mode setting"), }, "webRtcAllowPeerToPeer": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, diff --git a/src/theme.js b/src/theme.js index 43bc813d34..fa7e3f783b 100644 --- a/src/theme.js +++ b/src/theme.js @@ -84,7 +84,7 @@ export class ThemeWatcher { } isSystemThemeSupported() { - return this._preferDark || this._preferLight; + return this._preferDark.matches || this._preferLight.matches; } }