Start refactoring some business logic into view models

As Element Call grows in complexity, it has become a pain point that our business logic remains so tightly coupled to the UI code. In particular, this has made testing difficult, and the complex semantics of React hooks are not a great match for arbitrary business logic. Here, I show the beginnings of what it would look like for us to adopt the MVVM pattern. I've created a CallViewModel and TileViewModel that expose their state to the UI as rxjs Observables, as well as a couple of helper functions for consuming view models in React code.

This should contain no user-visible changes, but we need to watch out for regressions particularly around focus switching and promotion of speakers, because this was the logic I chose to refactor first.
This commit is contained in:
Robin
2023-11-30 22:59:19 -05:00
parent 445c7c4e0c
commit 169ccd9de5
23 changed files with 847 additions and 531 deletions

View File

@@ -19,11 +19,11 @@ import { RectReadOnly } from "react-use-measure";
import { FC, memo, ReactNode } from "react";
import { zip } from "lodash";
import { TileDescriptor } from "./VideoGrid";
import { Slot } from "./NewVideoGrid";
import { Layout } from "./Layout";
import { count, findLastIndex } from "../array-utils";
import styles from "./BigGrid.module.css";
import { TileDescriptor } from "../state/CallViewModel";
/**
* A 1×1 cell in a grid which belongs to a tile.

View File

@@ -18,7 +18,7 @@ import { ComponentType, ReactNode, useCallback, useMemo, useRef } from "react";
import type { RectReadOnly } from "react-use-measure";
import { useReactiveState } from "../useReactiveState";
import type { TileDescriptor } from "./VideoGrid";
import { TileDescriptor } from "../state/CallViewModel";
/**
* A video grid layout system with concrete states of type State.

View File

@@ -32,7 +32,6 @@ import styles from "./NewVideoGrid.module.css";
import {
VideoGridProps as Props,
TileSpring,
TileDescriptor,
ChildrenProperties,
} from "./VideoGrid";
import { useReactiveState } from "../useReactiveState";
@@ -40,6 +39,7 @@ import { useMergedRefs } from "../useMergedRefs";
import { TileWrapper } from "./TileWrapper";
import { BigGrid } from "./BigGrid";
import { useLayout } from "./Layout";
import { TileDescriptor } from "../state/CallViewModel";
interface Rect {
x: number;

View File

@@ -44,6 +44,7 @@ import styles from "./VideoGrid.module.css";
import { Layout } from "../room/LayoutToggle";
import { TileWrapper } from "./TileWrapper";
import { LayoutStatesMap } from "./Layout";
import { TileDescriptor } from "../state/CallViewModel";
interface TilePosition {
x: number;
@@ -838,20 +839,6 @@ export interface VideoGridProps<T> {
children: (props: ChildrenProperties<T>) => ReactNode;
}
// Represents something that should get a tile on the layout,
// ie. a user's video feed or a screen share feed.
export interface TileDescriptor<T> {
id: string;
focused: boolean;
isPresenter: boolean;
isSpeaker: boolean;
hasVideo: boolean;
local: boolean;
largeBaseSize: boolean;
placeNear?: string;
data: T;
}
export function VideoGrid<T>({
items,
layout,

View File

@@ -45,6 +45,11 @@ import { useReactiveState } from "../useReactiveState";
import { AudioButton, FullscreenButton } from "../button/Button";
import { VideoTileSettingsModal } from "./VideoTileSettingsModal";
import { MatrixInfo } from "../room/VideoPreview";
import {
ScreenShareTileViewModel,
TileViewModel,
UserMediaTileViewModel,
} from "../state/TileViewModel";
export interface ItemData {
id: string;
@@ -59,7 +64,7 @@ export enum TileContent {
}
interface Props {
data: ItemData;
vm: TileViewModel;
maximised: boolean;
fullscreen: boolean;
onToggleFullscreen: (itemId: string) => void;
@@ -78,7 +83,7 @@ interface Props {
export const VideoTile = forwardRef<HTMLDivElement, Props>(
(
{
data,
vm,
maximised,
fullscreen,
onToggleFullscreen,
@@ -94,7 +99,7 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
) => {
const { t } = useTranslation();
const { content, sfuParticipant, member } = data;
const { id, sfuParticipant, member } = vm;
// Handle display name changes.
const [displayName, setDisplayName] = useReactiveState(
@@ -115,13 +120,13 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
}, [member, setDisplayName]);
const audioInfo = useMediaTrack(
content === TileContent.UserMedia
vm instanceof UserMediaTileViewModel
? Track.Source.Microphone
: Track.Source.ScreenShareAudio,
sfuParticipant,
);
const videoInfo = useMediaTrack(
content === TileContent.UserMedia
vm instanceof UserMediaTileViewModel
? Track.Source.Camera
: Track.Source.ScreenShare,
sfuParticipant,
@@ -134,8 +139,8 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
const MicIcon = muted ? MicOffSolidIcon : MicOnSolidIcon;
const onFullscreen = useCallback(() => {
onToggleFullscreen(data.id);
}, [data, onToggleFullscreen]);
onToggleFullscreen(id);
}, [id, onToggleFullscreen]);
const [videoTileSettingsModalOpen, setVideoTileSettingsModalOpen] =
useState(false);
@@ -159,7 +164,7 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
/>,
);
if (content === TileContent.ScreenShare) {
if (vm instanceof ScreenShareTileViewModel) {
toolbarButtons.push(
<FullscreenButton
key="fullscreen"
@@ -177,9 +182,9 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
[styles.isLocal]: sfuParticipant.isLocal,
[styles.speaking]:
sfuParticipant.isSpeaking &&
content === TileContent.UserMedia &&
vm instanceof UserMediaTileViewModel &&
showSpeakingIndicator,
[styles.screenshare]: content === TileContent.ScreenShare,
[styles.screenshare]: vm instanceof ScreenShareTileViewModel,
[styles.maximised]: maximised,
})}
style={style}
@@ -189,7 +194,7 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
{toolbarButtons.length > 0 && (!maximised || fullscreen) && (
<div className={classNames(styles.toolbar)}>{toolbarButtons}</div>
)}
{content === TileContent.UserMedia &&
{vm instanceof UserMediaTileViewModel &&
!sfuParticipant.isCameraEnabled && (
<>
<div className={styles.videoMutedOverlay} />
@@ -203,7 +208,7 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
/>
</>
)}
{content === TileContent.ScreenShare ? (
{vm instanceof ScreenShareTileViewModel ? (
<div className={styles.presenterLabel}>
<span>{t("video_tile.presenter_label", { displayName })}</span>
</div>
@@ -245,7 +250,7 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
<VideoTrack
participant={sfuParticipant}
source={
content === TileContent.UserMedia
vm instanceof UserMediaTileViewModel
? Track.Source.Camera
: Track.Source.ScreenShare
}
@@ -260,9 +265,14 @@ export const VideoTile = forwardRef<HTMLDivElement, Props>(
// eslint-disable-next-line react/no-unknown-property
disablepictureinpicture="true"
/>
{!maximised && (
{!maximised && sfuParticipant instanceof RemoteParticipant && (
<VideoTileSettingsModal
data={data}
participant={sfuParticipant}
media={
vm instanceof UserMediaTileViewModel
? "user media"
: "screen share"
}
open={videoTileSettingsModalOpen}
onDismiss={closeVideoTileSettingsModal}
/>

View File

@@ -22,19 +22,18 @@ import { FieldRow } from "../input/Input";
import { Modal } from "../Modal";
import styles from "./VideoTileSettingsModal.module.css";
import { VolumeIcon } from "../button/VolumeIcon";
import { ItemData, TileContent } from "./VideoTile";
interface LocalVolumeProps {
participant: RemoteParticipant;
content: TileContent;
media: "user media" | "screen share";
}
const LocalVolume: FC<LocalVolumeProps> = ({
participant,
content,
media,
}: LocalVolumeProps) => {
const source =
content === TileContent.UserMedia
media === "user media"
? Track.Source.Microphone
: Track.Source.ScreenShareAudio;
@@ -67,13 +66,15 @@ const LocalVolume: FC<LocalVolumeProps> = ({
};
interface Props {
data: ItemData;
participant: RemoteParticipant;
media: "user media" | "screen share";
open: boolean;
onDismiss: () => void;
}
export const VideoTileSettingsModal: FC<Props> = ({
data,
participant,
media,
open,
onDismiss,
}) => {
@@ -87,10 +88,7 @@ export const VideoTileSettingsModal: FC<Props> = ({
onDismiss={onDismiss}
>
<div className={styles.content}>
<LocalVolume
participant={data.sfuParticipant as RemoteParticipant}
content={data.content}
/>
<LocalVolume participant={participant} media={media} />
</div>
</Modal>
);