From 0bfec65405e7ddfdf45e7139e2154128caf6ae53 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 25 Jul 2024 17:51:00 -0400 Subject: [PATCH 1/4] Refactor layout selection into smaller chunks --- src/state/CallViewModel.ts | 118 +++++++++++++++++++------------------ 1 file changed, 62 insertions(+), 56 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index c5d7af81..764b4d46 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -561,76 +561,84 @@ export class CallViewModel extends ViewModel { this.gridModeUserSelection.next(value); } + private readonly oneOnOne: Observable = combineLatest( + [this.grid, this.screenShares], + (grid, screenShares) => + grid.length == 2 && + // There might not be a remote tile if only the local user is in the call + // and they're using the duplicate tiles option + grid.some((vm) => !vm.local) && + screenShares.length === 0, + ); + + private readonly gridLayout: Observable = combineLatest( + [this.grid, this.spotlight], + (grid, spotlight) => ({ + type: "grid", + spotlight: spotlight.some((vm) => vm instanceof ScreenShareViewModel) + ? spotlight + : undefined, + grid, + }), + ); + + private readonly spotlightLandscapeLayout: Observable = combineLatest( + [this.grid, this.spotlight], + (grid, spotlight) => ({ type: "spotlight-landscape", spotlight, grid }), + ); + + private readonly spotlightPortraitLayout: Observable = combineLatest( + [this.grid, this.spotlight], + (grid, spotlight) => ({ type: "spotlight-portrait", spotlight, grid }), + ); + + private readonly spotlightExpandedLayout: Observable = combineLatest( + [this.spotlight, this.pip], + (spotlight, pip) => ({ + type: "spotlight-expanded", + spotlight, + pip: pip ?? undefined, + }), + ); + + private readonly oneOnOneLayout: Observable = this.grid.pipe( + map((grid) => ({ + type: "one-on-one", + local: grid.find((vm) => vm.local) as LocalUserMediaViewModel, + remote: grid.find((vm) => !vm.local) as RemoteUserMediaViewModel, + })), + ); + + private readonly pipLayout: Observable = this.spotlight.pipe( + map((spotlight): Layout => ({ type: "pip", spotlight })), + ); + public readonly layout: Observable = this.windowMode.pipe( switchMap((windowMode) => { - const spotlightLandscapeLayout = combineLatest( - [this.grid, this.spotlight], - (grid, spotlight): Layout => ({ - type: "spotlight-landscape", - spotlight, - grid, - }), - ); - const spotlightExpandedLayout = combineLatest( - [this.spotlight, this.pip], - (spotlight, pip): Layout => ({ - type: "spotlight-expanded", - spotlight, - pip: pip ?? undefined, - }), - ); - switch (windowMode) { case "normal": return this.gridMode.pipe( switchMap((gridMode) => { switch (gridMode) { case "grid": - return combineLatest( - [this.grid, this.spotlight, this.screenShares], - (grid, spotlight, screenShares): Layout => - grid.length == 2 && - // There might not be a remote tile if only the local user - // is in the call and they're using the duplicate tiles - // option - grid.some((vm) => !vm.local) && - screenShares.length === 0 - ? { - type: "one-on-one", - local: grid.find( - (vm) => vm.local, - ) as LocalUserMediaViewModel, - remote: grid.find( - (vm) => !vm.local, - ) as RemoteUserMediaViewModel, - } - : { - type: "grid", - spotlight: - screenShares.length > 0 ? spotlight : undefined, - grid, - }, + return this.oneOnOne.pipe( + switchMap((oneOnOne) => + oneOnOne ? this.oneOnOneLayout : this.gridLayout, + ), ); case "spotlight": return this.spotlightExpanded.pipe( switchMap((expanded) => expanded - ? spotlightExpandedLayout - : spotlightLandscapeLayout, + ? this.spotlightExpandedLayout + : this.spotlightLandscapeLayout, ), ); } }), ); case "narrow": - return combineLatest( - [this.grid, this.spotlight], - (grid, spotlight): Layout => ({ - type: "spotlight-portrait", - spotlight, - grid, - }), - ); + return this.spotlightPortraitLayout; case "flat": return this.gridMode.pipe( switchMap((gridMode) => { @@ -638,16 +646,14 @@ export class CallViewModel extends ViewModel { case "grid": // Yes, grid mode actually gets you a "spotlight" layout in // this window mode. - return spotlightLandscapeLayout; + return this.spotlightLandscapeLayout; case "spotlight": - return spotlightExpandedLayout; + return this.spotlightExpandedLayout; } }), ); case "pip": - return this.spotlight.pipe( - map((spotlight): Layout => ({ type: "pip", spotlight })), - ); + return this.pipLayout; } }), shareReplay(1), From 942e28f3c28d83877780f1e31d353e297559ab1f Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 25 Jul 2024 17:52:23 -0400 Subject: [PATCH 2/4] Improve the layouts on small mobile calls Due to an oversight of mine, 244003763953db033b5e5350142b3f00b7c8910a actually removed the ability to see the one-on-one layout on mobile. This restores mobile one-on-one calls to working order and also avoids showing the spotlight tile unless there are more than a few participants. --- src/state/CallViewModel.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 764b4d46..982df404 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -46,6 +46,7 @@ import { shareReplay, skip, startWith, + switchAll, switchMap, throttleTime, timer, @@ -75,6 +76,10 @@ import { duplicateTiles } from "../settings/settings"; // list again const POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS = 3000; +// This is the number of participants that we think constitutes a "small" call +// on mobile. No spotlight tile should be shown below this threshold. +const smallMobileCallThreshold = 3; + export interface GridLayout { type: "grid"; spotlight?: MediaViewModel[]; @@ -638,7 +643,20 @@ export class CallViewModel extends ViewModel { }), ); case "narrow": - return this.spotlightPortraitLayout; + return this.oneOnOne.pipe( + switchMap((oneOnOne) => + oneOnOne + ? this.oneOnOneLayout + : combineLatest( + [this.grid, this.spotlight], + (grid, spotlight) => + grid.length > smallMobileCallThreshold || + spotlight.some((vm) => vm instanceof ScreenShareViewModel) + ? this.spotlightPortraitLayout + : this.gridLayout, + ).pipe(switchAll()), + ), + ); case "flat": return this.gridMode.pipe( switchMap((gridMode) => { From eb051ab3185e8b5999d0dfdd77a45c2201ddda5e Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 1 Aug 2024 13:49:09 -0400 Subject: [PATCH 3/4] Replace the mobile one-on-one layout with an edge-to-edge spotlight --- src/grid/CallLayout.ts | 10 +---- src/grid/OneOnOneLayout.module.css | 11 +----- src/grid/SpotlightExpandedLayout.module.css | 11 +++++- src/room/InCallView.tsx | 2 +- src/state/CallViewModel.ts | 41 +++++++++++---------- yarn.lock | 6 +-- 6 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/grid/CallLayout.ts b/src/grid/CallLayout.ts index 119ed956..e1cc3117 100644 --- a/src/grid/CallLayout.ts +++ b/src/grid/CallLayout.ts @@ -95,7 +95,6 @@ export interface GridArrangement { const tileMaxAspectRatio = 17 / 9; const tileMinAspectRatio = 4 / 3; -const tileMobileMinAspectRatio = 2 / 3; /** * Determine the ideal arrangement of tiles into a grid of a particular size. @@ -138,15 +137,10 @@ export function arrangeTiles( // Impose a minimum and maximum aspect ratio on the tiles const tileAspectRatio = tileWidth / tileHeight; - // We enforce a different min aspect ratio in 1:1s on mobile - const minAspectRatio = - tileCount === 1 && width < 600 - ? tileMobileMinAspectRatio - : tileMinAspectRatio; if (tileAspectRatio > tileMaxAspectRatio) tileWidth = tileHeight * tileMaxAspectRatio; - else if (tileAspectRatio < minAspectRatio) - tileHeight = tileWidth / minAspectRatio; + else if (tileAspectRatio < tileMinAspectRatio) + tileHeight = tileWidth / tileMinAspectRatio; return { tileWidth, tileHeight, gap, columns }; } diff --git a/src/grid/OneOnOneLayout.module.css b/src/grid/OneOnOneLayout.module.css index 0c22b253..5bdeb2c8 100644 --- a/src/grid/OneOnOneLayout.module.css +++ b/src/grid/OneOnOneLayout.module.css @@ -26,18 +26,11 @@ limitations under the License. .local { position: absolute; - inline-size: 135px; - block-size: 160px; + inline-size: 180px; + block-size: 135px; inset: var(--cpd-space-4x); } -@media (min-width: 600px) { - .local { - inline-size: 170px; - block-size: 110px; - } -} - .spotlight { position: absolute; inline-size: 404px; diff --git a/src/grid/SpotlightExpandedLayout.module.css b/src/grid/SpotlightExpandedLayout.module.css index 6556110e..bf30dadb 100644 --- a/src/grid/SpotlightExpandedLayout.module.css +++ b/src/grid/SpotlightExpandedLayout.module.css @@ -25,11 +25,18 @@ limitations under the License. .pip { position: absolute; - inline-size: 180px; - block-size: 135px; + inline-size: 135px; + block-size: 160px; inset: var(--cpd-space-4x); } +@media (min-width: 600px) { + .pip { + inline-size: 180px; + block-size: 135px; + } +} + .pip[data-block-alignment="start"] { inset-block-end: unset; } diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index c595f173..d28e9539 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -295,7 +295,7 @@ export const InCallView: FC = ({ ref, ) { const spotlightExpanded = useObservableEagerState(vm.spotlightExpanded); - const [onToggleExpanded] = useObservableEagerState( + const onToggleExpanded = useObservableEagerState( vm.toggleSpotlightExpanded, ); const showSpeakingIndicatorsValue = useObservableEagerState( diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 982df404..a4db2232 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -646,7 +646,9 @@ export class CallViewModel extends ViewModel { return this.oneOnOne.pipe( switchMap((oneOnOne) => oneOnOne - ? this.oneOnOneLayout + ? // The expanded spotlight layout makes for a better one-on-one + // experience in narrow windows + this.spotlightExpandedLayout : combineLatest( [this.grid, this.spotlight], (grid, spotlight) => @@ -689,24 +691,25 @@ export class CallViewModel extends ViewModel { shareReplay(1), ); - // To work around https://github.com/crimx/observable-hooks/issues/131 we must - // wrap the emitted function here in a non-function wrapper type - public readonly toggleSpotlightExpanded: Observable< - readonly [(() => void) | null] - > = this.layout.pipe( - map( - (l) => - l.type === "spotlight-landscape" || l.type === "spotlight-expanded", - ), - distinctUntilChanged(), - map( - (enabled) => - [ - enabled ? (): void => this.spotlightExpandedToggle.next() : null, - ] as const, - ), - shareReplay(1), - ); + public readonly toggleSpotlightExpanded: Observable<(() => void) | null> = + this.windowMode.pipe( + switchMap((mode) => + mode === "normal" + ? this.layout.pipe( + map( + (l) => + l.type === "spotlight-landscape" || + l.type === "spotlight-expanded", + ), + ) + : of(false), + ), + distinctUntilChanged(), + map((enabled) => + enabled ? (): void => this.spotlightExpandedToggle.next() : null, + ), + shareReplay(1), + ); public constructor( // A call is permanently tied to a single Matrix room and LiveKit room diff --git a/yarn.lock b/yarn.lock index 857766cd..3181e4b5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6944,9 +6944,9 @@ object.values@^1.1.7: es-abstract "^1.22.1" observable-hooks@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/observable-hooks/-/observable-hooks-4.2.3.tgz#69e3353caafd7887ad9030bd440b053304e8d2d1" - integrity sha512-d6fYTIU+9sg1V+CT0GhgoE/ntjIqcy9DGaYGE6ELGVP4ojaWIEsaLvL/05hLOM+AL7aySN4DCTLvj6dDF9T8XA== + version "4.2.4" + resolved "https://registry.yarnpkg.com/observable-hooks/-/observable-hooks-4.2.4.tgz#e1ee0f867e0f2216f79c1e13c58716fb50b410ec" + integrity sha512-FdTQgyw1h5bG/QHCBIqctdBSnv9VARJCEilgpV6L2qlw1yeLqFIwPm4U15dMtl5kDmNN0hSt+Nl6iYbLFwEcQA== oidc-client-ts@^3.0.1: version "3.0.1" From f4cf3d8c62bc5e0a22fc297aa8104bc69428a32c Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 6 Aug 2024 10:08:56 -0400 Subject: [PATCH 4/4] Adjust the breakpoint --- src/state/CallViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a4db2232..dcbb93cf 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -520,7 +520,7 @@ export class CallViewModel extends ViewModel { const height = window.innerHeight; const width = window.innerWidth; if (height <= 400 && width <= 340) return "pip"; - if (width <= 660) return "narrow"; + if (width <= 600) return "narrow"; if (height <= 660) return "flat"; return "normal"; }),