Check 'useSystemTheme' in quick settings theme switcher (#7809)

* mock Element.scrollIntoView in jest setup

Signed-off-by: Kerry Archibald <kerrya@element.io>

* extract theme switcher from quick settings, add match system option, test

Signed-off-by: Kerry Archibald <kerrya@element.io>

* i18n

Signed-off-by: Kerry Archibald <kerrya@element.io>

* forgotten copyright

Signed-off-by: Kerry Archibald <kerrya@element.io>

* stylelint

Signed-off-by: Kerry Archibald <kerrya@element.io>

* remove old class

Signed-off-by: Kerry Archibald <kerrya@element.io>

res/css/_components.scss

@@ -4,6 +4,7 @@
 @import "./_font-sizes.scss";
 @import "./_font-weights.scss";
 @import "./_spacing.scss";
+@import "./components/views/spaces/_QuickThemeSwitcher.scss";
 @import "./structures/_AutoHideScrollbar.scss";
 @import "./structures/_BackdropPanel.scss";
 @import "./structures/_CompatibilityPage.scss";

res/css/components/views/spaces/_QuickThemeSwitcher.scss

@@ -0,0 +1,40 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+.mx_QuickThemeSwitcher {
+    display: flex;
+    align-items: center;
+
+    .mx_Dropdown {
+        min-width: 100px;
+        margin-left: auto;
+        height: min-content;
+    }
+
+    .mx_Dropdown_menu {
+        max-height: 70px;
+    }
+}
+
+.mx_QuickThemeSwitcher_heading {
+    font-weight: $font-semi-bold;
+    font-size: $font-12px;
+    line-height: $font-15px;
+    color: $secondary-content;
+    text-transform: uppercase;
+    display: inline-block;
+    margin: 0;
+}

res/css/structures/_QuickSettingsButton.scss

@@ -161,29 +161,4 @@ limitations under the License.
             mask-image: url('$(res)/img/element-icons/room/ellipsis.svg');
         }
     }
-
-    .mx_QuickSettingsButton_themePicker {
-        display: flex;
-        align-items: center;
-
-        > h4 {
-            font-weight: $font-semi-bold;
-            font-size: $font-12px;
-            line-height: $font-15px;
-            color: $secondary-content;
-            text-transform: uppercase;
-            display: inline-block;
-            margin: 0;
-        }
-
-        .mx_Dropdown {
-            min-width: 100px;
-            margin-left: auto;
-            height: min-content;
-        }
-
-        .mx_Dropdown_menu {
-            max-height: 70px;
-        }
-    }
 }

src/components/views/spaces/QuickSettingsButton.tsx

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { useMemo } from "react";
+import React from "react";
 import classNames from "classnames";
 
 import { _t } from "../../../languageHandler";
@@ -28,17 +28,9 @@ import { onMetaSpaceChangeFactory } from "../settings/tabs/user/SidebarUserSetti
 import defaultDispatcher from "../../../dispatcher/dispatcher";
 import { Action } from "../../../dispatcher/actions";
 import { UserTab } from "../dialogs/UserSettingsDialog";
