Skip to content

Commit 6457103

Browse files
mgamis-msftdmceachernmsft
authored and
pull[bot]
committed
Keep dominant speakers on the first page of overflow gallery (#2819)
* fix dominant speaker ordering to keep dominant speakers on the first page of overflow gallery * Change files * Duplicate change files * adding video participants at the end of dominant speaker list in case there is room * adjust max horizontal gallery dominant speakers based on pinned participants * order remote participants such that video participants are first. do not maintain order for indices after max dominant speakers. * update expectations in VideoGallery.test.tsx * added tests to videoGalleryLayoutUtils.test.tsx * change childrenPerPage initial value to 4 * update dominant speaker test expected value --------- Co-authored-by: Donald McEachern <94866715+dmceachernmsft@users.noreply.github.com>
1 parent 93afbca commit 6457103

13 files changed

Lines changed: 316 additions & 96 deletions

‎change-beta/@azure-communication-react-508d475c-616e-4784-be66-737a4a2facb2.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix dominant speaker ordering to best keep dominant speakers on the first page of overflow gallery.",
4+
"packageName": "@azure/communication-react",
5+
"email": "miguelgamis@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-communication-react-508d475c-616e-4784-be66-737a4a2facb2.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix dominant speaker ordering to best keep dominant speakers on the first page of overflow gallery.",
4+
"packageName": "@azure/communication-react",
5+
"email": "miguelgamis@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎packages/react-components/src/components/ResponsiveHorizontalGallery.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ export const ResponsiveHorizontalGallery = (props: {
1818
gapWidthRem: number;
1919
buttonWidthRem?: number;
2020
onFetchTilesToRender?: (indexes: number[]) => void;
21+
/** event to listen for children per page changes */
22+
onChildrenPerPageChange?: (childrenPerPage: number) => void;
2123
}): JSX.Element => {
22-
const { gapWidthRem, buttonWidthRem = 0, onFetchTilesToRender } = props;
24+
const { gapWidthRem, buttonWidthRem = 0, onFetchTilesToRender, onChildrenPerPageChange } = props;
2325
const containerRef = useRef<HTMLDivElement>(null);
2426
const containerWidth = _useContainerWidth(containerRef);
2527

@@ -32,6 +34,7 @@ export const ResponsiveHorizontalGallery = (props: {
3234
gapWidthRem,
3335
buttonWidthRem
3436
});
37+
onChildrenPerPageChange?.(childrenPerPage);
3538

3639
return (
3740
<div data-ui-id="responsive-horizontal-gallery" ref={containerRef} className={mergeStyles(props.containerStyles)}>

‎packages/react-components/src/components/ResponsiveVerticalGallery.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export interface ResponsiveVerticalGalleryProps {
2828
isShort?: boolean;
2929
/** Function to set which tiles to give video to in the children. */
3030
onFetchTilesToRender?: (indexes: number[]) => void;
31+
/** event to listen for children per page changes */
32+
onChildrenPerPageChange?: (childrenPerPage: number) => void;
3133
}
3234

3335
/**
@@ -45,7 +47,8 @@ export const ResponsiveVerticalGallery = (props: ResponsiveVerticalGalleryProps)
4547
gapHeightRem,
4648
controlBarHeightRem,
4749
isShort,
48-
onFetchTilesToRender
50+
onFetchTilesToRender,
51+
onChildrenPerPageChange
4952
} = props;
5053
const containerRef = useRef<HTMLDivElement>(null);
5154
const containerHeight = _useContainerHeight(containerRef);
@@ -60,6 +63,7 @@ export const ResponsiveVerticalGallery = (props: ResponsiveVerticalGalleryProps)
6063
controlBarHeight: controlBarHeightRem ?? 2,
6164
isShort: isShort ?? false
6265
});
66+
onChildrenPerPageChange?.(childrenPerPage);
6367
return (
6468
<div data-ui-id="responsive-vertical-gallery" ref={containerRef} className={mergeStyles(containerStyles)}>
6569
<VerticalGallery

‎packages/react-components/src/components/VideoGallery.test.tsx‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ describe('VideoGallery default layout tests', () => {
164164
expect(root.find(RemoteScreenShare).length).toBe(1);
165165
expect(root.find(HorizontalGallery).find(VideoTile).length).toBe(2);
166166
expect(root.find(HorizontalGallery).find(StreamMedia).length).toBe(1);
167-
expect(root.find(HorizontalGallery).find(VideoTile).first().prop('userId')).toBe('remoteVideoParticipant');
168-
expect(root.find(HorizontalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(true);
169-
expect(root.find(HorizontalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteScreenSharingParticipant');
170-
expect(root.find(HorizontalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(false);
167+
expect(root.find(HorizontalGallery).find(VideoTile).first().prop('userId')).toBe('remoteScreenSharingParticipant');
168+
expect(root.find(HorizontalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(false);
169+
expect(root.find(HorizontalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteVideoParticipant');
170+
expect(root.find(HorizontalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(true);
171171
});
172172
});
173173

@@ -285,10 +285,10 @@ describe('VideoGallery floating local video layout tests', () => {
285285
expect(root.find(RemoteScreenShare).length).toBe(1);
286286
expect(root.find(HorizontalGallery).find(VideoTile).length).toBe(2);
287287
expect(root.find(HorizontalGallery).find(StreamMedia).length).toBe(1);
288-
expect(root.find(HorizontalGallery).find(VideoTile).first().prop('userId')).toBe('remoteVideoParticipant');
289-
expect(root.find(HorizontalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(true);
290-
expect(root.find(HorizontalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteScreenSharingParticipant');
291-
expect(root.find(HorizontalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(false);
288+
expect(root.find(HorizontalGallery).find(VideoTile).first().prop('userId')).toBe('remoteScreenSharingParticipant');
289+
expect(root.find(HorizontalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(false);
290+
expect(root.find(HorizontalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteVideoParticipant');
291+
expect(root.find(HorizontalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(true);
292292
});
293293
});
294294

@@ -485,10 +485,10 @@ describe('VideoGallery with vertical overflow gallery tests', () => {
485485
expect(root.find(RemoteScreenShare).length).toBe(1);
486486
expect(root.find(VerticalGallery).find(VideoTile).length).toBe(4);
487487
expect(root.find(VerticalGallery).find(StreamMedia).length).toBe(1);
488-
expect(root.find(VerticalGallery).find(VideoTile).first().prop('userId')).toBe('remoteVideoParticipant');
489-
expect(root.find(VerticalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(true);
490-
expect(root.find(VerticalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteScreenSharingParticipant');
491-
expect(root.find(VerticalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(false);
488+
expect(root.find(VerticalGallery).find(VideoTile).first().prop('userId')).toBe('remoteScreenSharingParticipant');
489+
expect(root.find(VerticalGallery).find(VideoTile).first().find(StreamMedia).exists()).toBe(false);
490+
expect(root.find(VerticalGallery).find(VideoTile).at(1).prop('userId')).toBe('remoteVideoParticipant');
491+
expect(root.find(VerticalGallery).find(VideoTile).at(1).find(StreamMedia).exists()).toBe(true);
492492
});
493493

494494
/* @conditional-compile-remove(pinned-participants) */

‎packages/react-components/src/components/VideoGallery/DefaultLayout.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT license.
33

44
import { Stack } from '@fluentui/react';
5-
import React, { useMemo, useState } from 'react';
5+
import React, { useMemo, useState, useRef } from 'react';
66
import { GridLayout } from '../GridLayout';
77
import { isNarrowWidth } from '../utils/responsive';
88
/* @conditional-compile-remove(vertical-gallery) */
@@ -38,7 +38,7 @@ export const DefaultLayout = (props: DefaultLayoutProps): JSX.Element => {
3838
parentWidth,
3939
/* @conditional-compile-remove(vertical-gallery) */
4040
parentHeight,
41-
/* @conditional-compile-remove(pinned-participants) */ pinnedParticipantUserIds,
41+
pinnedParticipantUserIds = [],
4242
/* @conditional-compile-remove(vertical-gallery) */ overflowGalleryLayout = 'HorizontalBottom'
4343
} = props;
4444

@@ -47,11 +47,17 @@ export const DefaultLayout = (props: DefaultLayoutProps): JSX.Element => {
4747
/* @conditional-compile-remove(vertical-gallery) */
4848
const isShort = parentHeight ? isShortHeight(parentHeight) : false;
4949

50+
// This is for tracking the number of children in the first page of horizontal gallery.
51+
// This number will be used for the maxHorizontalDominantSpeakers when organizing the remote participants.
52+
const childrenPerPage = useRef(4);
5053
const { gridParticipants, horizontalGalleryParticipants } = useOrganizedParticipants({
5154
remoteParticipants,
5255
dominantSpeakers,
5356
maxRemoteVideoStreams,
5457
isScreenShareActive: !!screenShareComponent,
58+
maxHorizontalGalleryDominantSpeakers: screenShareComponent
59+
? childrenPerPage.current - (pinnedParticipantUserIds.length % childrenPerPage.current)
60+
: childrenPerPage.current,
5561
/* @conditional-compile-remove(pinned-participants) */ pinnedParticipantUserIds
5662
});
5763

@@ -107,6 +113,9 @@ export const DefaultLayout = (props: DefaultLayoutProps): JSX.Element => {
107113
/* @conditional-compile-remove(pinned-participants) */
108114
overflowGalleryLayout={overflowGalleryLayout}
109115
onFetchTilesToRender={setIndexesToRender}
116+
onChildrenPerPageChange={(n: number) => {
117+
childrenPerPage.current = n;
118+
}}
110119
/>
111120
);
112121
}, [

‎packages/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { LayerHost, mergeStyles, Stack } from '@fluentui/react';
55
import { useId } from '@fluentui/react-hooks';
6-
import React, { useMemo, useState } from 'react';
6+
import React, { useMemo, useRef, useState } from 'react';
77
import { useTheme } from '../../theming';
88
import { GridLayout } from '../GridLayout';
99
import { isNarrowWidth } from '../utils/responsive';
@@ -62,7 +62,7 @@ export const FloatingLocalVideoLayout = (props: FloatingLocalVideoLayoutProps):
6262
showCameraSwitcherInLocalPreview,
6363
parentWidth,
6464
parentHeight,
65-
/* @conditional-compile-remove(pinned-participants) */ pinnedParticipantUserIds,
65+
pinnedParticipantUserIds = [],
6666
/* @conditional-compile-remove(vertical-gallery) */ overflowGalleryLayout = 'HorizontalBottom'
6767
} = props;
6868

@@ -73,11 +73,17 @@ export const FloatingLocalVideoLayout = (props: FloatingLocalVideoLayoutProps):
7373
/* @conditional-compile-remove(vertical-gallery) */
7474
const isShort = parentHeight ? isShortHeight(parentHeight) : false;
7575

76+
// This is for tracking the number of children in the first page of horizontal gallery.
77+
// This number will be used for the maxHorizontalDominantSpeakers when organizing the remote participants.
78+
const childrenPerPage = useRef(4);
7679
const { gridParticipants, horizontalGalleryParticipants } = useOrganizedParticipants({
7780
remoteParticipants,
7881
dominantSpeakers,
7982
maxRemoteVideoStreams,
8083
isScreenShareActive: !!screenShareComponent,
84+
maxHorizontalGalleryDominantSpeakers: screenShareComponent
85+
? childrenPerPage.current - (pinnedParticipantUserIds.length % childrenPerPage.current)
86+
: childrenPerPage.current,
8187
/* @conditional-compile-remove(pinned-participants) */ pinnedParticipantUserIds
8288
});
8389

@@ -185,6 +191,9 @@ export const FloatingLocalVideoLayout = (props: FloatingLocalVideoLayoutProps):
185191
veritcalGalleryStyles={styles?.verticalGallery}
186192
/* @conditional-compile-remove(vertical-gallery) */
187193
overflowGalleryLayout={overflowGalleryLayout}
194+
onChildrenPerPageChange={(n: number) => {
195+
childrenPerPage.current = n;
196+
}}
188197
/>
189198
);
190199
}, [

‎packages/react-components/src/components/VideoGallery/Layout.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ export interface LayoutProps {
3939
* Height of parent element
4040
*/
4141
parentHeight?: number;
42-
/* @conditional-compile-remove(pinned-participants) */
4342
/**
4443
* List of pinned participant userIds
4544
*/

‎packages/react-components/src/components/VideoGallery/OverflowGallery.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const OverflowGallery = (props: {
4141
veritcalGalleryStyles?: VerticalGalleryStyles;
4242
/* @conditional-compile-remove(vertical-gallery) */
4343
overflowGalleryLayout?: OverflowGalleryLayout;
44+
onChildrenPerPageChange?: (childrenPerPage: number) => void;
4445
}): JSX.Element => {
4546
const {
4647
shouldFloatLocalVideo = false,
@@ -51,7 +52,8 @@ export const OverflowGallery = (props: {
5152
overflowGalleryElements,
5253
horizontalGalleryStyles,
5354
/* @conditional-compile-remove(vertical-gallery) */ overflowGalleryLayout = 'HorizontalBottom',
54-
/* @conditional-compile-remove(vertical-gallery) */ veritcalGalleryStyles
55+
/* @conditional-compile-remove(vertical-gallery) */ veritcalGalleryStyles,
56+
onChildrenPerPageChange
5557
} = props;
5658

5759
const containerStyles = useMemo(() => {
@@ -92,6 +94,7 @@ export const OverflowGallery = (props: {
9294
gapHeightRem={HORIZONTAL_GALLERY_GAP}
9395
isShort={isShort}
9496
onFetchTilesToRender={onFetchTilesToRender}
97+
onChildrenPerPageChange={onChildrenPerPageChange}
9598
>
9699
{overflowGalleryElements}
97100
</ResponsiveVerticalGallery>
@@ -100,6 +103,9 @@ export const OverflowGallery = (props: {
100103

101104
/* @conditional-compile-remove(pinned-participants) */
102105
if (isNarrow) {
106+
// There are no pages for ScrollableHorizontalGallery so we will approximate the first 3 remote
107+
// participant tiles are visible
108+
onChildrenPerPageChange?.(3);
103109
return (
104110
<ScrollableHorizontalGallery
105111
horizontalGalleryElements={overflowGalleryElements}
@@ -117,6 +123,7 @@ export const OverflowGallery = (props: {
117123
horizontalGalleryStyles={galleryStyles}
118124
buttonWidthRem={HORIZONTAL_GALLERY_BUTTON_WIDTH}
119125
gapWidthRem={HORIZONTAL_GALLERY_GAP}
126+
onChildrenPerPageChange={onChildrenPerPageChange}
120127
>
121128
{overflowGalleryElements}
122129
</ResponsiveHorizontalGallery>

0 commit comments

Comments
 (0)
⚡