From 80ee68a42f468e5754cad43797e37a0a8e668811 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Tue, 19 Nov 2019 22:36:55 +0000
Subject: [PATCH 1/2] Use a settings watcher to set the theme

Rather than listening for account data updates manually
---
 src/components/structures/MatrixChat.js       | 20 +++++++++----------
 .../tabs/user/GeneralUserSettingsTab.js       |  3 +++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js
index 620e73bf93..c6efb56a9d 100644
--- a/src/components/structures/MatrixChat.js
+++ b/src/components/structures/MatrixChat.js
@@ -274,6 +274,7 @@ export default createReactClass({
 
     componentDidMount: function() {
         this.dispatcherRef = dis.register(this.onAction);
+        this._themeWatchRef = SettingsStore.watchSetting("theme", null, this._onThemeChanged);
 
         this.focusComposer = false;
 
@@ -360,6 +361,7 @@ export default createReactClass({
     componentWillUnmount: function() {
         Lifecycle.stopMatrixClient();
         dis.unregister(this.dispatcherRef);
+        SettingsStore.unwatchSetting(this._themeWatchRef);
         window.removeEventListener("focus", this.onFocus);
         window.removeEventListener('resize', this.handleResize);
         this.state.resizeNotifier.removeListener("middlePanelResized", this._dispatchTimelineResize);
@@ -382,6 +384,13 @@ 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;
@@ -1376,17 +1385,6 @@ export default createReactClass({
             }, null, true);
         });
 
-        cli.on("accountData", function(ev) {
-            if (ev.getType() === 'im.vector.web.settings') {
-                if (ev.getContent() && ev.getContent().theme) {
-                    dis.dispatch({
-                        action: 'set_theme',
-                        value: ev.getContent().theme,
-                    });
-                }
-            }
-        });
-
         const dft = new DecryptionFailureTracker((total, errorCode) => {
             Analytics.trackEvent('E2E', 'Decryption failure', errorCode, total);
         }, (errorCode) => {
diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
index 78961ad663..42324f1379 100644
--- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
+++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
@@ -175,6 +175,9 @@ export default class GeneralUserSettingsTab extends React.Component {
 
         SettingsStore.setValue("theme", null, SettingLevel.ACCOUNT, newTheme);
         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});
     };
 

From a31d222570f7159d922ca0a94161574e92578678 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Tue, 19 Nov 2019 23:00:54 +0000
Subject: [PATCH 2/2] Add catch handler for theme setting

---
 .../views/settings/tabs/user/GeneralUserSettingsTab.js    | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
index 42324f1379..d400e7a839 100644
--- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
+++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js
@@ -173,7 +173,13 @@ export default class GeneralUserSettingsTab extends React.Component {
         const newTheme = e.target.value;
         if (this.state.theme === newTheme) return;
 
-        SettingsStore.setValue("theme", null, SettingLevel.ACCOUNT, newTheme);
+        // doing getValue in the .catch will still return the value we failed to set,
+        // 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});
+            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