-import { findNonHighContrastTheme, getOrderedThemes } from "../../../theme";
+import QuickThemeSwitcher from "./QuickThemeSwitcher";
-import Dropdown from "../elements/Dropdown";
-import ThemeChoicePanel from "../settings/ThemeChoicePanel";
-import SettingsStore from "../../../settings/SettingsStore";
-import { SettingLevel } from "../../../settings/SettingLevel";
-import dis from "../../../dispatcher/dispatcher";
-import { RecheckThemePayload } from "../../../dispatcher/payloads/RecheckThemePayload";
-import PosthogTrackers from "../../../PosthogTrackers";
 
 const QuickSettingsButton = ({ isPanelCollapsed = false }) => {
-    const orderedThemes = useMemo(getOrderedThemes, []);
     const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu<HTMLDivElement>();
 
     const {
@@ -48,10 +40,6 @@ const QuickSettingsButton = ({ isPanelCollapsed = false }) => {
 
     let contextMenu: JSX.Element;
     if (menuDisplayed) {
-        const themeState = ThemeChoicePanel.calculateThemeState();
-        const nonHighContrast = findNonHighContrastTheme(themeState.theme);
-        const theme = nonHighContrast ? nonHighContrast : themeState.theme;
-
         contextMenu = <ContextMenu
             {...alwaysAboveRightOf(handle.current.getBoundingClientRect(), ChevronFace.None, 16)}
             wrapperClassName="mx_QuickSettingsButton_ContextMenuWrapper"
@@ -100,39 +88,7 @@ const QuickSettingsButton = ({ isPanelCollapsed = false }) => {
                 { _t("More options") }
             </AccessibleButton>
 
-            <div className="mx_QuickSettingsButton_themePicker">
+            <QuickThemeSwitcher requestClose={closeMenu} />
-                <h4>{ _t("Theme") }</h4>
-                <Dropdown
-                    id="mx_QuickSettingsButton_themePickerDropdown"
-                    onOptionChange={async (newTheme: string) => {
-                        PosthogTrackers.trackInteraction("WebQuickSettingsThemeDropdown");
-
-                        // XXX: mostly copied from ThemeChoicePanel
-                        // 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: string = SettingsStore.getValue("theme");
-                        SettingsStore.setValue("theme", null, SettingLevel.DEVICE, newTheme).catch(() => {
-                            dis.dispatch<RecheckThemePayload>({ action: Action.RecheckTheme });
-                        });
-                        // 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
-                        // 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<RecheckThemePayload>({ action: Action.RecheckTheme, forceTheme: newTheme });
-                        closeMenu();
-                    }}
-                    value={theme}
-                    label={_t("Space selection")}
-                >
-                    { orderedThemes.map((theme) => (
-                        <div key={theme.id}>
-                            { theme.name }
-                        </div>
-                    )) }
-                </Dropdown>
-            </div>
         </ContextMenu>;
     }
 

src/components/views/spaces/QuickThemeSwitcher.tsx

@@ -0,0 +1,94 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React, { useMemo } from "react";
+
+import { _t } from "../../../languageHandler";
+import { Action } from "../../../dispatcher/actions";
+import { findNonHighContrastTheme, getOrderedThemes } from "../../../theme";
+import Dropdown from "../elements/Dropdown";
+import ThemeChoicePanel from "../settings/ThemeChoicePanel";
+import SettingsStore from "../../../settings/SettingsStore";
+import { SettingLevel } from "../../../settings/SettingLevel";
+import dis from "../../../dispatcher/dispatcher";
+import { RecheckThemePayload } from "../../../dispatcher/payloads/RecheckThemePayload";
+import PosthogTrackers from "../../../PosthogTrackers";
+
+type Props = {
+    requestClose: () => void;
+};
+
+const MATCH_SYSTEM_THEME_ID = 'MATCH_SYSTEM_THEME_ID';
+
+const QuickThemeSwitcher: React.FC<Props> = ({ requestClose }) => {
+    const orderedThemes = useMemo(getOrderedThemes, []);
+
+    const themeState = ThemeChoicePanel.calculateThemeState();
+    const nonHighContrast = findNonHighContrastTheme(themeState.theme);
+    const theme = nonHighContrast ? nonHighContrast : themeState.theme;
+    const { useSystemTheme } = themeState;
+
+    const themeOptions = [{
+        id: MATCH_SYSTEM_THEME_ID,
+        name: 'Match system',
+    }, ...orderedThemes];
+
+    const selectedTheme = useSystemTheme ? MATCH_SYSTEM_THEME_ID : theme;
+
+    const onOptionChange = async (newTheme: string) => {
+        PosthogTrackers.trackInteraction("WebQuickSettingsThemeDropdown");
+
+        try {
+            if (newTheme === MATCH_SYSTEM_THEME_ID) {
+                await SettingsStore.setValue("use_system_theme", null, SettingLevel.DEVICE, true);
+            } else {
+                // 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
+                // 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<RecheckThemePayload>({ action: Action.RecheckTheme, forceTheme: newTheme });
+                await Promise.all([
+                    SettingsStore.setValue("theme", null, SettingLevel.DEVICE, newTheme),
+                    SettingsStore.setValue("use_system_theme", null, SettingLevel.DEVICE, false),
+                ]);
+            }
+        } catch (_error) {
+            dis.dispatch<RecheckThemePayload>({ action: Action.RecheckTheme });
+        }
+
+        requestClose();
+    };
+
+    return <div className="mx_QuickThemeSwitcher">
+        <h4 className="mx_QuickThemeSwitcher_heading">{ _t("Theme") }</h4>
+        <Dropdown
+            id="mx_QuickSettingsButton_themePickerDropdown"
+            onOptionChange={onOptionChange}
+            value={selectedTheme}
+            label={_t("Space selection")}
+        >
+            { themeOptions.map((theme) => (
+                <div key={theme.id}>
+                    { theme.name }
+                </div>
+            )) }
+        </Dropdown>
+    </div>;
+};
+
+export default QuickThemeSwitcher;

src/i18n/strings/en_EN.json

@@ -1112,9 +1112,9 @@
     "All settings": "All settings",
     "Pin to sidebar": "Pin to sidebar",
     "More options": "More options",
+    "Settings": "Settings",
     "Theme": "Theme",
     "Space selection": "Space selection",
-    "Settings": "Settings",
     "Delete avatar": "Delete avatar",
     "Delete": "Delete",
     "Upload avatar": "Upload avatar",

test/components/views/spaces/QuickThemeSwitcher-test.tsx

@@ -0,0 +1,133 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from 'react';
+import { mount } from 'enzyme';
+import { mocked } from 'jest-mock';
+import { act } from 'react-dom/test-utils';
+
+import '../../../skinned-sdk';
+import QuickThemeSwitcher from '../../../../src/components/views/spaces/QuickThemeSwitcher';
+import { getOrderedThemes } from '../../../../src/theme';
+import ThemeChoicePanel from '../../../../src/components/views/settings/ThemeChoicePanel';
+import SettingsStore from '../../../../src/settings/SettingsStore';
+import { findById } from '../../../test-utils';
+import { SettingLevel } from '../../../../src/settings/SettingLevel';
+import dis from '../../../../src/dispatcher/dispatcher';
+import { Action } from '../../../../src/dispatcher/actions';
+
+jest.mock('../../../../src/theme');
+jest.mock('../../../../src/components/views/settings/ThemeChoicePanel', () => ({
+    calculateThemeState: jest.fn(),
+}));
+jest.mock('../../../../src/settings/SettingsStore', () => ({
+    setValue: jest.fn(),
+    getValue: jest.fn(),
+    monitorSetting: jest.fn(),
+}));
+
+jest.mock('../../../../src/dispatcher/dispatcher', () => ({
+    dispatch: jest.fn(),
+    register: jest.fn(),
+}));
+
+describe('<QuickThemeSwitcher />', () => {
+    const defaultProps = {
+        requestClose: jest.fn(),
+    };
+    const getComponent = (props = {}) =>
+        mount(<QuickThemeSwitcher {...defaultProps} {...props} />);
+
+    beforeEach(() => {
+        mocked(getOrderedThemes).mockClear().mockReturnValue([
+            { id: 'light', name: 'Light' },
+            { id: 'dark', name: 'Dark' },
+        ]);
+        mocked(ThemeChoicePanel).calculateThemeState.mockClear().mockReturnValue({
+            theme: 'light', useSystemTheme: false,
+        });
+        mocked(SettingsStore).setValue.mockClear().mockResolvedValue();
+        mocked(dis).dispatch.mockClear();
+    });
+
+    const getSelectedLabel = (component) =>
+        findById(component, "mx_QuickSettingsButton_themePickerDropdown_value").text();
+
+    const openDropdown = component => act(async () => {
+        component.find('.mx_Dropdown_input').at(0).simulate('click');
+        component.setProps({});
+    });
+    const getOption = (component, themeId) =>
+        findById(component, `mx_QuickSettingsButton_themePickerDropdown__${themeId}`).at(0);
+
+    const selectOption = async (component, themeId: string) => {
+        await openDropdown(component);
+        await act(async () => {
+            getOption(component, themeId).simulate('click');
+        });
+    };
+
+    it('renders dropdown correctly when light theme is selected', () => {
+        const component = getComponent();
+        expect(getSelectedLabel(component)).toEqual('Light');
+    });
+
+    it('renders dropdown correctly when use system theme is truthy', () => {
+        mocked(ThemeChoicePanel).calculateThemeState.mockClear().mockReturnValue({
+            theme: 'light', useSystemTheme: true,
+        });
+        const component = getComponent();
+        expect(getSelectedLabel(component)).toEqual('Match system');
+    });
+
+    it('updates settings when match system is selected', async () => {
+        const requestClose = jest.fn();
+        const component = getComponent({ requestClose });
+
+        await selectOption(component, 'MATCH_SYSTEM_THEME_ID');
+
+        expect(SettingsStore.setValue).toHaveBeenCalledTimes(1);
+        expect(SettingsStore.setValue).toHaveBeenCalledWith('use_system_theme', null, SettingLevel.DEVICE, true);
+
+        expect(dis.dispatch).not.toHaveBeenCalled();
+        expect(requestClose).toHaveBeenCalled();
+    });
+
+    it('updates settings when a theme is selected', async () => {
+        // ie not match system
+        const requestClose = jest.fn();
+        const component = getComponent({ requestClose });
+
+        await selectOption(component, 'dark');
+
+        expect(SettingsStore.setValue).toHaveBeenCalledWith('use_system_theme', null, SettingLevel.DEVICE, false);
+        expect(SettingsStore.setValue).toHaveBeenCalledWith('theme', null, SettingLevel.DEVICE, 'dark');
+
+        expect(dis.dispatch).toHaveBeenCalledWith({ action: Action.RecheckTheme, forceTheme: 'dark' });
+        expect(requestClose).toHaveBeenCalled();
+    });
+
+    it('rechecks theme when setting theme fails', async () => {
+        mocked(SettingsStore.setValue).mockRejectedValue('oops');
+        const requestClose = jest.fn();
+        const component = getComponent({ requestClose });
+
+        await selectOption(component, 'MATCH_SYSTEM_THEME_ID');
+
+        expect(dis.dispatch).toHaveBeenCalledWith({ action: Action.RecheckTheme });
+        expect(requestClose).toHaveBeenCalled();
+    });
+});

test/setupTests.js

@@ -48,3 +48,5 @@ const mockMatchMedia = jest.fn().mockImplementation(query => ({
     dispatchEvent: jest.fn(),
 }));
 global.matchMedia = mockMatchMedia;
+
+window.HTMLElement.prototype.scrollIntoView = jest.fn();