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 <florianduros@element.io>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Co-authored-by: Florian Duros <florianduros@element.io>
This commit is contained in:
Michael Telatynski
2025-10-09 14:20:53 +01:00
committed by GitHub
parent 3098eba4f2
commit bc7b50f97c
14 changed files with 253 additions and 55 deletions

View File

@@ -209,6 +209,7 @@
@import "./views/elements/_SSOButtons.pcss"; @import "./views/elements/_SSOButtons.pcss";
@import "./views/elements/_SearchWarning.pcss"; @import "./views/elements/_SearchWarning.pcss";
@import "./views/elements/_ServerPicker.pcss"; @import "./views/elements/_ServerPicker.pcss";
@import "./views/elements/_SettingsDropdown.pcss";
@import "./views/elements/_SettingsFlag.pcss"; @import "./views/elements/_SettingsFlag.pcss";
@import "./views/elements/_Spinner.pcss"; @import "./views/elements/_Spinner.pcss";
@import "./views/elements/_StyledRadioButton.pcss"; @import "./views/elements/_StyledRadioButton.pcss";

View File

@@ -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;
}
}

View File

@@ -70,6 +70,7 @@ export default abstract class BasePlatform {
protected notificationCount = 0; protected notificationCount = 0;
protected errorDidOccur = false; protected errorDidOccur = false;
protected _favicon?: Favicon; protected _favicon?: Favicon;
public readonly initialised = Promise.resolve<void>(undefined);
protected constructor() { protected constructor() {
dis.register(this.onAction.bind(this)); dis.register(this.onAction.bind(this));

View File

@@ -8,9 +8,6 @@ Please see LICENSE files in the repository root for full details.
*/ */
import type BasePlatform from "./BasePlatform"; 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. * 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 { export class PlatformPeg {
private platform: BasePlatform | null = null; private platform: BasePlatform | null = null;
private platformPromiseWithResolvers = Promise.withResolvers<BasePlatform>();
/**
* Returns a promise for the Platform object for the application.
*/
public get platformPromise(): Promise<BasePlatform> {
return this.platformPromiseWithResolvers.promise;
}
/** /**
* Returns the current Platform object for the application. * 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. * @param {BasePlatform} platform an instance of a class extending BasePlatform.
*/ */
public set(platform: BasePlatform): void { public set(platform: BasePlatform): void {
this.platformPromiseWithResolvers.resolve(platform);
this.platform = platform; this.platform = platform;
defaultDispatcher.dispatch<PlatformSetPayload>({
action: Action.PlatformSet,
platform,
});
} }
} }

View File

@@ -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 <div key={option.value}>{_t(option.label)}</div>;
}) as DropdownProps["children"];
return (
<div className="mx_SettingsDropdown">
<label className="mx_SettingsDropdown_label" htmlFor={id}>
<span className="mx_SettingsDropdown_labelText">{label}</span>
</label>
<Dropdown
id={id}
onOptionChange={onOptionChange}
menuWidth={360} // matches CSS width
value={value}
disabled={disabled}
label={label}
>
{options}
</Dropdown>
</div>
);
};
export default SettingsDropdown;

View File

