From 3a706ea3e0b8db4e22b4d9ee91dc57d896dca8d1 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 28 Oct 2024 14:45:06 -0400 Subject: [PATCH 1/3] Show speaking indicators in spotlight during screen sharing --- src/state/CallViewModel.test.ts | 7 +++++++ src/state/CallViewModel.ts | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 89f95b92..01dfad4c 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -221,6 +221,9 @@ test("screen sharing activates spotlight layout", () => { // sharing, then return to grid, then manually go into spotlight, and // remain in spotlight until we manually go back to grid const laytMarbles = "ab(cc)(dd)ae(bb)(ee)a 59979ms a"; + // Speaking indicators should always be shown except for when the active + // speaker is present in the spotlight + const showMarbles = "y----------ny---n---y"; withCallViewModel( helpers, @@ -270,6 +273,10 @@ test("screen sharing activates spotlight layout", () => { }, }, ); + expectObservable(vm.showSpeakingIndicators).toBe(showMarbles, { + y: true, + n: false, + }); }, ); }); diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 00e10dfe..a9893f43 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -714,7 +714,21 @@ export class CallViewModel extends ViewModel { } public showSpeakingIndicators: Observable = this.layout.pipe( - map((l) => l.type !== "one-on-one" && !l.type.startsWith("spotlight-")), + map((l) => { + switch (l.type) { + case "spotlight-landscape": + case "spotlight-portrait": + // If the spotlight is showing the active speaker, we can do without + // speaking indicators as they're a redundant visual cue. But if + // screen sharing feeds are in the spotlight we still need them. + return l.spotlight[0] instanceof ScreenShareViewModel; + case "spotlight-expanded": + case "one-on-one": + return false; + default: + return true; + } + }), this.scope.state(), ); From 42be187182ac1e15e0760d47a698c17457f87c06 Mon Sep 17 00:00:00 2001 From: Robin Date: Fri, 1 Nov 2024 11:25:55 -0400 Subject: [PATCH 2/3] Explain why speaking indicators are hidden --- src/state/CallViewModel.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a9893f43..c668e863 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -722,7 +722,11 @@ export class CallViewModel extends ViewModel { // speaking indicators as they're a redundant visual cue. But if // screen sharing feeds are in the spotlight we still need them. return l.spotlight[0] instanceof ScreenShareViewModel; + // In expanded spotlight layout, the active speaker is always shown in + // the picture-in-picture tile so there is no need for speaking + // indicators case "spotlight-expanded": + // In one-on-one layout there's no question as to who is speaking case "one-on-one": return false; default: From b903e11cfc8283c27c8f423d1acc1bc0c795e075 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 4 Nov 2024 10:56:29 -0500 Subject: [PATCH 3/3] Fix lint error --- src/state/CallViewModel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index c668e863..7a4048f6 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -724,9 +724,9 @@ export class CallViewModel extends ViewModel { return l.spotlight[0] instanceof ScreenShareViewModel; // In expanded spotlight layout, the active speaker is always shown in // the picture-in-picture tile so there is no need for speaking - // indicators + // indicators. And in one-on-one layout there's no question as to who is + // speaking. case "spotlight-expanded": - // In one-on-one layout there's no question as to who is speaking case "one-on-one": return false; default: