From 9080f3dd552d5ed44ad733cb9adf587b443511e5 Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 29 May 2023 10:19:37 +1200 Subject: [PATCH] await promises (#10992) --- .../tabs/user/SidebarUserSettingsTab.tsx | 4 +- .../tabs/user/VoiceUserSettingsTab.tsx | 37 +++++++++++++++---- .../tabs/user/VoiceUserSettingsTab-test.tsx | 27 +++++++++++++- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx b/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx index 3ff7afdf49..3c9aaffe55 100644 --- a/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx @@ -34,9 +34,9 @@ import SettingsSubsection, { SettingsSubsectionText } from "../../shared/Setting type InteractionName = "WebSettingsSidebarTabSpacesCheckbox" | "WebQuickSettingsPinToSidebarCheckbox"; export const onMetaSpaceChangeFactory = - (metaSpace: MetaSpace, interactionName: InteractionName) => (e: ChangeEvent) => { + (metaSpace: MetaSpace, interactionName: InteractionName) => async (e: ChangeEvent) => { const currentValue = SettingsStore.getValue("Spaces.enabledMetaSpaces"); - SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.ACCOUNT, { + await SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.ACCOUNT, { ...currentValue, [metaSpace]: e.target.checked, }); diff --git a/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx b/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx index 2821817466..f70fdee33a 100644 --- a/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx @@ -16,6 +16,7 @@ limitations under the License. */ import React, { ReactNode } from "react"; +import { logger } from "matrix-js-sdk/src/logger"; import { _t } from "../../../../../languageHandler"; import MediaDeviceHandler, { IMediaDevices, MediaDeviceKindEnum } from "../../../../../MediaDeviceHandler"; @@ -40,6 +41,21 @@ interface IState { audioNoiseSuppression: boolean; } +/** + * Maps deviceKind to the right get method on MediaDeviceHandler + * Helpful for setting state + */ +const mapDeviceKindToHandlerValue = (deviceKind: MediaDeviceKindEnum): string | null => { + switch (deviceKind) { + case MediaDeviceKindEnum.AudioOutput: + return MediaDeviceHandler.getAudioOutput(); + case MediaDeviceKindEnum.AudioInput: + return MediaDeviceHandler.getAudioInput(); + case MediaDeviceKindEnum.VideoInput: + return MediaDeviceHandler.getVideoInput(); + } +}; + export default class VoiceUserSettingsTab extends React.Component<{}, IState> { public constructor(props: {}) { super(props); @@ -58,16 +74,16 @@ export default class VoiceUserSettingsTab extends React.Component<{}, IState> { public async componentDidMount(): Promise { const canSeeDeviceLabels = await MediaDeviceHandler.hasAnyLabeledDevices(); if (canSeeDeviceLabels) { - this.refreshMediaDevices(); + await this.refreshMediaDevices(); } } private refreshMediaDevices = async (stream?: MediaStream): Promise => { this.setState({ mediaDevices: (await MediaDeviceHandler.getDevices()) ?? null, - [MediaDeviceKindEnum.AudioOutput]: MediaDeviceHandler.getAudioOutput(), - [MediaDeviceKindEnum.AudioInput]: MediaDeviceHandler.getAudioInput(), - [MediaDeviceKindEnum.VideoInput]: MediaDeviceHandler.getVideoInput(), + [MediaDeviceKindEnum.AudioOutput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.AudioOutput), + [MediaDeviceKindEnum.AudioInput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.AudioInput), + [MediaDeviceKindEnum.VideoInput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.VideoInput), }); if (stream) { // kill stream (after we've enumerated the devices, otherwise we'd get empty labels again) @@ -80,13 +96,20 @@ export default class VoiceUserSettingsTab extends React.Component<{}, IState> { private requestMediaPermissions = async (): Promise => { const stream = await requestMediaPermissions(); if (stream) { - this.refreshMediaDevices(stream); + await this.refreshMediaDevices(stream); } }; - private setDevice = (deviceId: string, kind: MediaDeviceKindEnum): void => { - MediaDeviceHandler.instance.setDevice(deviceId, kind); + private setDevice = async (deviceId: string, kind: MediaDeviceKindEnum): Promise => { + // set state immediately so UI is responsive this.setState({ [kind]: deviceId }); + try { + await MediaDeviceHandler.instance.setDevice(deviceId, kind); + } catch (error) { + logger.error(`Failed to set device ${kind}: ${deviceId}`); + // reset state to current value + this.setState({ [kind]: mapDeviceKindToHandlerValue(kind) }); + } }; private changeWebRtcMethod = (p2p: boolean): void => { diff --git a/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx index 1771b9c8ab..f9a9237f46 100644 --- a/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx @@ -17,6 +17,7 @@ limitations under the License. import React from "react"; import { mocked } from "jest-mock"; import { fireEvent, render, screen } from "@testing-library/react"; +import { logger } from "matrix-js-sdk/src/logger"; import VoiceUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/VoiceUserSettingsTab"; import MediaDeviceHandler, { IMediaDevices, MediaDeviceKindEnum } from "../../../../../../src/MediaDeviceHandler"; @@ -56,9 +57,10 @@ describe("", () => { jest.clearAllMocks(); MediaDeviceHandlerMock.hasAnyLabeledDevices.mockResolvedValue(true); MediaDeviceHandlerMock.getDevices.mockResolvedValue(defaultMediaDevices); + MediaDeviceHandlerMock.getVideoInput.mockReturnValue(videoIn1.deviceId); // @ts-ignore bad mocking - MediaDeviceHandlerMock.instance = { setDevice: jest.fn() }; + MediaDeviceHandlerMock.instance = { setDevice: jest.fn().mockResolvedValue(undefined) }; }); describe("devices", () => { @@ -84,6 +86,29 @@ describe("", () => { expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn2.label); }); + it("logs and resets device when update fails", async () => { + // stub to avoid littering console with expected error + jest.spyOn(logger, "error").mockImplementation(() => {}); + MediaDeviceHandlerMock.instance.setDevice.mockRejectedValue("oups!"); + render(getComponent()); + await flushPromises(); + + fireEvent.change(screen.getByLabelText("Camera"), { target: { value: videoIn2.deviceId } }); + + expect(MediaDeviceHandlerMock.instance.setDevice).toHaveBeenCalledWith( + videoIn2.deviceId, + MediaDeviceKindEnum.VideoInput, + ); + + expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn2.label); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith("Failed to set device videoinput: 3"); + // reset to original + expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn1.label); + }); + it("does not render dropdown when no devices exist for type", async () => { render(getComponent()); await flushPromises();