From f4cd71fd47c74d0eab2a0bf9ef58598d83c81930 Mon Sep 17 00:00:00 2001 From: Kerry Date: Thu, 24 Feb 2022 10:01:06 +0100 Subject: [PATCH] Check 'useSystemTheme' in quick settings theme switcher (#7809) * mock Element.scrollIntoView in jest setup Signed-off-by: Kerry Archibald * extract theme switcher from quick settings, add match system option, test Signed-off-by: Kerry Archibald * i18n Signed-off-by: Kerry Archibald * forgotten copyright Signed-off-by: Kerry Archibald * stylelint Signed-off-by: Kerry Archibald * remove old class Signed-off-by: Kerry Archibald --- res/css/_components.scss | 1 + .../views/spaces/_QuickThemeSwitcher.scss | 40 ++++++ res/css/structures/_QuickSettingsButton.scss | 25 ---- .../views/spaces/QuickSettingsButton.tsx | 50 +------ .../views/spaces/QuickThemeSwitcher.tsx | 94 +++++++++++++ src/i18n/strings/en_EN.json | 2 +- .../views/spaces/QuickThemeSwitcher-test.tsx | 133 ++++++++++++++++++ test/setupTests.js | 2 + 8 files changed, 274 insertions(+), 73 deletions(-) create mode 100644 res/css/components/views/spaces/_QuickThemeSwitcher.scss create mode 100644 src/components/views/spaces/QuickThemeSwitcher.tsx create mode 100644 test/components/views/spaces/QuickThemeSwitcher-test.tsx diff --git a/res/css/_components.scss b/res/css/_components.scss index aaa12ca083..583efed3b8 100644 --- a/res/css/_components.scss +++ b/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"; diff --git a/res/css/components/views/spaces/_QuickThemeSwitcher.scss b/res/css/components/views/spaces/_QuickThemeSwitcher.scss new file mode 100644 index 0000000000..c0ca83eb17 --- /dev/null +++ b/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; +} diff --git a/res/css/structures/_QuickSettingsButton.scss b/res/css/structures/_QuickSettingsButton.scss index c490b05e89..0c58e44982 100644 --- a/res/css/structures/_QuickSettingsButton.scss +++ b/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; - } - } } diff --git a/src/components/views/spaces/QuickSettingsButton.tsx b/src/components/views/spaces/QuickSettingsButton.tsx index 2ad47a8363..ecd760bd7a 100644 --- a/src/components/views/spaces/QuickSettingsButton.tsx +++ b/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 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"; +import QuickThemeSwitcher from "./QuickThemeSwitcher"; const QuickSettingsButton = ({ isPanelCollapsed = false }) => { - const orderedThemes = useMemo(getOrderedThemes, []); const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu(); 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 = { { _t("More options") } -
-

{ _t("Theme") }

- { - 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({ 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({ action: Action.RecheckTheme, forceTheme: newTheme }); - closeMenu(); - }} - value={theme} - label={_t("Space selection")} - > - { orderedThemes.map((theme) => ( -
- { theme.name } -
- )) } - -
+ ; } diff --git a/src/components/views/spaces/QuickThemeSwitcher.tsx b/src/components/views/spaces/QuickThemeSwitcher.tsx new file mode 100644 index 0000000000..5b6fbd9ac1 --- /dev/null +++ b/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 = ({ 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({ 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({ action: Action.RecheckTheme }); + } + + requestClose(); + }; + + return
+

{ _t("Theme") }

+ + { themeOptions.map((theme) => ( +
+ { theme.name } +
+ )) } + +
; +}; + +export default QuickThemeSwitcher; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5175e2f6dd..eb898c2e33 100644 --- a/src/i18n/strings/en_EN.json +++ b/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", diff --git a/test/components/views/spaces/QuickThemeSwitcher-test.tsx b/test/components/views/spaces/QuickThemeSwitcher-test.tsx new file mode 100644 index 0000000000..b0688a6f1f --- /dev/null +++ b/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('', () => { + const defaultProps = { + requestClose: jest.fn(), + }; + const getComponent = (props = {}) => + mount(); + + 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(); + }); +}); diff --git a/test/setupTests.js b/test/setupTests.js index 6297ddbd8f..6ce8ad9491 100644 --- a/test/setupTests.js +++ b/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();