From 85b04e02dca0691617eb9a1db0ed7742b1d34cab Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 8 May 2024 15:29:39 -0400 Subject: [PATCH] Refactor settings to use observables Also removing some unused settings along the way. --- public/locales/en-GB/app.json | 1 - src/analytics/PosthogAnalytics.ts | 31 +++------ src/home/RegisteredView.tsx | 7 +- src/home/UnauthenticatedView.tsx | 7 +- src/livekit/MediaDevicesContext.tsx | 42 +++++------ src/room/RoomPage.tsx | 7 +- src/settings/SettingsModal.tsx | 33 +++------ src/settings/settings.ts | 98 ++++++++++++++++++++++++++ src/settings/useSetting.ts | 104 ---------------------------- src/state/useObservable.ts | 41 ++++++++++- 10 files changed, 190 insertions(+), 181 deletions(-) create mode 100644 src/settings/settings.ts delete mode 100644 src/settings/useSetting.ts diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 7a804a5dd..4279bbb51 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -140,7 +140,6 @@ "feedback_tab_title": "Feedback", "more_tab_title": "More", "opt_in_description": "<0><1>You may withdraw consent by unchecking this box. If you are currently in a call, this setting will take effect at the end of the call.", - "show_connection_stats_label": "Show connection stats", "speaker_device_selection_label": "Speaker" }, "star_rating_input_label_one": "{{count}} stars", diff --git a/src/analytics/PosthogAnalytics.ts b/src/analytics/PosthogAnalytics.ts index cfeb1e7a1..5cbc552b4 100644 --- a/src/analytics/PosthogAnalytics.ts +++ b/src/analytics/PosthogAnalytics.ts @@ -20,7 +20,6 @@ import { MatrixClient } from "matrix-js-sdk"; import { Buffer } from "buffer"; import { widget } from "../widget"; -import { getSetting, setSetting, getSettingKey } from "../settings/useSetting"; import { CallEndedTracker, CallStartedTracker, @@ -35,7 +34,7 @@ import { } from "./PosthogEvents"; import { Config } from "../config/Config"; import { getUrlParams } from "../UrlParams"; -import { localStorageBus } from "../useLocalStorage"; +import { optInAnalytics } from "../settings/settings"; /* Posthog analytics tracking. * @@ -131,7 +130,7 @@ export class PosthogAnalytics { const { analyticsID } = getUrlParams(); // if the embedding platform (element web) already got approval to communicating with posthog // element call can also send events to posthog - setSetting("opt-in-analytics", Boolean(analyticsID)); + optInAnalytics.setValue(Boolean(analyticsID)); } this.posthog.init(posthogConfig.project_api_key, { @@ -151,9 +150,7 @@ export class PosthogAnalytics { ); this.enabled = false; } - this.startListeningToSettingsChanges(); - const optInAnalytics = getSetting("opt-in-analytics", false); - this.updateAnonymityAndIdentifyUser(optInAnalytics); + this.startListeningToSettingsChanges(); // Triggers maybeIdentifyUser } private sanitizeProperties = ( @@ -336,8 +333,7 @@ export class PosthogAnalytics { } public onLoginStatusChanged(): void { - const optInAnalytics = getSetting("opt-in-analytics", false); - this.updateAnonymityAndIdentifyUser(optInAnalytics); + this.maybeIdentifyUser(); } private updateSuperProperties(): void { @@ -360,20 +356,12 @@ export class PosthogAnalytics { return this.eventSignup.getSignupEndTime() > new Date(0); } - private async updateAnonymityAndIdentifyUser( - pseudonymousOptIn: boolean, - ): Promise { - // Update this.anonymity based on the user's analytics opt-in settings - const anonymity = pseudonymousOptIn - ? Anonymity.Pseudonymous - : Anonymity.Disabled; - this.setAnonymity(anonymity); - + private async maybeIdentifyUser(): Promise { // We may not yet have a Matrix client at this point, if not, bail. This should get // triggered again by onLoginStatusChanged once we do have a client. if (!window.matrixclient) return; - if (anonymity === Anonymity.Pseudonymous) { + if (this.anonymity === Anonymity.Pseudonymous) { this.setRegistrationType( window.matrixclient.isGuest() || window.passwordlessUser ? RegistrationType.Guest @@ -389,7 +377,7 @@ export class PosthogAnalytics { } } - if (anonymity !== Anonymity.Disabled) { + if (this.anonymity !== Anonymity.Disabled) { this.updateSuperProperties(); } } @@ -419,8 +407,9 @@ export class PosthogAnalytics { // * When the user changes their preferences on this device // Note that for new accounts, pseudonymousAnalyticsOptIn won't be set, so updateAnonymityFromSettings // won't be called (i.e. this.anonymity will be left as the default, until the setting changes) - localStorageBus.on(getSettingKey("opt-in-analytics"), (optInAnalytics) => { - this.updateAnonymityAndIdentifyUser(optInAnalytics); + optInAnalytics.value.subscribe((optIn) => { + this.setAnonymity(optIn ? Anonymity.Pseudonymous : Anonymity.Disabled); + this.maybeIdentifyUser(); }); } diff --git a/src/home/RegisteredView.tsx b/src/home/RegisteredView.tsx index 35e958ab0..1c1b0d3af 100644 --- a/src/home/RegisteredView.tsx +++ b/src/home/RegisteredView.tsx @@ -38,9 +38,12 @@ import { UserMenuContainer } from "../UserMenuContainer"; import { JoinExistingCallModal } from "./JoinExistingCallModal"; import { Caption } from "../typography/Typography"; import { Form } from "../form/Form"; -import { useOptInAnalytics } from "../settings/useSetting"; import { AnalyticsNotice } from "../analytics/AnalyticsNotice"; import { E2eeType } from "../e2ee/e2eeType"; +import { + useSetting, + optInAnalytics as optInAnalyticsSetting, +} from "../settings/settings"; interface Props { client: MatrixClient; @@ -49,7 +52,7 @@ interface Props { export const RegisteredView: FC = ({ client }) => { const [loading, setLoading] = useState(false); const [error, setError] = useState(); - const [optInAnalytics] = useOptInAnalytics(); + const [optInAnalytics] = useSetting(optInAnalyticsSetting); const history = useHistory(); const { t } = useTranslation(); const [joinExistingCallModalOpen, setJoinExistingCallModalOpen] = diff --git a/src/home/UnauthenticatedView.tsx b/src/home/UnauthenticatedView.tsx index d5f00fea5..35cc832e2 100644 --- a/src/home/UnauthenticatedView.tsx +++ b/src/home/UnauthenticatedView.tsx @@ -41,15 +41,18 @@ import styles from "./UnauthenticatedView.module.css"; import commonStyles from "./common.module.css"; import { generateRandomName } from "../auth/generateRandomName"; import { AnalyticsNotice } from "../analytics/AnalyticsNotice"; -import { useOptInAnalytics } from "../settings/useSetting"; import { Config } from "../config/Config"; import { E2eeType } from "../e2ee/e2eeType"; +import { + useSetting, + optInAnalytics as optInAnalyticsSetting, +} from "../settings/settings"; export const UnauthenticatedView: FC = () => { const { setClient } = useClient(); const [loading, setLoading] = useState(false); const [error, setError] = useState(); - const [optInAnalytics] = useOptInAnalytics(); + const [optInAnalytics] = useSetting(optInAnalyticsSetting); const { recaptchaKey, register } = useInteractiveRegistration(); const { execute, reset, recaptchaId } = useRecaptcha(recaptchaKey); diff --git a/src/livekit/MediaDevicesContext.tsx b/src/livekit/MediaDevicesContext.tsx index 027f12ae9..19fa1dbdd 100644 --- a/src/livekit/MediaDevicesContext.tsx +++ b/src/livekit/MediaDevicesContext.tsx @@ -29,11 +29,12 @@ import { Observable } from "rxjs"; import { logger } from "matrix-js-sdk/src/logger"; import { + useSetting, + audioInput as audioInputSetting, + audioOutput as audioOutputSetting, + videoInput as videoInputSetting, isFirefox, - useAudioInput, - useAudioOutput, - useVideoInput, -} from "../settings/useSetting"; +} from "../settings/settings"; export interface MediaDevice { available: MediaDeviceInfo[]; @@ -145,43 +146,36 @@ export const MediaDevicesProvider: FC = ({ children }) => { // for ouput devices because the selector wont be shown on FF. const useOutputNames = usingNames && !isFirefox(); - const [audioInputSetting, setAudioInputSetting] = useAudioInput(); - const [audioOutputSetting, setAudioOutputSetting] = useAudioOutput(); - const [videoInputSetting, setVideoInputSetting] = useVideoInput(); + const [storedAudioInput, setStoredAudioInput] = useSetting(audioInputSetting); + const [storedAudioOutput, setStoredAudioOutput] = + useSetting(audioOutputSetting); + const [storedVideoInput, setStoredVideoInput] = useSetting(videoInputSetting); - const audioInput = useMediaDevice( - "audioinput", - audioInputSetting, - usingNames, - ); + const audioInput = useMediaDevice("audioinput", storedAudioInput, usingNames); const audioOutput = useMediaDevice( "audiooutput", - audioOutputSetting, + storedAudioOutput, useOutputNames, alwaysUseDefaultAudio, ); - const videoInput = useMediaDevice( - "videoinput", - videoInputSetting, - usingNames, - ); + const videoInput = useMediaDevice("videoinput", storedVideoInput, usingNames); useEffect(() => { if (audioInput.selectedId !== undefined) - setAudioInputSetting(audioInput.selectedId); - }, [setAudioInputSetting, audioInput.selectedId]); + setStoredAudioInput(audioInput.selectedId); + }, [setStoredAudioInput, audioInput.selectedId]); useEffect(() => { // Skip setting state for ff output. Redundent since it is set to always return 'undefined' // but makes it clear while debugging that this is not happening on FF. + perf ;) if (audioOutput.selectedId !== undefined && !isFirefox()) - setAudioOutputSetting(audioOutput.selectedId); - }, [setAudioOutputSetting, audioOutput.selectedId]); + setStoredAudioOutput(audioOutput.selectedId); + }, [setStoredAudioOutput, audioOutput.selectedId]); useEffect(() => { if (videoInput.selectedId !== undefined) - setVideoInputSetting(videoInput.selectedId); - }, [setVideoInputSetting, videoInput.selectedId]); + setStoredVideoInput(videoInput.selectedId); + }, [setStoredVideoInput, videoInput.selectedId]); const startUsingDeviceNames = useCallback( () => setNumCallersUsingNames((n) => n + 1), diff --git a/src/room/RoomPage.tsx b/src/room/RoomPage.tsx index 2f18cb638..1a14fb7ed 100644 --- a/src/room/RoomPage.tsx +++ b/src/room/RoomPage.tsx @@ -26,7 +26,6 @@ import { GroupCallLoader } from "./GroupCallLoader"; import { GroupCallView } from "./GroupCallView"; import { useRoomIdentifier, useUrlParams } from "../UrlParams"; import { useRegisterPasswordlessUser } from "../auth/useRegisterPasswordlessUser"; -import { useOptInAnalytics } from "../settings/useSetting"; import { HomePage } from "../home/HomePage"; import { platform } from "../Platform"; import { AppSelectionModal } from "./AppSelectionModal"; @@ -36,6 +35,10 @@ import { LobbyView } from "./LobbyView"; import { E2eeType } from "../e2ee/e2eeType"; import { useProfile } from "../profile/useProfile"; import { useMuteStates } from "./MuteStates"; +import { + useSetting, + optInAnalytics as optInAnalyticsSetting, +} from "../settings/settings"; export const RoomPage: FC = () => { const { @@ -80,7 +83,7 @@ export const RoomPage: FC = () => { registerPasswordlessUser, ]); - const [optInAnalytics, setOptInAnalytics] = useOptInAnalytics(); + const [optInAnalytics, setOptInAnalytics] = useSetting(optInAnalyticsSetting); useEffect(() => { // During the beta, opt into analytics by default if (optInAnalytics === null && setOptInAnalytics) setOptInAnalytics(true); diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index a86f20c53..05784d159 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -29,12 +29,6 @@ import OverflowIcon from "../icons/Overflow.svg?react"; import UserIcon from "../icons/User.svg?react"; import FeedbackIcon from "../icons/Feedback.svg?react"; import { SelectInput } from "../input/SelectInput"; -import { - useOptInAnalytics, - useDeveloperSettingsTab, - useShowConnectionStats, - isFirefox, -} from "./useSetting"; import { FieldRow, InputField } from "../input/Input"; import { Body, Caption } from "../typography/Typography"; import { AnalyticsNotice } from "../analytics/AnalyticsNotice"; @@ -46,6 +40,12 @@ import { useMediaDeviceNames, } from "../livekit/MediaDevicesContext"; import { widget } from "../widget"; +import { + useSetting, + optInAnalytics as optInAnalyticsSetting, + developerSettingsTab as developerSettingsTabSetting, + isFirefox, +} from "./settings"; type SettingsTab = | "audio" @@ -76,11 +76,10 @@ export const SettingsModal: FC = ({ }) => { const { t } = useTranslation(); - const [optInAnalytics, setOptInAnalytics] = useOptInAnalytics(); - const [developerSettingsTab, setDeveloperSettingsTab] = - useDeveloperSettingsTab(); - const [showConnectionStats, setShowConnectionStats] = - useShowConnectionStats(); + const [optInAnalytics, setOptInAnalytics] = useSetting(optInAnalyticsSetting); + const [developerSettingsTab, setDeveloperSettingsTab] = useSetting( + developerSettingsTabSetting, + ); // Generate a `SelectInput` with a list of devices for a given device kind. const generateDeviceSelection = ( @@ -245,18 +244,6 @@ export const SettingsModal: FC = ({ })} - - ): void => - setShowConnectionStats(e.target.checked) - } - /> - ); diff --git a/src/settings/settings.ts b/src/settings/settings.ts new file mode 100644 index 000000000..bfffa0c5e --- /dev/null +++ b/src/settings/settings.ts @@ -0,0 +1,98 @@ +/* +Copyright 2024 New Vector Ltd + +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 { logger } from "matrix-js-sdk/src/logger"; +import { BehaviorSubject, Observable } from "rxjs"; + +import { useObservableValue } from "../state/useObservable"; +import { PosthogAnalytics } from "../analytics/PosthogAnalytics"; + +export class Setting { + public constructor(key: string, defaultValue: T) { + this.key = `matrix-setting-${key}`; + + const storedValue = localStorage.getItem(this.key); + let initialValue = defaultValue; + if (storedValue !== null) { + try { + initialValue = JSON.parse(storedValue); + } catch (e) { + logger.warn(`Invalid value stored for setting ${key}: ${storedValue}`); + } + } + + this._value = new BehaviorSubject(initialValue); + this.value = this._value; + } + + private readonly key: string; + + private readonly _value: BehaviorSubject; + public readonly value: Observable; + + public readonly setValue = (value: T): void => { + this._value.next(value); + localStorage.setItem(this.key, JSON.stringify(value)); + }; +} + +/** + * React hook that returns a settings's current value and a setter. + */ +export function useSetting(setting: Setting): [T, (value: T) => void] { + return [useObservableValue(setting.value), setting.setValue]; +} + +// TODO: This doesn't belong here +export const isFirefox = (): boolean => { + const { userAgent } = navigator; + return userAgent.includes("Firefox"); +}; + +// null = undecided +export const optInAnalytics = new Setting( + "opt-in-analytics", + null, +); +// TODO: This setting can be disabled. Work out an approach to disableable +// settings thats works for Observables in addition to React. +export const useOptInAnalytics = (): [ + boolean | null, + ((value: boolean | null) => void) | null, +] => { + const setting = useSetting(optInAnalytics); + if (PosthogAnalytics.instance.isEnabled()) return setting; + + return [false, null]; +}; + +export const developerSettingsTab = new Setting( + "developer-settings-tab", + false, +); + +export const audioInput = new Setting( + "audio-input", + undefined, +); +export const audioOutput = new Setting( + "audio-output", + undefined, +); +export const videoInput = new Setting( + "video-input", + undefined, +); diff --git a/src/settings/useSetting.ts b/src/settings/useSetting.ts deleted file mode 100644 index a2733b98c..000000000 --- a/src/settings/useSetting.ts +++ /dev/null @@ -1,104 +0,0 @@ -/* -Copyright 2022 - 2023 New Vector Ltd - -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 { useCallback, useMemo } from "react"; - -import { PosthogAnalytics } from "../analytics/PosthogAnalytics"; -import { - getLocalStorageItem, - setLocalStorageItem, - useLocalStorage, -} from "../useLocalStorage"; - -type Setting = [T, (value: T) => void]; -type DisableableSetting = [T, ((value: T) => void) | null]; - -export const getSettingKey = (name: string): string => { - return `matrix-setting-${name}`; -}; -// Like useState, but reads from and persists the value to localStorage -export const useSetting = (name: string, defaultValue: T): Setting => { - const key = useMemo(() => getSettingKey(name), [name]); - - const [item, setItem] = useLocalStorage(key); - - const value = useMemo( - () => (item == null ? defaultValue : JSON.parse(item)), - [item, defaultValue], - ); - const setValue = useCallback( - (value: T) => { - setItem(JSON.stringify(value)); - }, - [setItem], - ); - - return [value, setValue]; -}; - -export const getSetting = (name: string, defaultValue: T): T => { - const item = getLocalStorageItem(getSettingKey(name)); - return item === null ? defaultValue : JSON.parse(item); -}; - -export const setSetting = (name: string, newValue: T): void => - setLocalStorageItem(getSettingKey(name), JSON.stringify(newValue)); - -export const isFirefox = (): boolean => { - const { userAgent } = navigator; - return userAgent.includes("Firefox"); -}; - -const canEnableSpatialAudio = (): boolean => { - // Spatial audio means routing audio through audio contexts. On Chrome, - // this bypasses the AEC processor and so breaks echo cancellation. - // We only allow spatial audio to be enabled on Firefox which we know - // passes audio context audio through the AEC algorithm. - // https://bugs.chromium.org/p/chromium/issues/detail?id=687574 is the - // chrome bug for this: once this is fixed and the updated version is deployed - // widely enough, we can allow spatial audio everywhere. It's currently in a - // chrome flag, so we could enable this in Electron if we enabled the chrome flag - // in the Electron wrapper. - return isFirefox(); -}; - -export const useSpatialAudio = (): DisableableSetting => { - const settingVal = useSetting("spatial-audio", false); - if (canEnableSpatialAudio()) return settingVal; - - return [false, null]; -}; - -// null = undecided -export const useOptInAnalytics = (): DisableableSetting => { - const settingVal = useSetting("opt-in-analytics", null); - if (PosthogAnalytics.instance.isEnabled()) return settingVal; - - return [false, null]; -}; - -export const useDeveloperSettingsTab = (): Setting => - useSetting("developer-settings-tab", false); - -export const useShowConnectionStats = (): Setting => - useSetting("show-connection-stats", false); - -export const useAudioInput = (): Setting => - useSetting("audio-input", undefined); -export const useAudioOutput = (): Setting => - useSetting("audio-output", undefined); -export const useVideoInput = (): Setting => - useSetting("video-input", undefined); diff --git a/src/state/useObservable.ts b/src/state/useObservable.ts index 037c3bd59..5704f6c2f 100644 --- a/src/state/useObservable.ts +++ b/src/state/useObservable.ts @@ -14,10 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Ref, useCallback, useRef } from "react"; -import { BehaviorSubject, Observable } from "rxjs"; +import { Ref, useCallback, useRef, useState } from "react"; +import { BehaviorSubject, Observable, Subscription } from "rxjs"; import { useInitial } from "../useInitial"; +import { usePrevious } from "../usePrevious"; /** * React hook that creates an Observable from a changing value. The Observable @@ -41,3 +42,39 @@ export function useObservableRef(initialValue: T): [Observable, Ref] { const ref = useCallback((value: T) => subject.next(value), [subject]); return [subject, ref]; } + +const unknownValue = Symbol("unknown value"); + +/** + * React hook that gets a changing value from an Observable. The Observable must + * replay its current value upon subscription and emit whenever the value + * changes. + */ +export function useObservableValue(observable: Observable): T { + const subscription = useRef(); + const prevObservable = usePrevious(observable); + const value = useRef(unknownValue); + const [, forceUpdate] = useState(0); + + if (observable !== prevObservable) { + subscription.current?.unsubscribe(); + value.current = unknownValue; + // We want to extract a value immediately, rather than using an effect and + // having to wait until the next render + subscription.current = observable.subscribe((v) => { + // As long as the Observable replays its current value upon subscription, + // this will execute synchronously and we will have our initial value + const synchronous = value.current === unknownValue; + value.current = v; + // If this is an asynchronous update to the Observable's value, trigger a + // new render + if (!synchronous) forceUpdate((n) => n + 1); + }); + } + + if (value.current === unknownValue) + throw new Error( + "Observable did not replay its current value upon subscription", + ); + return value.current; +}