From bc7b50f97c5b7e05f29aa44884413096a628ae5e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Oct 2025 14:20:53 +0100 Subject: [PATCH] Fix platform settings race condition and make auto-launch tri-state (#30977) * Fix race condition with platform settings not being read correctly Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Allow Desktop app to be auto-started minimised or focused Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * i18n Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Use onChange prop Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update res/css/views/elements/_SettingsDropdown.pcss Co-authored-by: Florian Duros --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Florian Duros --- res/css/_components.pcss | 1 + res/css/views/elements/_SettingsDropdown.pcss | 20 +++++ src/BasePlatform.ts | 1 + src/PlatformPeg.ts | 16 ++-- .../views/elements/SettingsDropdown.tsx | 81 +++++++++++++++++++ .../tabs/user/PreferencesUserSettingsTab.tsx | 18 ++++- src/dispatcher/actions.ts | 6 -- src/dispatcher/payloads/PlatformSetPayload.ts | 16 ---- src/i18n/strings/en_EN.json | 8 +- src/settings/Settings.tsx | 19 ++++- .../handlers/PlatformSettingsHandler.ts | 31 ++++--- src/vector/platform/ElectronPlatform.tsx | 6 +- .../views/elements/SettingsDropdown-test.tsx | 37 +++++++++ .../SettingsDropdown-test.tsx.snap | 48 +++++++++++ 14 files changed, 253 insertions(+), 55 deletions(-) create mode 100644 res/css/views/elements/_SettingsDropdown.pcss create mode 100644 src/components/views/elements/SettingsDropdown.tsx delete mode 100644 src/dispatcher/payloads/PlatformSetPayload.ts create mode 100644 test/unit-tests/components/views/elements/SettingsDropdown-test.tsx create mode 100644 test/unit-tests/components/views/elements/__snapshots__/SettingsDropdown-test.tsx.snap diff --git a/res/css/_components.pcss b/res/css/_components.pcss index 602885546e..bb0f769b68 100644 --- a/res/css/_components.pcss +++ b/res/css/_components.pcss @@ -209,6 +209,7 @@ @import "./views/elements/_SSOButtons.pcss"; @import "./views/elements/_SearchWarning.pcss"; @import "./views/elements/_ServerPicker.pcss"; +@import "./views/elements/_SettingsDropdown.pcss"; @import "./views/elements/_SettingsFlag.pcss"; @import "./views/elements/_Spinner.pcss"; @import "./views/elements/_StyledRadioButton.pcss"; diff --git a/res/css/views/elements/_SettingsDropdown.pcss b/res/css/views/elements/_SettingsDropdown.pcss new file mode 100644 index 0000000000..dff255a418 --- /dev/null +++ b/res/css/views/elements/_SettingsDropdown.pcss @@ -0,0 +1,20 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +.mx_SettingsDropdown { + margin-bottom: var(--cpd-space-1x); + width: 100%; + + .mx_SettingsDropdown_label { + color: $primary-content; + margin: var(--cpd-space-1x) 0; + } + + .mx_Dropdown_input { + width: 360px; + } +} diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 21641aec09..7bd830f3f2 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -70,6 +70,7 @@ export default abstract class BasePlatform { protected notificationCount = 0; protected errorDidOccur = false; protected _favicon?: Favicon; + public readonly initialised = Promise.resolve(undefined); protected constructor() { dis.register(this.onAction.bind(this)); diff --git a/src/PlatformPeg.ts b/src/PlatformPeg.ts index 9e8852170c..00ac04468b 100644 --- a/src/PlatformPeg.ts +++ b/src/PlatformPeg.ts @@ -8,9 +8,6 @@ Please see LICENSE files in the repository root for full details. */ import type BasePlatform from "./BasePlatform"; -import defaultDispatcher from "./dispatcher/dispatcher"; -import { Action } from "./dispatcher/actions"; -import { type PlatformSetPayload } from "./dispatcher/payloads/PlatformSetPayload"; /* * Holds the current instance of the `Platform` to use across the codebase. @@ -25,6 +22,14 @@ import { type PlatformSetPayload } from "./dispatcher/payloads/PlatformSetPayloa */ export class PlatformPeg { private platform: BasePlatform | null = null; + private platformPromiseWithResolvers = Promise.withResolvers(); + + /** + * Returns a promise for the Platform object for the application. + */ + public get platformPromise(): Promise { + return this.platformPromiseWithResolvers.promise; + } /** * Returns the current Platform object for the application. @@ -39,11 +44,8 @@ export class PlatformPeg { * @param {BasePlatform} platform an instance of a class extending BasePlatform. */ public set(platform: BasePlatform): void { + this.platformPromiseWithResolvers.resolve(platform); this.platform = platform; - defaultDispatcher.dispatch({ - action: Action.PlatformSet, - platform, - }); } } diff --git a/src/components/views/elements/SettingsDropdown.tsx b/src/components/views/elements/SettingsDropdown.tsx new file mode 100644 index 0000000000..859ed4ac55 --- /dev/null +++ b/src/components/views/elements/SettingsDropdown.tsx @@ -0,0 +1,81 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import React, { type JSX, useCallback, useId, useState } from "react"; + +import SettingsStore from "../../../settings/SettingsStore"; +import { type SettingLevel } from "../../../settings/SettingLevel"; +import { SETTINGS, type StringSettingKey } from "../../../settings/Settings"; +import { useSettingValueAt } from "../../../hooks/useSettings.ts"; +import Dropdown, { type DropdownProps } from "./Dropdown.tsx"; +import { _t } from "../../../shared-components/utils/i18n.tsx"; + +interface Props { + settingKey: StringSettingKey; + level: SettingLevel; + roomId?: string; // for per-room settings + label?: string; + isExplicit?: boolean; + hideIfCannotSet?: boolean; + onChange?(option: string): void; +} + +const SettingsDropdown = ({ + settingKey, + roomId, + level, + label: specificLabel, + isExplicit, + hideIfCannotSet, + onChange, +}: Props): JSX.Element => { + const id = useId(); + const settingValue = useSettingValueAt(level, settingKey, roomId ?? null, isExplicit); + const [value, setValue] = useState(settingValue); + const setting = SETTINGS[settingKey]; + + const onOptionChange = useCallback( + (value: string): void => { + setValue(value); // local echo + SettingsStore.setValue(settingKey, roomId ?? null, level, value); + onChange?.(value); + }, + [settingKey, roomId, level, onChange], + ); + + const disabled = !SettingsStore.canSetValue(settingKey, roomId ?? null, level); + if (disabled && hideIfCannotSet) return <>; + if (!setting.options) { + console.error("SettingsDropdown used for a setting with no `options`"); + return <>; + } + + const label = specificLabel ?? SettingsStore.getDisplayName(settingKey, level)!; + const options = setting.options.map((option) => { + return
{_t(option.label)}
; + }) as DropdownProps["children"]; + + return ( +
+ + + {options} + +
+ ); +}; + +export default SettingsDropdown; diff --git a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.tsx b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.tsx index b1cae7d634..d373aee60e 100644 --- a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.tsx @@ -34,6 +34,7 @@ import * as TimezoneHandler from "../../../../../TimezoneHandler"; import { type BooleanSettingKey } from "../../../../../settings/Settings.tsx"; import { MediaPreviewAccountSettings } from "./MediaPreviewAccountSettings.tsx"; import { InviteRulesAccountSetting } from "./InviteRulesAccountSettings.tsx"; +import SettingsDropdown from "../../../elements/SettingsDropdown.tsx"; interface IProps { closeSettingsFn(success: boolean): void; @@ -248,11 +249,12 @@ export default class PreferencesUserSettingsTab extends React.Component { return
{tz}
; }); + // Always prepend the default option timezones.unshift(
{browserTimezoneLabel}
); return ( @@ -264,6 +266,17 @@ export default class PreferencesUserSettingsTab extends React.Component + {SettingsStore.canSetValue("Electron.autoLaunch", null, SettingLevel.PLATFORM) && ( + + + + )} + {!newRoomListEnabled && this.renderGroup(PreferencesUserSettingsTab.ROOM_LIST_SETTINGS)} {/* The settings is on device level where the other room list settings are on account level */} @@ -356,7 +369,7 @@ export default class PreferencesUserSettingsTab extends React.Component - { * Whether the setting should be exported in a rageshake report. */ shouldExportToRageshake?: boolean; + + /** + * Options array for a setting controlled by a dropdown. + */ + options?: { + value: T; + label: TranslationKey; + }[]; } export interface IFeature extends Omit, "isFeature"> { @@ -353,7 +361,7 @@ export interface Settings { "videoInputMuted": IBaseSetting; "activeCallRoomIds": IBaseSetting; "releaseAnnouncementData": IBaseSetting; - "Electron.autoLaunch": IBaseSetting; + "Electron.autoLaunch": IBaseSetting<"enabled" | "minimised" | "disabled">; "Electron.warnBeforeExit": IBaseSetting; "Electron.alwaysShowMenuBar": IBaseSetting; "Electron.showTrayIcon": IBaseSetting; @@ -1438,8 +1446,13 @@ export const SETTINGS: Settings = { // We store them over there are they are necessary to know before the renderer process launches. "Electron.autoLaunch": { supportedLevels: [SettingLevel.PLATFORM], - displayName: _td("settings|start_automatically"), - default: false, + displayName: _td("settings|start_automatically|label"), + options: [ + { value: "enabled", label: _td("settings|start_automatically|enabled") }, + { value: "disabled", label: _td("settings|start_automatically|disabled") }, + { value: "minimised", label: _td("settings|start_automatically|minimised") }, + ], + default: "disabled", }, "Electron.warnBeforeExit": { supportedLevels: [SettingLevel.PLATFORM], diff --git a/src/settings/handlers/PlatformSettingsHandler.ts b/src/settings/handlers/PlatformSettingsHandler.ts index 48420650b1..4741f35530 100644 --- a/src/settings/handlers/PlatformSettingsHandler.ts +++ b/src/settings/handlers/PlatformSettingsHandler.ts @@ -10,9 +10,6 @@ import SettingsHandler from "./SettingsHandler"; import PlatformPeg from "../../PlatformPeg"; import { SETTINGS } from "../Settings"; import { SettingLevel } from "../SettingLevel"; -import defaultDispatcher from "../../dispatcher/dispatcher"; -import { type ActionPayload } from "../../dispatcher/payloads"; -import { Action } from "../../dispatcher/actions"; /** * Gets and sets settings at the "platform" level for the current device. @@ -24,22 +21,22 @@ export default class PlatformSettingsHandler extends SettingsHandler { public constructor() { super(); - defaultDispatcher.register(this.onAction); + void this.setup(); } - private onAction = (payload: ActionPayload): void => { - if (payload.action === Action.PlatformSet) { - this.store = {}; - // Load setting values as they are async and `getValue` must be synchronous - Object.entries(SETTINGS).forEach(([key, setting]) => { - if (setting.supportedLevels?.includes(SettingLevel.PLATFORM) && payload.platform.supportsSetting(key)) { - payload.platform.getSettingValue(key).then((value: any) => { - this.store[key] = value; - }); - } - }); - } - }; + private async setup(): Promise { + const platform = await PlatformPeg.platformPromise; + await platform.initialised; + + // Load setting values as they are async and `getValue` must be synchronous + Object.entries(SETTINGS).forEach(([key, setting]) => { + if (setting.supportedLevels?.includes(SettingLevel.PLATFORM) && platform.supportsSetting(key)) { + platform.getSettingValue(key).then((value: any) => { + this.store[key] = value; + }); + } + }); + } public canSetValue(settingName: string, roomId: string): boolean { return PlatformPeg.get()?.supportsSetting(settingName) ?? false; diff --git a/src/vector/platform/ElectronPlatform.tsx b/src/vector/platform/ElectronPlatform.tsx index 236703922c..0671801f09 100644 --- a/src/vector/platform/ElectronPlatform.tsx +++ b/src/vector/platform/ElectronPlatform.tsx @@ -374,11 +374,13 @@ export default class ElectronPlatform extends BasePlatform { return this.supportedSettings?.[settingName] === true; } - public getSettingValue(settingName: string): Promise { + public async getSettingValue(settingName: string): Promise { + await this.initialised; return this.electron.getSettingValue(settingName); } - public setSettingValue(settingName: string, value: any): Promise { + public async setSettingValue(settingName: string, value: any): Promise { + await this.initialised; return this.electron.setSettingValue(settingName, value); } diff --git a/test/unit-tests/components/views/elements/SettingsDropdown-test.tsx b/test/unit-tests/components/views/elements/SettingsDropdown-test.tsx new file mode 100644 index 0000000000..40a6126147 --- /dev/null +++ b/test/unit-tests/components/views/elements/SettingsDropdown-test.tsx @@ -0,0 +1,37 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import React from "react"; +import { render, screen } from "jest-matrix-react"; + +import { SettingLevel } from "../../../../../src/settings/SettingLevel.ts"; +import SettingsDropdown from "../../../../../src/components/views/elements/SettingsDropdown.tsx"; + +describe("", () => { + it("should render a disabled setting", async () => { + const { asFragment } = render( + , + ); + expect(asFragment()).toMatchSnapshot(); + + const trigger = screen.getByRole("button"); + expect(trigger).toHaveTextContent("No"); + expect(trigger).toHaveAttribute("aria-disabled", "true"); + }); + + it("should not render a disabled setting if hideIfCannotSet=true", async () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it("should not render a non-options setting", async () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/test/unit-tests/components/views/elements/__snapshots__/SettingsDropdown-test.tsx.snap b/test/unit-tests/components/views/elements/__snapshots__/SettingsDropdown-test.tsx.snap new file mode 100644 index 0000000000..98c6b8b63e --- /dev/null +++ b/test/unit-tests/components/views/elements/__snapshots__/SettingsDropdown-test.tsx.snap @@ -0,0 +1,48 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should render a disabled setting 1`] = ` + +
+ +
+ +
+
+ +`;