Skip to content

Commit d3f069e

Browse files
robintownhughns
andauthored
Keep tiles in a stable order (#2670)
* Keep tiles in a stable order This introduces a new layer of abstraction on top of MediaViewModel: TileViewModel, which gives us a place to store data relating to tiles rather than their media, and also generally makes it easier to reason about tiles as they move about the call layout. I have created a class called TileStore to keep track of these tiles. This allows us to swap out the media shown on a tile as the spotlight speaker changes, and avoid moving tiles around unless they really need to jump between the visible/invisible regions of the layout. * Don't throttle spotlight updates Since we now assume that the spotlight and grid will be in sync (i.e. an active speaker in one will behave as an active speaker in the other), we don't want the spotlight to ever lag behind due to throttling. If this causes usability issues we should maybe look into making LiveKit's 'speaking' indicators less erratic first. * Make layout shifts due to a change in speaker less surprising Although we try now to avoid layout shifts due to the spotlight speaker changing wherever possible, a spotlight speaker coming from off screen can still trigger one. Let's shift the layout a bit more gracefully in this case. * Improve the tile ordering tests * Maximize the spotlight tile in portrait layout * Tell tiles whether they're actually visible in a more timely manner * Fix test * Fix speaking indicators logic * Improve readability of marbles * Fix test case --------- Co-authored-by: Hugh Nimmo-Smith <[email protected]>
1 parent 22cca28 commit d3f069e

22 files changed

+1178
-339
lines changed

src/grid/CallLayout.ts

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ Please see LICENSE in the repository root for full details.
88
import { BehaviorSubject, Observable } from "rxjs";
99
import { ComponentType } from "react";
1010

11-
import { MediaViewModel, UserMediaViewModel } from "../state/MediaViewModel";
1211
import { LayoutProps } from "./Grid";
12+
import { TileViewModel } from "../state/TileViewModel";
1313

1414
export interface Bounds {
1515
width: number;
@@ -42,19 +42,6 @@ export interface CallLayoutInputs {
4242
pipAlignment: BehaviorSubject<Alignment>;
4343
}
4444

45-
export interface GridTileModel {
46-
type: "grid";
47-
vm: UserMediaViewModel;
48-
}
49-
50-
export interface SpotlightTileModel {
51-
type: "spotlight";
52-
vms: MediaViewModel[];
53-
maximised: boolean;
54-
}
55-
56-
export type TileModel = GridTileModel | SpotlightTileModel;
57-
5845
export interface CallLayoutOutputs<Model> {
5946
/**
6047
* Whether the scrolling layer of the layout should appear on top.
@@ -63,11 +50,11 @@ export interface CallLayoutOutputs<Model> {
6350
/**
6451
* The visually fixed (non-scrolling) layer of the layout.
6552
*/
66-
fixed: ComponentType<LayoutProps<Model, TileModel, HTMLDivElement>>;
53+
fixed: ComponentType<LayoutProps<Model, TileViewModel, HTMLDivElement>>;
6754
/**
6855
* The layer of the layout that can overflow and be scrolled.
6956
*/
70-
scrolling: ComponentType<LayoutProps<Model, TileModel, HTMLDivElement>>;
57+
scrolling: ComponentType<LayoutProps<Model, TileViewModel, HTMLDivElement>>;
7158
}
7259

7360
/**

src/grid/Grid.tsx

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
createContext,
2525
forwardRef,
2626
memo,
27+
useCallback,
2728
useContext,
2829
useEffect,
2930
useMemo,
@@ -33,6 +34,8 @@ import {
3334
import useMeasure from "react-use-measure";
3435
import classNames from "classnames";
3536
import { logger } from "matrix-js-sdk/src/logger";
37+
import { useObservableEagerState } from "observable-hooks";
38+
import { fromEvent, map, startWith } from "rxjs";
3639

3740
import styles from "./Grid.module.css";
3841
import { useMergedRefs } from "../useMergedRefs";
@@ -51,6 +54,7 @@ interface Tile<Model> {
5154
id: string;
5255
model: Model;
5356
onDrag: DragCallback | undefined;
57+
setVisible: (visible: boolean) => void;
5458
}
5559

5660
type PlacedTile<Model> = Tile<Model> & Rect;
@@ -84,6 +88,7 @@ interface SlotProps<Model> extends Omit<ComponentProps<"div">, "onDrag"> {
8488
id: string;
8589
model: Model;
8690
onDrag?: DragCallback;
91+
onVisibilityChange?: (visible: boolean) => void;
8792
style?: CSSProperties;
8893
className?: string;
8994
}
@@ -131,6 +136,11 @@ export function useUpdateLayout(): void {
131136
);
132137
}
133138

139+
const windowHeightObservable = fromEvent(window, "resize").pipe(
140+
startWith(null),
141+
map(() => window.innerHeight),
142+
);
143+
134144
export interface LayoutProps<LayoutModel, TileModel, R extends HTMLElement> {
135145
ref: LegacyRef<R>;
136146
model: LayoutModel;
@@ -232,19 +242,42 @@ export function Grid<
232242
const [gridRoot, gridRef2] = useState<HTMLElement | null>(null);
233243
const gridRef = useMergedRefs<HTMLElement>(gridRef1, gridRef2);
234244

245+
const windowHeight = useObservableEagerState(windowHeightObservable);
235246
const [layoutRoot, setLayoutRoot] = useState<HTMLElement | null>(null);
236247
const [generation, setGeneration] = useState<number | null>(null);
237248
const tiles = useInitial(() => new Map<string, Tile<TileModel>>());
238249
const prefersReducedMotion = usePrefersReducedMotion();
239250

240251
const Slot: FC<SlotProps<TileModel>> = useMemo(
241252
() =>
242-
function Slot({ id, model, onDrag, style, className, ...props }) {
253+
function Slot({
254+
id,
255+
model,
256+
onDrag,
257+
onVisibilityChange,
258+
style,
259+
className,
260+
...props
261+
}) {
243262
const ref = useRef<HTMLDivElement | null>(null);
263+
const prevVisible = useRef<boolean | null>(null);
264+
const setVisible = useCallback(
265+
(visible: boolean) => {
266+
if (
267+
onVisibilityChange !== undefined &&
268+
visible !== prevVisible.current
269+
) {
270+
onVisibilityChange(visible);
271+
prevVisible.current = visible;
272+
}
273+
},
274+
[onVisibilityChange],
275+
);
276+
244277
useEffect(() => {
245-
tiles.set(id, { id, model, onDrag });
278+
tiles.set(id, { id, model, onDrag, setVisible });
246279
return (): void => void tiles.delete(id);
247-
}, [id, model, onDrag]);
280+
}, [id, model, onDrag, setVisible]);
248281

249282
return (
250283
<div
@@ -302,6 +335,17 @@ export function Grid<
302335
// eslint-disable-next-line react-hooks/exhaustive-deps
303336
}, [gridRoot, layoutRoot, tiles, gridBounds, generation]);
304337

338+
// The height of the portion of the grid visible at any given time
339+
const visibleHeight = useMemo(
340+
() => Math.min(gridBounds.bottom, windowHeight) - gridBounds.top,
341+
[gridBounds, windowHeight],
342+
);
343+
344+
useEffect(() => {
345+
for (const tile of placedTiles)
346+
tile.setVisible(tile.y + tile.height <= visibleHeight);
347+
}, [placedTiles, visibleHeight]);
348+
305349
// Drag state is stored in a ref rather than component state, because we use
306350
// react-spring's imperative API during gestures to improve responsiveness
307351
const dragState = useRef<DragState | null>(null);

src/grid/GridLayout.tsx

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,7 @@ import { useObservableEagerState } from "observable-hooks";
1212
import { GridLayout as GridLayoutModel } from "../state/CallViewModel";
1313
import styles from "./GridLayout.module.css";
1414
import { useInitial } from "../useInitial";
15-
import {
16-
CallLayout,
17-
GridTileModel,
18-
TileModel,
19-
arrangeTiles,
20-
} from "./CallLayout";
15+
import { CallLayout, arrangeTiles } from "./CallLayout";
2116
import { DragCallback, useUpdateLayout } from "./Grid";
2217

2318
interface GridCSSProperties extends CSSProperties {
@@ -49,15 +44,6 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
4944
),
5045
),
5146
);
52-
const tileModel: TileModel | undefined = useMemo(
53-
() =>
54-
model.spotlight && {
55-
type: "spotlight",
56-
vms: model.spotlight,
57-
maximised: false,
58-
},
59-
[model.spotlight],
60-
);
6147

6248
const onDragSpotlight: DragCallback = useCallback(
6349
({ xRatio, yRatio }) =>
@@ -70,11 +56,11 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
7056

7157
return (
7258
<div ref={ref} className={styles.fixed}>
73-
{tileModel && (
59+
{model.spotlight && (
7460
<Slot
7561
className={styles.slot}
7662
id="spotlight"
77-
model={tileModel}
63+
model={model.spotlight}
7864
onDrag={onDragSpotlight}
7965
data-block-alignment={alignment.block}
8066
data-inline-alignment={alignment.inline}
@@ -93,11 +79,6 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
9379
[width, minHeight, model.grid.length],
9480
);
9581

96-
const tileModels: GridTileModel[] = useMemo(
97-
() => model.grid.map((vm) => ({ type: "grid", vm })),
98-
[model.grid],
99-
);
100-
10182
return (
10283
<div
10384
ref={ref}
@@ -111,8 +92,14 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
11192
} as GridCSSProperties
11293
}
11394
>
114-
{tileModels.map((m) => (
115-
<Slot key={m.vm.id} className={styles.slot} id={m.vm.id} model={m} />
95+
{model.grid.map((m) => (
96+
<Slot
97+
key={m.id}
98+
className={styles.slot}
99+
id={m.id}
100+
model={m}
101+
onVisibilityChange={m.setVisible}
102+
/>
116103
))}
117104
</div>
118105
);

src/grid/OneOnOneLayout.tsx

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { useObservableEagerState } from "observable-hooks";
1010
import classNames from "classnames";
1111

1212
import { OneOnOneLayout as OneOnOneLayoutModel } from "../state/CallViewModel";
13-
import { CallLayout, GridTileModel, arrangeTiles } from "./CallLayout";
13+
import { CallLayout, arrangeTiles } from "./CallLayout";
1414
import styles from "./OneOnOneLayout.module.css";
1515
import { DragCallback, useUpdateLayout } from "./Grid";
1616

@@ -38,15 +38,6 @@ export const makeOneOnOneLayout: CallLayout<OneOnOneLayoutModel> = ({
3838
[width, height],
3939
);
4040

41-
const remoteTileModel: GridTileModel = useMemo(
42-
() => ({ type: "grid", vm: model.remote }),
43-
[model.remote],
44-
);
45-
const localTileModel: GridTileModel = useMemo(
46-
() => ({ type: "grid", vm: model.local }),
47-
[model.local],
48-
);
49-
5041
const onDragLocalTile: DragCallback = useCallback(
5142
({ xRatio, yRatio }) =>
5243
pipAlignment.next({
@@ -59,16 +50,18 @@ export const makeOneOnOneLayout: CallLayout<OneOnOneLayoutModel> = ({
5950
return (
6051
<div ref={ref} className={styles.layer}>
6152
<Slot
62-
id={remoteTileModel.vm.id}
63-
model={remoteTileModel}
53+
id={model.remote.id}
54+
model={model.remote}
55+
onVisibilityChange={model.remote.setVisible}
6456
className={styles.container}
6557
style={{ width: tileWidth, height: tileHeight }}
6658
>
6759
<Slot
6860
className={classNames(styles.slot, styles.local)}
69-
id={localTileModel.vm.id}
70-
model={localTileModel}
61+
id={model.local.id}
62+
model={model.local}
7163
onDrag={onDragLocalTile}
64+
onVisibilityChange={model.local.setVisible}
7265
data-block-alignment={pipAlignmentValue.block}
7366
data-inline-alignment={pipAlignmentValue.inline}
7467
/>

src/grid/SpotlightExpandedLayout.tsx

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ SPDX-License-Identifier: AGPL-3.0-only
55
Please see LICENSE in the repository root for full details.
66
*/
77

8-
import { forwardRef, useCallback, useMemo } from "react";
8+
import { forwardRef, useCallback } from "react";
99
import { useObservableEagerState } from "observable-hooks";
1010

1111
import { SpotlightExpandedLayout as SpotlightExpandedLayoutModel } from "../state/CallViewModel";
12-
import { CallLayout, GridTileModel, SpotlightTileModel } from "./CallLayout";
12+
import { CallLayout } from "./CallLayout";
1313
import { DragCallback, useUpdateLayout } from "./Grid";
1414
import styles from "./SpotlightExpandedLayout.module.css";
1515

@@ -27,17 +27,13 @@ export const makeSpotlightExpandedLayout: CallLayout<
2727
ref,
2828
) {
2929
useUpdateLayout();
30-
const spotlightTileModel: SpotlightTileModel = useMemo(
31-
() => ({ type: "spotlight", vms: model.spotlight, maximised: true }),
32-
[model.spotlight],
33-
);
3430

3531
return (
3632
<div ref={ref} className={styles.layer}>
3733
<Slot
3834
className={styles.spotlight}
3935
id="spotlight"
40-
model={spotlightTileModel}
36+
model={model.spotlight}
4137
/>
4238
</div>
4339
);
@@ -50,11 +46,6 @@ export const makeSpotlightExpandedLayout: CallLayout<
5046
useUpdateLayout();
5147
const pipAlignmentValue = useObservableEagerState(pipAlignment);
5248

53-
const pipTileModel: GridTileModel | undefined = useMemo(
54-
() => model.pip && { type: "grid", vm: model.pip },
55-
[model.pip],
56-
);
57-
5849
const onDragPip: DragCallback = useCallback(
5950
({ xRatio, yRatio }) =>
6051
pipAlignment.next({
@@ -66,12 +57,13 @@ export const makeSpotlightExpandedLayout: CallLayout<
6657

6758
return (
6859
<div ref={ref} className={styles.layer}>
69-
{pipTileModel && (
60+
{model.pip && (
7061
<Slot
7162
className={styles.pip}
72-
id="pip"
73-
model={pipTileModel}
63+
id={model.pip.id}
64+
model={model.pip}
7465
onDrag={onDragPip}
66+
onVisibilityChange={model.pip.setVisible}
7567
data-block-alignment={pipAlignmentValue.block}
7668
data-inline-alignment={pipAlignmentValue.inline}
7769
/>

0 commit comments

Comments
 (0)