@@ -34,6 +34,7 @@ import * as TimezoneHandler from "../../../../../TimezoneHandler";
import { type BooleanSettingKey } from "../../../../../settings/Settings.tsx"; import { type BooleanSettingKey } from "../../../../../settings/Settings.tsx";
import { MediaPreviewAccountSettings } from "./MediaPreviewAccountSettings.tsx"; import { MediaPreviewAccountSettings } from "./MediaPreviewAccountSettings.tsx";
import { InviteRulesAccountSetting } from "./InviteRulesAccountSettings.tsx"; import { InviteRulesAccountSetting } from "./InviteRulesAccountSettings.tsx";
import SettingsDropdown from "../../../elements/SettingsDropdown.tsx";
interface IProps { interface IProps {
closeSettingsFn(success: boolean): void; closeSettingsFn(success: boolean): void;
@@ -248,11 +249,12 @@ export default class PreferencesUserSettingsTab extends React.Component<IProps,
}); });
const newRoomListEnabled = SettingsStore.getValue("feature_new_room_list"); const newRoomListEnabled = SettingsStore.getValue("feature_new_room_list");
const brand = SdkConfig.get().brand;
// Always Preprend the default option
const timezones = this.state.timezones.map((tz) => { const timezones = this.state.timezones.map((tz) => {
return <div key={tz}>{tz}</div>; return <div key={tz}>{tz}</div>;
}); });
// Always prepend the default option
timezones.unshift(<div key="">{browserTimezoneLabel}</div>); timezones.unshift(<div key="">{browserTimezoneLabel}</div>);
return ( return (
@@ -264,6 +266,17 @@ export default class PreferencesUserSettingsTab extends React.Component<IProps,
<SpellCheckSection /> <SpellCheckSection />
</SettingsSubsection> </SettingsSubsection>
{SettingsStore.canSetValue("Electron.autoLaunch", null, SettingLevel.PLATFORM) && (
<SettingsSubsection heading={_t("settings|preferences|startup_window_behaviour_label")}>
<SettingsDropdown
settingKey="Electron.autoLaunch"
label={_t("settings|start_automatically|label", { brand })}
level={SettingLevel.PLATFORM}
hideIfCannotSet
/>
</SettingsSubsection>
)}
<SettingsSubsection heading={_t("settings|preferences|room_list_heading")}> <SettingsSubsection heading={_t("settings|preferences|room_list_heading")}>
{!newRoomListEnabled && this.renderGroup(PreferencesUserSettingsTab.ROOM_LIST_SETTINGS)} {!newRoomListEnabled && this.renderGroup(PreferencesUserSettingsTab.ROOM_LIST_SETTINGS)}
{/* The settings is on device level where the other room list settings are on account level */} {/* 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<IProps,
level={SettingLevel.PLATFORM} level={SettingLevel.PLATFORM}
hideIfCannotSet hideIfCannotSet
label={_t("settings|preferences|Electron.enableHardwareAcceleration", { label={_t("settings|preferences|Electron.enableHardwareAcceleration", {
appName: SdkConfig.get().brand, appName: brand,
})} })}
/> />
<SettingsFlag <SettingsFlag
@@ -366,7 +379,6 @@ export default class PreferencesUserSettingsTab extends React.Component<IProps,
label={_t("settings|preferences|Electron.enableContentProtection")} label={_t("settings|preferences|Electron.enableContentProtection")}
/> />
<SettingsFlag name="Electron.alwaysShowMenuBar" level={SettingLevel.PLATFORM} hideIfCannotSet /> <SettingsFlag name="Electron.alwaysShowMenuBar" level={SettingLevel.PLATFORM} hideIfCannotSet />
<SettingsFlag name="Electron.autoLaunch" level={SettingLevel.PLATFORM} hideIfCannotSet />
<SettingsFlag name="Electron.warnBeforeExit" level={SettingLevel.PLATFORM} hideIfCannotSet /> <SettingsFlag name="Electron.warnBeforeExit" level={SettingLevel.PLATFORM} hideIfCannotSet />
<Field <Field

View File

@@ -331,12 +331,6 @@ export enum Action {
*/ */
OverwriteLogin = "overwrite_login", OverwriteLogin = "overwrite_login",
/**
* Fired when the PlatformPeg gets a new platform set upon it, should only happen once per app load lifecycle.
* Fires with the PlatformSetPayload.
*/
PlatformSet = "platform_set",
/** /**
* Fired when we want to view a thread, either a new one or an existing one * Fired when we want to view a thread, either a new one or an existing one
*/ */

View File

@@ -1,16 +0,0 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2022 The Matrix.org Foundation C.I.C.
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 { type Action } from "../actions";
import { type ActionPayload } from "../payloads";
import type BasePlatform from "../../BasePlatform";
export interface PlatformSetPayload extends ActionPayload {
action: Action.PlatformSet;
platform: BasePlatform;
}

View File

@@ -2891,6 +2891,7 @@
"room_list_heading": "Room list", "room_list_heading": "Room list",
"show_avatars_pills": "Show avatars in user, room and event mentions", "show_avatars_pills": "Show avatars in user, room and event mentions",
"show_polls_button": "Show polls button", "show_polls_button": "Show polls button",
"startup_window_behaviour_label": "Start-up and window behaviour",
"surround_text": "Surround selected text when typing special characters", "surround_text": "Surround selected text when typing special characters",
"time_heading": "Displaying time", "time_heading": "Displaying time",
"user_timezone": "Set timezone" "user_timezone": "Set timezone"
@@ -3064,7 +3065,12 @@
"spaces_explainer": "Spaces are ways to group rooms and people. Alongside the spaces you're in, you can use some pre-built ones too.", "spaces_explainer": "Spaces are ways to group rooms and people. Alongside the spaces you're in, you can use some pre-built ones too.",
"title": "Sidebar" "title": "Sidebar"
}, },
"start_automatically": "Start automatically after system login", "start_automatically": {
"disabled": "No",
"enabled": "Yes",
"label": "Open %(brand)s when you log in to your computer",
"minimised": "Minimised"
},
"tac_only_notifications": "Only show notifications in the thread activity centre", "tac_only_notifications": "Only show notifications in the thread activity centre",
"use_12_hour_format": "Show timestamps in 12 hour format (e.g. 2:30pm)", "use_12_hour_format": "Show timestamps in 12 hour format (e.g. 2:30pm)",
"use_command_enter_send_message": "Use Command + Enter to send a message", "use_command_enter_send_message": "Use Command + Enter to send a message",

View File

@@ -179,6 +179,14 @@ export interface IBaseSetting<T extends SettingValueType = SettingValueType> {
* Whether the setting should be exported in a rageshake report. * Whether the setting should be exported in a rageshake report.
*/ */
shouldExportToRageshake?: boolean; shouldExportToRageshake?: boolean;
/**
* Options array for a setting controlled by a dropdown.
*/
options?: {
value: T;
label: TranslationKey;
}[];
} }
export interface IFeature extends Omit<IBaseSetting<boolean>, "isFeature"> { export interface IFeature extends Omit<IBaseSetting<boolean>, "isFeature"> {
@@ -353,7 +361,7 @@ export interface Settings {
"videoInputMuted": IBaseSetting<boolean>; "videoInputMuted": IBaseSetting<boolean>;
"activeCallRoomIds": IBaseSetting<string[]>; "activeCallRoomIds": IBaseSetting<string[]>;
"releaseAnnouncementData": IBaseSetting<ReleaseAnnouncementData>; "releaseAnnouncementData": IBaseSetting<ReleaseAnnouncementData>;
"Electron.autoLaunch": IBaseSetting<boolean>; "Electron.autoLaunch": IBaseSetting<"enabled" | "minimised" | "disabled">;
"Electron.warnBeforeExit": IBaseSetting<boolean>; "Electron.warnBeforeExit": IBaseSetting<boolean>;
"Electron.alwaysShowMenuBar": IBaseSetting<boolean>; "Electron.alwaysShowMenuBar": IBaseSetting<boolean>;
"Electron.showTrayIcon": IBaseSetting<boolean>; "Electron.showTrayIcon": IBaseSetting<boolean>;
@@ -1438,8 +1446,13 @@ export const SETTINGS: Settings = {
// We store them over there are they are necessary to know before the renderer process launches. // We store them over there are they are necessary to know before the renderer process launches.
"Electron.autoLaunch": { "Electron.autoLaunch": {
supportedLevels: [SettingLevel.PLATFORM], supportedLevels: [SettingLevel.PLATFORM],
displayName: _td("settings|start_automatically"), displayName: _td("settings|start_automatically|label"),
default: false, 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": { "Electron.warnBeforeExit": {
supportedLevels: [SettingLevel.PLATFORM], supportedLevels: [SettingLevel.PLATFORM],

View File

@@ -10,9 +10,6 @@ import SettingsHandler from "./SettingsHandler";
import PlatformPeg from "../../PlatformPeg"; import PlatformPeg from "../../PlatformPeg";
import { SETTINGS } from "../Settings"; import { SETTINGS } from "../Settings";
import { SettingLevel } from "../SettingLevel"; 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. * Gets and sets settings at the "platform" level for the current device.
@@ -24,22 +21,22 @@ export default class PlatformSettingsHandler extends SettingsHandler {
public constructor() { public constructor() {
super(); super();
defaultDispatcher.register(this.onAction); void this.setup();
} }
private onAction = (payload: ActionPayload): void => { private async setup(): Promise<void> {
if (payload.action === Action.PlatformSet) { const platform = await PlatformPeg.platformPromise;
this.store = {}; await platform.initialised;
// Load setting values as they are async and `getValue` must be synchronous
Object.entries(SETTINGS).forEach(([key, setting]) => { // Load setting values as they are async and `getValue` must be synchronous
if (setting.supportedLevels?.includes(SettingLevel.PLATFORM) && payload.platform.supportsSetting(key)) { Object.entries(SETTINGS).forEach(([key, setting]) => {
payload.platform.getSettingValue(key).then((value: any) => { if (setting.supportedLevels?.includes(SettingLevel.PLATFORM) && platform.supportsSetting(key)) {
this.store[key] = value; platform.getSettingValue(key).then((value: any) => {
}); this.store[key] = value;
} });
}); }
} });
}; }
public canSetValue(settingName: string, roomId: string): boolean { public canSetValue(settingName: string, roomId: string): boolean {
return PlatformPeg.get()?.supportsSetting(settingName) ?? false; return PlatformPeg.get()?.supportsSetting(settingName) ?? false;

View File

@@ -374,11 +374,13 @@ export default class ElectronPlatform extends BasePlatform {
return this.supportedSettings?.[settingName] === true; return this.supportedSettings?.[settingName] === true;
} }
public getSettingValue(settingName: string): Promise<any> { public async getSettingValue(settingName: string): Promise<any> {
await this.initialised;
return this.electron.getSettingValue(settingName); return this.electron.getSettingValue(settingName);
} }
public setSettingValue(settingName: string, value: any): Promise<void> { public async setSettingValue(settingName: string, value: any): Promise<void> {
await this.initialised;
return this.electron.setSettingValue(settingName, value); return this.electron.setSettingValue(settingName, value);
} }

View File

@@ -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("<SettingsDropdown />", () => {
it("should render a disabled setting", async () => {
const { asFragment } = render(
<SettingsDropdown settingKey="Electron.autoLaunch" level={SettingLevel.PLATFORM} />,
);
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(
<SettingsDropdown settingKey="Electron.autoLaunch" level={SettingLevel.PLATFORM} hideIfCannotSet />,
);
expect(container).toBeEmptyDOMElement();
});
it("should not render a non-options setting", async () => {
const { container } = render(<SettingsDropdown settingKey="systemFont" level={SettingLevel.DEVICE} />);
expect(container).toBeEmptyDOMElement();
});
});

View File

@@ -0,0 +1,48 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`<SettingsDropdown /> should render a disabled setting 1`] = `
<DocumentFragment>
<div
class="mx_SettingsDropdown"
>
<label
class="mx_SettingsDropdown_label"
for="«r0»"
>
<span
class="mx_SettingsDropdown_labelText"
>
Open %(brand)s when you log in to your computer
</span>
</label>
<div
class="mx_Dropdown mx_Dropdown_disabled"
>
<div
aria-describedby="«r0»_value"
aria-disabled="true"
aria-expanded="false"
aria-haspopup="listbox"
aria-label="Open %(brand)s when you log in to your computer"
aria-owns="«r0»_input"
class="mx_AccessibleButton mx_Dropdown_input mx_no_textinput mx_AccessibleButton_disabled"
disabled=""
role="button"
tabindex="0"
>
<div
class="mx_Dropdown_option"
id="«r0»_value"
>
<div>
No
</div>
</div>
<span
class="mx_Dropdown_arrow"
/>
</div>
</div>
</div>
</DocumentFragment>
`;