From 7294bec1c8b86f1a6afa7c91412eb63d29279f8d Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 13:43:08 +0000 Subject: [PATCH 1/2] Use new theme API in react-sdk A thesis presented in two parts. This part has the absolute minimum logic changes to the themeing code in vector/index.js because I know how subtle and fragile this code is. However, it also looks like it's completely duplicated from react-sdk, so in the next part I'm going to remove that logic and make it use the logic in react-sdk, then we can see what breaks. Requires https://github.com/matrix-org/matrix-react-sdk/pull/3637 --- src/vector/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vector/index.js b/src/vector/index.js index 507863b1a57..402587bc7a5 100644 --- a/src/vector/index.js +++ b/src/vector/index.js @@ -57,7 +57,7 @@ import WebPlatform from './platform/WebPlatform'; import MatrixClientPeg from 'matrix-react-sdk/lib/MatrixClientPeg'; import SettingsStore from "matrix-react-sdk/lib/settings/SettingsStore"; import SdkConfig from "matrix-react-sdk/lib/SdkConfig"; -import {getBaseTheme, setTheme} from "matrix-react-sdk/lib/theme"; +import {getBaseTheme, setTheme, ThemeWatcher} from "matrix-react-sdk/lib/theme"; import Olm from 'olm'; @@ -258,7 +258,8 @@ async function loadApp() { // we do this by checking to see if the theme's "base" has loaded first so we can // safely rely on the assets. let a; - const theme = SettingsStore.getValue("theme"); + const themeWatcher = new ThemeWatcher(); + const theme = themeWatcher.getEffectiveTheme(); const baseTheme = getBaseTheme(theme); for (let i = 0; (a = document.getElementsByTagName("link")[i]); i++) { const href = a.getAttribute("href"); From e34c46c89306e30a953acce8487b0a8f5fb83c64 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 15:42:15 +0000 Subject: [PATCH 2/2] Dedup theming code Following on from https://github.com/matrix-org/matrix-react-sdk/pull/3637 this removes the code dealing with themes in vector/index.js and uses the code from react-sdk. The two did almost exactly the same thing but in subtley different ways. This code can be incredibly subtle though, so doing this a separate PR. --- src/vector/index.js | 51 ++------------------------------------------- 1 file changed, 2 insertions(+), 49 deletions(-) diff --git a/src/vector/index.js b/src/vector/index.js index 402587bc7a5..2a88fed54f1 100644 --- a/src/vector/index.js +++ b/src/vector/index.js @@ -57,7 +57,7 @@ import WebPlatform from './platform/WebPlatform'; import MatrixClientPeg from 'matrix-react-sdk/lib/MatrixClientPeg'; import SettingsStore from "matrix-react-sdk/lib/settings/SettingsStore"; import SdkConfig from "matrix-react-sdk/lib/SdkConfig"; -import {getBaseTheme, setTheme, ThemeWatcher} from "matrix-react-sdk/lib/theme"; +import {setTheme} from "matrix-react-sdk/lib/theme"; import Olm from 'olm'; @@ -255,54 +255,7 @@ async function loadApp() { } // as quickly as we possibly can, set a default theme... - // we do this by checking to see if the theme's "base" has loaded first so we can - // safely rely on the assets. - let a; - const themeWatcher = new ThemeWatcher(); - const theme = themeWatcher.getEffectiveTheme(); - const baseTheme = getBaseTheme(theme); - for (let i = 0; (a = document.getElementsByTagName("link")[i]); i++) { - const href = a.getAttribute("href"); - if (!href) continue; - // shouldn't we be using the 'title' tag rather than the href? - const match = href.match(/^bundles\/.*\/theme-(.*)\.css$/); - if (match) { - if (match[1] === baseTheme) { - // remove the disabled flag off the stylesheet - - // Firefox requires setting the attribute to false, so do - // that instead of removing it. Related: - // https://bugzilla.mozilla.org/show_bug.cgi?id=1281135 - a.disabled = false; - - // in case the Tinter.tint() in MatrixChat fires before the - // CSS has actually loaded (which in practice happens)... - - // This if fixes setTheme to not fire on Firefox - // in case it is the first time loading Riot. - // `InstallTrigger` is a Object which only exists on Firefox - // (it is used for their Plugins) and can be used as a - // feature check. - // Firefox loads css always before js. This is why we dont use - // onload or it's EventListener as those will never trigger. - if (typeof InstallTrigger !== 'undefined') { - setTheme(theme); - } else { - // FIXME: we should probably block loading the app or even - // showing a spinner until the theme is loaded, to avoid - // flashes of unstyled content. - a.onload = () => { - setTheme(theme); - }; - } - } else { - // Firefox requires this to not be done via `setAttribute` - // or via HTML. - // https://bugzilla.mozilla.org/show_bug.cgi?id=1281135 - a.disabled = true; - } - } - } + await setTheme(); // Now that we've loaded the theme (CSS), display the config syntax error if needed. if (configSyntaxError) {