From 3f74ac37e8cedcb5ed283f0734305b829b0137a7 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 22 Nov 2022 07:57:38 +0100 Subject: [PATCH] Refactor PipView + fix strict errors (#9604) --- .../views/voip/PictureInPictureDragger.tsx | 9 ++- src/components/views/voip/PipView.tsx | 58 +++++++++++-------- test/components/views/voip/PipView-test.tsx | 45 ++++++++------ 3 files changed, 68 insertions(+), 44 deletions(-) diff --git a/src/components/views/voip/PictureInPictureDragger.tsx b/src/components/views/voip/PictureInPictureDragger.tsx index dd02a13e90..77babeefab 100644 --- a/src/components/views/voip/PictureInPictureDragger.tsx +++ b/src/components/views/voip/PictureInPictureDragger.tsx @@ -33,14 +33,21 @@ const PADDING = { right: 8, }; +/** + * The type of a callback which will create the pip content children. + */ +export type CreatePipChildren = (options: IChildrenOptions) => JSX.Element; + interface IChildrenOptions { + // a callback which is called when a mouse event (most likely mouse down) occurs at start of moving the pip around onStartMoving: (event: React.MouseEvent) => void; + // a callback which is called when the content fo the pip changes in a way that is likely to cause a resize onResize: (event: Event) => void; } interface IProps { className?: string; - children: ({ onStartMoving, onResize }: IChildrenOptions) => React.ReactNode; + children: CreatePipChildren; draggable: boolean; onDoubleClick?: () => void; onMove?: () => void; diff --git a/src/components/views/voip/PipView.tsx b/src/components/views/voip/PipView.tsx index 54140e0f4e..b5ac6a85f7 100644 --- a/src/components/views/voip/PipView.tsx +++ b/src/components/views/voip/PipView.tsx @@ -25,7 +25,7 @@ import LegacyCallView from "./LegacyCallView"; import LegacyCallHandler, { LegacyCallHandlerEvent } from '../../../LegacyCallHandler'; import PersistentApp from "../elements/PersistentApp"; import { MatrixClientPeg } from '../../../MatrixClientPeg'; -import PictureInPictureDragger from './PictureInPictureDragger'; +import PictureInPictureDragger, { CreatePipChildren } from './PictureInPictureDragger'; import dis from '../../../dispatcher/dispatcher'; import { Action } from "../../../dispatcher/actions"; import { Container, WidgetLayoutStore } from '../../../stores/widgets/WidgetLayoutStore'; @@ -129,7 +129,9 @@ function getPrimarySecondaryCallsForPip(roomId: Optional): [MatrixCall | */ class PipView extends React.Component { - private movePersistedElement = createRef<() => void>(); + // The cast is not so great, but solves the typing issue for the moment. + // Proper solution: use useRef (requires the component to be refactored to a functional component). + private movePersistedElement = createRef<() => void>() as React.MutableRefObject<() => void>; constructor(props: IProps) { super(props); @@ -328,9 +330,37 @@ class PipView extends React.Component { this.setState({ showWidgetInPip, persistentWidgetId, persistentRoomId }); } + private createVoiceBroadcastPreRecordingPipContent( + voiceBroadcastPreRecording: VoiceBroadcastPreRecording, + ): CreatePipChildren { + return ({ onStartMoving }) =>
+ +
; + } + + private createVoiceBroadcastRecordingPipContent( + voiceBroadcastRecording: VoiceBroadcastRecording, + ): CreatePipChildren { + return ({ onStartMoving }) =>
+ +
; + } + public render() { const pipMode = true; - let pipContent; + let pipContent: CreatePipChildren | null = null; + + if (this.props.voiceBroadcastPreRecording) { + pipContent = this.createVoiceBroadcastPreRecordingPipContent(this.props.voiceBroadcastPreRecording); + } + + if (this.props.voiceBroadcastRecording) { + pipContent = this.createVoiceBroadcastRecordingPipContent(this.props.voiceBroadcastRecording); + } if (this.state.primaryCall) { // get a ref to call inside the current scope @@ -359,7 +389,7 @@ class PipView extends React.Component { pipContent = ({ onStartMoving }) =>
{ onStartMoving(event); this.onStartMoving.bind(this)(); }} + onPipMouseDown={(event) => { onStartMoving?.(event); this.onStartMoving.bind(this)(); }} pipMode={pipMode} callRooms={[roomForWidget]} onExpand={!isCall && !viewingCallRoom ? this.onExpand : undefined} @@ -375,26 +405,6 @@ class PipView extends React.Component {
; } - if (this.props.voiceBroadcastPreRecording) { - // get a ref to pre-recording inside the current scope - const preRecording = this.props.voiceBroadcastPreRecording; - pipContent = ({ onStartMoving }) =>
- -
; - } - - if (this.props.voiceBroadcastRecording) { - // get a ref to recording inside the current scope - const recording = this.props.voiceBroadcastRecording; - pipContent = ({ onStartMoving }) =>
- -
; - } - if (!!pipContent) { return { ActiveWidgetStore.instance.destroyPersistentWidget("1", room.roomId); }; + const setUpVoiceBroadcastRecording = () => { + const voiceBroadcastInfoEvent = mkVoiceBroadcastInfoStateEvent( + room.roomId, + VoiceBroadcastInfoState.Started, + alice.userId, + client.getDeviceId() || "", + ); + + const voiceBroadcastRecording = new VoiceBroadcastRecording(voiceBroadcastInfoEvent, client); + voiceBroadcastRecordingsStore.setCurrent(voiceBroadcastRecording); + }; + + const setUpVoiceBroadcastPreRecording = () => { + const voiceBroadcastPreRecording = new VoiceBroadcastPreRecording( + room, + alice, + client, + voiceBroadcastRecordingsStore, + ); + voiceBroadcastPreRecordingStore.setCurrent(voiceBroadcastPreRecording); + }; + it("hides if there's no content", () => { renderPip(); expect(screen.queryByRole("complementary")).toBeNull(); @@ -199,18 +221,10 @@ describe("PipView", () => { }); }); - describe("when there is a voice broadcast recording", () => { + describe("when there is a voice broadcast recording and pre-recording", () => { beforeEach(() => { - const voiceBroadcastInfoEvent = mkVoiceBroadcastInfoStateEvent( - room.roomId, - VoiceBroadcastInfoState.Started, - alice.userId, - client.getDeviceId() || "", - ); - - const voiceBroadcastRecording = new VoiceBroadcastRecording(voiceBroadcastInfoEvent, client); - voiceBroadcastRecordingsStore.setCurrent(voiceBroadcastRecording); - + setUpVoiceBroadcastPreRecording(); + setUpVoiceBroadcastRecording(); renderPip(); }); @@ -222,14 +236,7 @@ describe("PipView", () => { describe("when there is a voice broadcast pre-recording", () => { beforeEach(() => { - const voiceBroadcastPreRecording = new VoiceBroadcastPreRecording( - room, - alice, - client, - voiceBroadcastRecordingsStore, - ); - voiceBroadcastPreRecordingStore.setCurrent(voiceBroadcastPreRecording); - + setUpVoiceBroadcastPreRecording(); renderPip(); });