From fdc6d4a1b6245f12c59118167f025f2f63b526d1 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 8 May 2024 16:00:42 -0400 Subject: [PATCH 1/6] Add a developer option to duplicate tiles This is useful for testing how the UI behaves with different numbers of participants. --- public/locales/en-GB/app.json | 1 + src/settings/SettingsModal.tsx | 18 ++++++++++++++- src/settings/settings.ts | 2 ++ src/state/CallViewModel.ts | 41 +++++++++++++++++++++++----------- 4 files changed, 48 insertions(+), 14 deletions(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 4279bbb5..97f2be93 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -132,6 +132,7 @@ "developer_settings_label": "Developer Settings", "developer_settings_label_description": "Expose developer settings in the settings window.", "developer_tab_title": "Developer", + "duplicate_tiles_label": "Number of duplicate tiles", "feedback_tab_body": "If you are experiencing issues or simply would like to provide some feedback, please send us a short description below.", "feedback_tab_description_label": "Your feedback", "feedback_tab_h4": "Submit feedback", diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index fd0b6295..d8430022 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { ChangeEvent, FC, Key, ReactNode } from "react"; +import { ChangeEvent, FC, Key, ReactNode, useCallback } from "react"; import { Item } from "@react-stately/collections"; import { Trans, useTranslation } from "react-i18next"; import { MatrixClient } from "matrix-js-sdk"; @@ -44,6 +44,7 @@ import { useSetting, optInAnalytics as optInAnalyticsSetting, developerSettingsTab as developerSettingsTabSetting, + duplicateTiles as duplicateTilesSetting, } from "./settings"; import { isFirefox } from "../Platform"; @@ -80,6 +81,7 @@ export const SettingsModal: FC = ({ const [developerSettingsTab, setDeveloperSettingsTab] = useSetting( developerSettingsTabSetting, ); + const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); // Generate a `SelectInput` with a list of devices for a given device kind. const generateDeviceSelection = ( @@ -244,6 +246,20 @@ export const SettingsModal: FC = ({ })} + + ): void => { + setDuplicateTiles(event.target.valueAsNumber); + }, + [setDuplicateTiles], + )} + /> + ); diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 307a557e..5f2cc529 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -76,6 +76,8 @@ export const developerSettingsTab = new Setting( false, ); +export const duplicateTiles = new Setting("duplicate-tiles", 0); + export const audioInput = new Setting( "audio-input", undefined, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index e6d55b72..59f0f5dc 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -67,6 +67,7 @@ import { } from "./MediaViewModel"; import { finalizeValue } from "../observable-utils"; import { ObservableScope } from "./ObservableScope"; +import { duplicateTiles } from "../settings/settings"; // How long we wait after a focus switch before showing the real participant // list again @@ -308,11 +309,16 @@ export class CallViewModel extends ViewModel { combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), + duplicateTiles.value, ]).pipe( scan( ( prevItems, - [remoteParticipants, { participant: localParticipant }], + [ + remoteParticipants, + { participant: localParticipant }, + duplicateTiles, + ], ) => { let allGhosts = true; @@ -330,20 +336,29 @@ export class CallViewModel extends ViewModel { ); } - const userMediaId = p.identity; - yield [ - userMediaId, - prevItems.get(userMediaId) ?? - new UserMedia(userMediaId, member, p, this.encrypted), - ]; - - if (p.isScreenShareEnabled) { - const screenShareId = `${userMediaId}:screen-share`; + // Create as many tiles for this participant as called for by + // the duplicateTiles option + for (let i = 0; i < 1 + duplicateTiles; i++) { + const userMediaId = `${p.identity}:${i}`; yield [ - screenShareId, - prevItems.get(screenShareId) ?? - new ScreenShare(screenShareId, member, p, this.encrypted), + userMediaId, + prevItems.get(userMediaId) ?? + new UserMedia(userMediaId, member, p, this.encrypted), ]; + + if (p.isScreenShareEnabled) { + const screenShareId = `${userMediaId}:screen-share`; + yield [ + screenShareId, + prevItems.get(screenShareId) ?? + new ScreenShare( + screenShareId, + member, + p, + this.encrypted, + ), + ]; + } } } }.bind(this)(), From 8c21e8f277915c3c38d38fdafbb9ae43b3db343e Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 17 Jul 2024 14:55:45 -0400 Subject: [PATCH 2/6] Use a more descriptive string --- public/locales/en-GB/app.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 97f2be93..95bf334f 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -132,7 +132,7 @@ "developer_settings_label": "Developer Settings", "developer_settings_label_description": "Expose developer settings in the settings window.", "developer_tab_title": "Developer", - "duplicate_tiles_label": "Number of duplicate tiles", + "duplicate_tiles_label": "Number of additional tile copies per participant", "feedback_tab_body": "If you are experiencing issues or simply would like to provide some feedback, please send us a short description below.", "feedback_tab_description_label": "Your feedback", "feedback_tab_h4": "Submit feedback", From a59875dab5936ed606b0533c486712de8b86d989 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 17 Jul 2024 15:37:41 -0400 Subject: [PATCH 3/6] Explain what each sorting bin means --- src/state/CallViewModel.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index f2656166..74061abe 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -131,13 +131,37 @@ export type WindowMode = "normal" | "full screen" | "pip"; * Sorting bins defining the order in which media tiles appear in the layout. */ enum SortingBin { + /** + * Yourself, when the "always show self" option is on. + */ SelfAlwaysShown, + /** + * Participants that are sharing their screen. + */ Presenters, + /** + * Participants that have been speaking recently. + */ Speakers, + /** + * Participants with both video and audio. + */ VideoAndAudio, + /** + * Participants with video but no audio. + */ Video, + /** + * Participants with audio but no video. + */ Audio, + /** + * Participants not sharing any media. + */ NoMedia, + /** + * Yourself, when the "always show self" option is off. + */ SelfNotAlwaysShown, } From 2bc56dbff2efad277edbde1c80614842d0def8d5 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 17 Jul 2024 15:37:55 -0400 Subject: [PATCH 4/6] Use fewer ML-style variable names --- src/state/CallViewModel.ts | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 74061abe..083cf556 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -386,7 +386,7 @@ export class CallViewModel extends ViewModel { }, new Map(), ), - map((ms) => [...ms.values()]), + map((mediaItems) => [...mediaItems.values()]), finalizeValue((ts) => { for (const t of ts) t.destroy(); }), @@ -394,35 +394,41 @@ export class CallViewModel extends ViewModel { ); private readonly userMedia: Observable = this.mediaItems.pipe( - map((ms) => ms.filter((m): m is UserMedia => m instanceof UserMedia)), + map((mediaItems) => + mediaItems.filter((m): m is UserMedia => m instanceof UserMedia), + ), ); private readonly screenShares: Observable = this.mediaItems.pipe( - map((ms) => ms.filter((m): m is ScreenShare => m instanceof ScreenShare)), + map((mediaItems) => + mediaItems.filter((m): m is ScreenShare => m instanceof ScreenShare), + ), ); private readonly spotlightSpeaker: Observable = this.userMedia.pipe( - switchMap((ms) => - ms.length === 0 + switchMap((mediaItems) => + mediaItems.length === 0 ? of([]) : combineLatest( - ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), + mediaItems.map((m) => + m.vm.speaking.pipe(map((s) => [m, s] as const)), + ), ), ), scan<(readonly [UserMedia, boolean])[], UserMedia | null, null>( - (prev, ms) => + (prev, mediaItems) => // Decide who to spotlight: // If the previous speaker is still speaking, stick with them rather // than switching eagerly to someone else - ms.find(([m, s]) => m === prev && s)?.[0] ?? + mediaItems.find(([m, s]) => m === prev && s)?.[0] ?? // Otherwise, select anyone who is speaking - ms.find(([, s]) => s)?.[0] ?? + mediaItems.find(([, s]) => s)?.[0] ?? // Otherwise, stick with the person who was last speaking prev ?? // Otherwise, spotlight the local user - ms.find(([m]) => m.vm.local)?.[0] ?? + mediaItems.find(([m]) => m.vm.local)?.[0] ?? null, null, ), @@ -431,8 +437,8 @@ export class CallViewModel extends ViewModel { ); private readonly grid: Observable = this.userMedia.pipe( - switchMap((ms) => { - const bins = ms.map((m) => + switchMap((mediaItems) => { + const bins = mediaItems.map((m) => combineLatest( [ m.speaker, From 1efa594430d6d64d5507adbf22b3f2559d70b507 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 17 Jul 2024 16:06:48 -0400 Subject: [PATCH 5/6] Use Array.some where it's appropriate --- 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 31a1fdb3..62d71048 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -377,7 +377,7 @@ export class CallViewModel extends ViewModel { private readonly hasRemoteScreenShares: Observable = this.screenShares.pipe( - map((ms) => ms.find((m) => !m.vm.local) !== undefined), + map((ms) => ms.some((m) => !m.vm.local)), distinctUntilChanged(), ); From e04affe93e7e40a82fc47dcc321f95492d66dbb4 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 18 Jul 2024 10:00:26 -0400 Subject: [PATCH 6/6] Justify the use of a participant count threshold --- src/state/CallViewModel.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 3d48da22..c3d038df 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -74,6 +74,13 @@ import { ObservableScope } from "./ObservableScope"; // list again const POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS = 3000; +// This is the number of participants that we think constitutes a "large" grid. +// The hypothesis is that, after this many participants there's enough cognitive +// load that it makes sense to show the speaker in an easy-to-locate spotlight +// tile. We might change this to a scroll-based condition or do something else +// entirely with the spotlight tile, if we workshop this further. +const largeGridThreshold = 20; + // Represents something that should get a tile on the layout, // ie. a user's video feed or a screen share feed. // TODO: This exposes too much information to the view layer, let's keep this @@ -504,7 +511,8 @@ export class CallViewModel extends ViewModel { (grid, spotlight, screenShares): Layout => ({ type: "grid", spotlight: - screenShares.length > 0 || grid.length > 20 + screenShares.length > 0 || + grid.length > largeGridThreshold ? spotlight : undefined, grid,