Skip to content

Commit

Permalink
Merge branch 'develop' into cpettet/vidcs-2954-rework-the-overflow
Browse files Browse the repository at this point in the history
  • Loading branch information
cpettet authored Mar 3, 2025
2 parents 20a9f7d + 2b22bfb commit d5b1e23
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ const VideoTileCanvas = ({
const isViewingScreenshare = subscriberWrappers.some((subWrapper) => subWrapper.isScreenshare);
const sessionHasScreenshare = isViewingScreenshare || isSharingScreen;
const isViewingLargeTile = sessionHasScreenshare || layoutMode === 'active-speaker';
const hasPinnedSubscribers = subscriberWrappers.some((subWrapper) => subWrapper.isPinned);
const pinnedSubscriberCount = subscriberWrappers.filter(
(subWrapper) => subWrapper.isPinned
).length;

// Check which subscribers we will display, in large calls we will hide some subscribers
const { hiddenSubscribers, subscribersOnScreen } = getSubscribersToDisplay(
Expand All @@ -65,7 +67,7 @@ const VideoTileCanvas = ({
const layoutBoxes = getLayoutBoxes({
activeSpeakerId,
getLayout,
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers,
isSharingScreen,
layoutMode,
Expand Down
7 changes: 1 addition & 6 deletions frontend/src/hooks/useLayoutManager.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { useRef } from 'react';
import { Box, Element } from 'opentok-layout-js';
import LayoutManager from '../utils/layoutManager';

export type GetLayout = (
containerDimensions: { height: number; width: number },
boxes: Element[]
) => Box[];

export type GetLayout = InstanceType<typeof LayoutManager>['getLayout'];
/**
* React hook to return a getLayout function as defined here:
* https://github.com/aullman/opentok-layout-js?tab=readme-ov-file#usage
Expand Down
52 changes: 50 additions & 2 deletions frontend/src/utils/helpers/getLayoutBoxes/getLayoutBoxes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const initialArguments: GetLayoutBoxesProps = {
activeSpeakerId: undefined,
hiddenSubscribers: [],
isSharingScreen: false,
hasPinnedSubscribers: false,
pinnedSubscriberCount: 0,
layoutMode: 'active-speaker',
publisher: null,
screensharingPublisher: null,
Expand All @@ -56,7 +56,7 @@ const initialArguments: GetLayoutBoxesProps = {
const typicalRoomArguments: GetLayoutBoxesProps = {
publisher: createPublisher(),
isSharingScreen: true,
hasPinnedSubscribers: false,
pinnedSubscriberCount: 0,
screensharingPublisher: createPublisher(),
wrapRef: { current: {} } as unknown as MutableRefObject<HTMLElement | null>,
subscribersInDisplayOrder: [
Expand Down Expand Up @@ -183,4 +183,52 @@ describe('getLayoutBoxes', () => {
subscriberBoxes: ['subscriber1Box', 'subscriber2Box', 'subscriber3Box'],
});
});

it('should call getLayout with shouldMakeLargeTilesLandscape flag true for multiple pinned participants with no screenshare', () => {
const getLayoutMock = vi.fn().mockReturnValue([]);
const args = {
...typicalRoomArguments,
sessionHasScreenshare: false,
getLayout: getLayoutMock,
pinnedSubscriberCount: 2,
};
getLayoutBoxes(args);
expect(getLayoutMock).toHaveBeenCalledWith(
typicalRoomArguments.wrapDimensions,
undefined,
true // shouldMakeLargeTilesLandscape
);
});

it('should call getLayout with shouldMakeLargeTilesLandscape flag false for multiple pinned participants with screenshare', () => {
const getLayoutMock = vi.fn().mockReturnValue([]);
const args = {
...typicalRoomArguments,
sessionHasScreenshare: true,
getLayout: getLayoutMock,
pinnedSubscriberCount: 2,
};
getLayoutBoxes(args);
expect(getLayoutMock).toHaveBeenCalledWith(
typicalRoomArguments.wrapDimensions,
undefined,
false // shouldMakeLargeTilesLandscape
);
});

it('should call getLayout with shouldMakeLargeTilesLandscape flag false for single pinned participants with no screenshare', () => {
const getLayoutMock = vi.fn().mockReturnValue([]);
const args = {
...typicalRoomArguments,
sessionHasScreenshare: false,
getLayout: getLayoutMock,
pinnedSubscriberCount: 1,
};
getLayoutBoxes(args);
expect(getLayoutMock).toHaveBeenCalledWith(
typicalRoomArguments.wrapDimensions,
undefined,
false // shouldMakeLargeTilesLandscape
);
});
});
10 changes: 9 additions & 1 deletion frontend/src/utils/helpers/getLayoutBoxes/getLayoutBoxes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,22 @@ const getLayoutBoxes = ({
if (!wrapRef.current) {
return {};
}
// For multiple pinned participants with no screenshare we make all large tiles landscape
const shouldMakeLargeTilesLandscape =
layoutProps.pinnedSubscriberCount > 1 && !layoutProps.sessionHasScreenshare;

// Boxes are returned at the same index as the layout Element passed in
// See: https://github.com/aullman/opentok-layout-js/?tab=readme-ov-file#usage
// So for us:
// index 0 - publisher box
// last index n - hidden tile participant if present
// last index n after popping hidden tile - local screenshare box
// remaining indices between 1 and n after popping screenshare - subscriber boxes in display order
const boxes = getLayout(wrapDimensions, getLayoutElementArray(layoutProps));
const boxes = getLayout(
wrapDimensions,
getLayoutElementArray(layoutProps),
shouldMakeLargeTilesLandscape
);
const publisherBox = boxes.shift();
const hiddenParticipantsBox = layoutProps.hiddenSubscribers.length ? boxes.pop() : undefined;
const localScreenshareBox = layoutProps.isSharingScreen ? boxes.pop() : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const hiddenParticipantTileLayoutElement = {

describe('getLayoutElementArray', () => {
let activeSpeakerId: string;
let hasPinnedSubscribers: boolean;
let pinnedSubscriberCount: number;
let hiddenSubscribers: SubscriberWrapper[];
let isSharingScreen: boolean;
let layoutMode: LayoutMode;
Expand All @@ -119,7 +119,7 @@ describe('getLayoutElementArray', () => {
let subscribersInDisplayOrder: SubscriberWrapper[];
beforeEach(() => {
activeSpeakerId = 'sub2';
hasPinnedSubscribers = false;
pinnedSubscriberCount = 0;
hiddenSubscribers = [subscriber4];
isSharingScreen = true;
layoutMode = 'active-speaker';
Expand All @@ -132,7 +132,7 @@ describe('getLayoutElementArray', () => {
it('returns elements in correct order', () => {
const layoutElements = getLayoutElementArray({
activeSpeakerId,
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers,
isSharingScreen,
layoutMode,
Expand All @@ -155,7 +155,7 @@ describe('getLayoutElementArray', () => {
it('makes active speaker element big if screenshare is not present', () => {
const layoutElements = getLayoutElementArray({
activeSpeakerId: 'sub1',
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers: [],
isSharingScreen: false,
layoutMode,
Expand All @@ -176,7 +176,7 @@ describe('getLayoutElementArray', () => {
it('does not make active speaker element big if layout-mode is grid', () => {
const layoutElements = getLayoutElementArray({
activeSpeakerId: 'sub1',
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers,
isSharingScreen: false,
layoutMode: 'grid',
Expand All @@ -198,7 +198,7 @@ describe('getLayoutElementArray', () => {
it('makes screenshare big and fixedRatio, and active-speaker is small', () => {
const layoutElements = getLayoutElementArray({
activeSpeakerId: 'sub1',
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers: [],
isSharingScreen: false,
layoutMode,
Expand Down
18 changes: 9 additions & 9 deletions frontend/src/utils/helpers/getLayoutBoxes/getLayoutElements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const getPublisherLayoutElement = (publisher: Publisher | null) => ({
* We display subscriber as large if it is screenshare or active speaker given that layout mode is active speaker and
* screenshare is not being displayed
* @param {SubscriberWrapper} subWrapper - Subscriber that we are checking
* @param {boolean} hasPinnedSubscribers - whether there are currently pinned subscribers
* @param {number} pinnedSubscriberCount - the number of pinned participant tiles
* @param {boolean} sessionHasScreenshare - whether there is currently a screenshare in the session
* @param {LayoutMode} layoutMode - current layout mode
* @param {(string | undefined)} activeSpeakerId - current active speaker id
Expand All @@ -58,7 +58,7 @@ const getPublisherLayoutElement = (publisher: Publisher | null) => ({
*/
const shouldDisplayBig = (
subWrapper: SubscriberWrapper,
hasPinnedSubscribers: boolean,
pinnedSubscriberCount: number,
sessionHasScreenshare: boolean,
layoutMode: LayoutMode,
activeSpeakerId: string | undefined,
Expand All @@ -76,7 +76,7 @@ const shouldDisplayBig = (

if (
layoutMode === 'active-speaker' &&
hasPinnedSubscribers === false &&
pinnedSubscriberCount === 0 &&
isActiveSpeaker(activeSpeakerId, subWrapper.id, index)
) {
return true;
Expand All @@ -90,15 +90,15 @@ const shouldDisplayBig = (
* - height and width from subscriber
* - isBig for screenshare or active speaker
* - fixedRatio only for screenshare
* @param {boolean} hasPinnedSubscribers - whether there are currently pinned subscribers
* @param {number} pinnedSubscriberCount - the number of pinned subscribers
* @param {SubscriberWrapper[]} subscribersInDisplayOrder - subscriber array in order to be displayed
* @param {boolean} sessionHasScreenshare - boolean indicating if a screenshare is present in the session, used to determine whether to make the active speaker big or not
* @param {LayoutMode} layoutMode - layout mode, to determine whether to make active speaker big or not
* @param {(string | undefined)} activeSpeakerId - current active speaker id
* @returns {Element[]} elements - array of subscriber Elements in order of display
*/
const getSubscriberLayoutElements = (
hasPinnedSubscribers: boolean,
pinnedSubscriberCount: number,
subscribersInDisplayOrder: SubscriberWrapper[],
sessionHasScreenshare: boolean,
layoutMode: LayoutMode,
Expand All @@ -109,7 +109,7 @@ const getSubscriberLayoutElements = (
...getVideoDimensions(subWrapper.subscriber),
big: shouldDisplayBig(
subWrapper,
hasPinnedSubscribers,
pinnedSubscriberCount,
sessionHasScreenshare,
layoutMode,
activeSpeakerId,
Expand Down Expand Up @@ -147,7 +147,7 @@ const hiddenParticipantTileLayoutElement = {

export type GetLayoutElementArrayProps = {
activeSpeakerId: string | undefined;
hasPinnedSubscribers: boolean;
pinnedSubscriberCount: number;
hiddenSubscribers: SubscriberWrapper[];
isSharingScreen: boolean;
layoutMode: LayoutMode;
Expand All @@ -164,7 +164,7 @@ export type GetLayoutElementArrayProps = {
*/
const getLayoutElementArray = ({
activeSpeakerId,
hasPinnedSubscribers,
pinnedSubscriberCount,
hiddenSubscribers,
isSharingScreen,
layoutMode,
Expand All @@ -178,7 +178,7 @@ const getLayoutElementArray = ({
const elements: MaybeElement[] = [
getPublisherLayoutElement(publisher),
...getSubscriberLayoutElements(
hasPinnedSubscribers,
pinnedSubscriberCount,
subscribersInDisplayOrder,
sessionHasScreenshare,
layoutMode,
Expand Down
64 changes: 64 additions & 0 deletions frontend/src/utils/layoutManager.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// import OpenTokLayoutManager, { Box, Element, LayoutContainer } from 'opentok-layout-js';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import LayoutManager from './layoutManager';

const { mockGetLayout, mockConstructor } = vi.hoisted(() => {
const getLayout = vi.fn();
const constructor = vi.fn().mockReturnValue({
getLayout,
});
return { mockGetLayout: getLayout, mockConstructor: constructor };
});

vi.mock('opentok-layout-js', () => ({
default: mockConstructor,
}));

describe('LayoutManager', () => {
let layoutManager: LayoutManager;
beforeEach(() => {
layoutManager = new LayoutManager();
});

it('should create a new layout manager with options', () => {
layoutManager.getLayout({ width: 100, height: 150 }, [], false);
expect(mockConstructor).toHaveBeenCalledWith(
expect.objectContaining({
containerWidth: 100,
containerHeight: 150,
})
);
});

it('should set bigMaxRatio to 9 / 16 if shouldMakeLargeTilesLandscape flag is true', () => {
layoutManager.getLayout({ width: 100, height: 150 }, [], true);
expect(mockConstructor).toHaveBeenCalledWith(
expect.objectContaining({
bigMaxRatio: 9 / 16,
})
);
});

it('should set bigMaxRatio to 3 / 2 if shouldMakeLargeTilesLandscape flag is false', () => {
layoutManager.getLayout({ width: 100, height: 150 }, [], false);
expect(mockConstructor).toHaveBeenCalledWith(
expect.objectContaining({
bigMaxRatio: 3 / 2,
})
);
});

it('should set return boxes from layout manager', () => {
const boxes = [
{
height: 0,
left: 5,
top: 10,
width: 20,
},
];
mockGetLayout.mockReturnValue({ boxes });
const layoutBoxes = layoutManager.getLayout({ width: 100, height: 150 }, [], false);
expect(layoutBoxes).toBe(boxes);
});
});
18 changes: 12 additions & 6 deletions frontend/src/utils/layoutManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @flow
import OpenTokLayoutManager, { Box, Element, LayoutContainer } from 'opentok-layout-js';

// Opentok element methods videoHeight() and videoWidth() can return undefined so
Expand All @@ -16,7 +15,10 @@ export type MaybeElement = {
class LayoutManager {
manager?: LayoutContainer;

init(containerDimensions: { height: number; width: number }) {
init(
containerDimensions: { height: number; width: number },
shouldMakeLargeTilesLandscape: boolean = false
) {
// Layout options see: https://github.com/aullman/opentok-layout-js?tab=readme-ov-file#usage
this.manager = OpenTokLayoutManager({
fixedRatio: false,
Expand All @@ -34,18 +36,22 @@ class LayoutManager {
smallMaxHeight: Infinity,
bigMaxWidth: Infinity,
bigMaxHeight: Infinity,
bigMaxRatio: 3 / 2,
bigMaxRatio: shouldMakeLargeTilesLandscape ? 9 / 16 : 3 / 2,
bigMinRatio: 9 / 16,
bigFirst: true,
containerWidth: containerDimensions.width,
containerHeight: containerDimensions.height,
});
}
getLayout(containerDimensions: { height: number; width: number }, boxes: Element[]): Box[] {
getLayout(
containerDimensions: { height: number; width: number },
elements: Element[],
shouldMakeLargeTilesLandscape: boolean
): Box[] {
this.init(containerDimensions, shouldMakeLargeTilesLandscape);
// Currently the layout manager doesn't support updating dimensions on the fly so we must re-create the manager every time
// https://github.com/aullman/opentok-layout-js/issues/141
this.init(containerDimensions);
return this.manager?.getLayout(boxes)?.boxes ?? [];
return this.manager?.getLayout(elements)?.boxes ?? [];
}
}
export default LayoutManager;

0 comments on commit d5b1e23

Please sign in to comment.