Merge pull request #2514 from robintown/mobile-layouts

Improve the layouts on small mobile calls

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 };
 }

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;

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;
 }

src/room/InCallView.tsx

@@ -295,7 +295,7 @@ export const InCallView: FC<InCallViewProps> = ({
         ref,
       ) {
         const spotlightExpanded = useObservableEagerState(vm.spotlightExpanded);
-        const [onToggleExpanded] = useObservableEagerState(
+        const onToggleExpanded = useObservableEagerState(
           vm.toggleSpotlightExpanded,
         );
         const showSpeakingIndicatorsValue = useObservableEagerState(

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[];
@@ -515,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";
     }),
@@ -561,75 +566,98 @@ export class CallViewModel extends ViewModel {
     this.gridModeUserSelection.next(value);
   }
 
+  private readonly oneOnOne: Observable<boolean> = 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<Layout> = combineLatest(
+    [this.grid, this.spotlight],
+    (grid, spotlight) => ({
+      type: "grid",
+      spotlight: spotlight.some((vm) => vm instanceof ScreenShareViewModel)
+        ? spotlight
+        : undefined,
+      grid,
+    }),
+  );
+
+  private readonly spotlightLandscapeLayout: Observable<Layout> = combineLatest(
+    [this.grid, this.spotlight],
+    (grid, spotlight) => ({ type: "spotlight-landscape", spotlight, grid }),
+  );
+
+  private readonly spotlightPortraitLayout: Observable<Layout> = combineLatest(
+    [this.grid, this.spotlight],
+    (grid, spotlight) => ({ type: "spotlight-portrait", spotlight, grid }),
+  );
+
+  private readonly spotlightExpandedLayout: Observable<Layout> = combineLatest(
+    [this.spotlight, this.pip],
+    (spotlight, pip) => ({
+      type: "spotlight-expanded",
+      spotlight,
+      pip: pip ?? undefined,
+    }),
+  );
+
+  private readonly oneOnOneLayout: Observable<Layout> = 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<Layout> = this.spotlight.pipe(
+    map((spotlight): Layout => ({ type: "pip", spotlight })),
+  );
+
   public readonly layout: Observable<Layout> = 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.oneOnOne.pipe(
+            switchMap((oneOnOne) =>
+              oneOnOne
+                ? // The expanded spotlight layout makes for a better one-on-one
+                  // experience in narrow windows
+                  this.spotlightExpandedLayout
+                : 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(
@@ -638,16 +666,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),
@@ -665,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