Enable the PiP tile in expanded spotlight layout to swap speakers without a layout shift
This was apparently left unimplemented during the first iteration of the TileStore. It's a welcome UI optimization and we can reliably test for it.
This commit is contained in:
@@ -732,6 +732,84 @@ test("layout enters picture-in-picture mode when requested", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("PiP tile in expanded spotlight layout switches speakers without layout shifts", () => {
|
||||||
|
withTestScheduler(({ behavior, schedule, expectObservable }) => {
|
||||||
|
// Switch to spotlight immediately
|
||||||
|
const modeInputMarbles = " s";
|
||||||
|
// And expand the spotlight immediately
|
||||||
|
const expandInputMarbles = " a";
|
||||||
|
// First Bob speaks, then Dave, then Bob again
|
||||||
|
const bSpeakingInputMarbles = "n-yn--yn";
|
||||||
|
const dSpeakingInputMarbles = "n---yn";
|
||||||
|
// Should show Alice (presenter) in the PiP, then Bob, then Dave, then Bob
|
||||||
|
// again
|
||||||
|
const expectedLayoutMarbles = "a-b-c-b";
|
||||||
|
|
||||||
|
withCallViewModel(
|
||||||
|
{
|
||||||
|
remoteParticipants$: constant([
|
||||||
|
aliceSharingScreen,
|
||||||
|
bobParticipant,
|
||||||
|
daveParticipant,
|
||||||
|
]),
|
||||||
|
rtcMembers$: constant([
|
||||||
|
localRtcMember,
|
||||||
|
aliceRtcMember,
|
||||||
|
bobRtcMember,
|
||||||
|
daveRtcMember,
|
||||||
|
]),
|
||||||
|
speaking: new Map([
|
||||||
|
[bobParticipant, behavior(bSpeakingInputMarbles, yesNo)],
|
||||||
|
[daveParticipant, behavior(dSpeakingInputMarbles, yesNo)],
|
||||||
|
]),
|
||||||
|
},
|
||||||
|
(vm) => {
|
||||||
|
schedule(modeInputMarbles, {
|
||||||
|
s: () => vm.setGridMode("spotlight"),
|
||||||
|
});
|
||||||
|
schedule(expandInputMarbles, {
|
||||||
|
a: () => vm.toggleSpotlightExpanded$.value!(),
|
||||||
|
});
|
||||||
|
|
||||||
|
expectObservable(summarizeLayout$(vm.layout$)).toBe(
|
||||||
|
expectedLayoutMarbles,
|
||||||
|
{
|
||||||
|
a: {
|
||||||
|
type: "spotlight-expanded",
|
||||||
|
spotlight: [`${aliceId}:0:screen-share`],
|
||||||
|
pip: `${aliceId}:0`,
|
||||||
|
},
|
||||||
|
b: {
|
||||||
|
type: "spotlight-expanded",
|
||||||
|
spotlight: [`${aliceId}:0:screen-share`],
|
||||||
|
pip: `${bobId}:0`,
|
||||||
|
},
|
||||||
|
c: {
|
||||||
|
type: "spotlight-expanded",
|
||||||
|
spotlight: [`${aliceId}:0:screen-share`],
|
||||||
|
pip: `${daveId}:0`,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// While we expect the media on the PiP tile to change, layout$ itself
|
||||||
|
// should *never* meaningfully change. That is, we expect the same PiP
|
||||||
|
// tile to exist throughout the test and just have its media swapped out
|
||||||
|
// when the speaker changes, rather than for tiles to animate in/out.
|
||||||
|
// This is meaningful for keeping the interface not too visually
|
||||||
|
// distracting during back-and-forth conversations.
|
||||||
|
expectObservable(
|
||||||
|
vm.layout$.pipe(
|
||||||
|
distinctUntilChanged(deepCompare),
|
||||||
|
debounceTime(0),
|
||||||
|
map(() => "x"),
|
||||||
|
),
|
||||||
|
).toBe("x"); // Expect just one emission
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
test("spotlight remembers whether it's expanded", () => {
|
test("spotlight remembers whether it's expanded", () => {
|
||||||
withTestScheduler(({ schedule, expectObservable }) => {
|
withTestScheduler(({ schedule, expectObservable }) => {
|
||||||
// Start in spotlight mode, then switch to grid and back to spotlight a
|
// Start in spotlight mode, then switch to grid and back to spotlight a
|
||||||
@@ -754,11 +832,7 @@ test("spotlight remembers whether it's expanded", () => {
|
|||||||
g: () => vm.setGridMode("grid"),
|
g: () => vm.setGridMode("grid"),
|
||||||
});
|
});
|
||||||
schedule(expandInputMarbles, {
|
schedule(expandInputMarbles, {
|
||||||
a: () => {
|
a: () => vm.toggleSpotlightExpanded$.value!(),
|
||||||
let toggle: () => void;
|
|
||||||
vm.toggleSpotlightExpanded$.subscribe((val) => (toggle = val!));
|
|
||||||
toggle!();
|
|
||||||
},
|
|
||||||
});
|
});
|
||||||
|
|
||||||
expectObservable(summarizeLayout$(vm.layout$)).toBe(
|
expectObservable(summarizeLayout$(vm.layout$)).toBe(
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ export function spotlightExpandedLayout(
|
|||||||
): [SpotlightExpandedLayout, TileStore] {
|
): [SpotlightExpandedLayout, TileStore] {
|
||||||
const update = prevTiles.from(1);
|
const update = prevTiles.from(1);
|
||||||
update.registerSpotlight(media.spotlight, true);
|
update.registerSpotlight(media.spotlight, true);
|
||||||
if (media.pip !== undefined) update.registerGridTile(media.pip);
|
if (media.pip !== undefined) update.registerPipTile(media.pip);
|
||||||
const tiles = update.build();
|
const tiles = update.build();
|
||||||
|
|
||||||
return [
|
return [
|
||||||
|
|||||||
@@ -261,6 +261,33 @@ export class TileStoreBuilder {
|
|||||||
this.numGridEntries++;
|
this.numGridEntries++;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sets up a PiP tile for the given media. This is a special kind of grid tile
|
||||||
|
* that is expected to stand on its own and switch between speakers, so this
|
||||||
|
* method will more eagerly try to reuse an existing tile, replacing its
|
||||||
|
* media, than registerGridTile would.
|
||||||
|
*/
|
||||||
|
public registerPipTile(media: UserMediaViewModel): void {
|
||||||
|
if (DEBUG_ENABLED)
|
||||||
|
logger.debug(
|
||||||
|
`[TileStore, ${this.generation}] register PiP tile: ${media.member?.rawDisplayName ?? "[👻]"}`,
|
||||||
|
);
|
||||||
|
|
||||||
|
// If there is a single grid tile that we can reuse
|
||||||
|
if (this.prevGrid.length === 1) {
|
||||||
|
const entry = this.prevGrid[0];
|
||||||
|
this.stationaryGridEntries[0] = entry;
|
||||||
|
// Do the media swap
|
||||||
|
entry.media = media;
|
||||||
|
this.prevGridByMedia.delete(entry.media);
|
||||||
|
this.prevGridByMedia.set(media, [entry, 0]);
|
||||||
|
} else {
|
||||||
|
this.visibleGridEntries.push(new GridTileData(media));
|
||||||
|
}
|
||||||
|
|
||||||
|
this.numGridEntries++;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Constructs a new collection of all registered tiles, transferring ownership
|
* Constructs a new collection of all registered tiles, transferring ownership
|
||||||
* of the tiles to the new collection. Any tiles present in the previous
|
* of the tiles to the new collection. Any tiles present in the previous
|
||||||
|
|||||||
Reference in New Issue
Block a user