From 6798e070ac0bd3d8ad22287244030f358568ac55 Mon Sep 17 00:00:00 2001 From: mgamis-msft <79475487+mgamis-msft@users.noreply.github.com> Date: Sun, 5 Nov 2023 01:00:25 -0800 Subject: [PATCH] Prevent loop of switching between scrollable and paginated horizontal gallery (#3730) * Prevent possible endless loop of switching between scrollable and paginated horizontal gallery in VideoGallery * Change files * Duplicate change files for beta release * fix and memoize scrollable horizontal gallery container style * lint fix * fix scrollable horizontal gallery width for other video gallery layouts * dependecy array lint fix --- ...-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json | 9 +++++++ ...-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json | 9 +++++++ .../review/beta/acs-ui-common.api.md | 3 +++ .../review/stable/acs-ui-common.api.md | 3 +++ packages/acs-ui-common/src/common.ts | 10 ++++++++ packages/acs-ui-common/src/index.ts | 2 +- .../components/VideoGallery/DefaultLayout.tsx | 4 ++- .../VideoGallery/FloatingLocalVideoLayout.tsx | 4 ++- .../VideoGallery/LargeGalleryLayout.tsx | 4 ++- .../VideoGallery/OverflowGallery.tsx | 25 ++++++++++++++++--- .../ScrollableHorizontalGallery.tsx | 18 +++---------- .../VideoGallery/SpeakerVideoLayout.tsx | 4 ++- .../ScrollableHorizontalGallery.style.ts | 20 ++++++--------- 13 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 change-beta/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json create mode 100644 change/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json diff --git a/change-beta/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json b/change-beta/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json new file mode 100644 index 00000000000..4bf2e5c6ddc --- /dev/null +++ b/change-beta/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json @@ -0,0 +1,9 @@ +{ + "type": "patch", + "area": "fix", + "workstream": "VideoGallery", + "comment": "Prevent possible endless loop of switching between scrollable and paginated horizontal gallery in VideoGallery", + "packageName": "@azure/communication-react", + "email": "79475487+mgamis-msft@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json b/change/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json new file mode 100644 index 00000000000..4bf2e5c6ddc --- /dev/null +++ b/change/@azure-communication-react-8d637679-4d5b-4c50-826a-ce0acdd7c66d.json @@ -0,0 +1,9 @@ +{ + "type": "patch", + "area": "fix", + "workstream": "VideoGallery", + "comment": "Prevent possible endless loop of switching between scrollable and paginated horizontal gallery in VideoGallery", + "packageName": "@azure/communication-react", + "email": "79475487+mgamis-msft@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/acs-ui-common/review/beta/acs-ui-common.api.md b/packages/acs-ui-common/review/beta/acs-ui-common.api.md index 67fab56e7a0..e89cbb0d938 100644 --- a/packages/acs-ui-common/review/beta/acs-ui-common.api.md +++ b/packages/acs-ui-common/review/beta/acs-ui-common.api.md @@ -30,6 +30,9 @@ export type CommonProperties = { [P in keyof A & keyof B]: A[P] extends B[P] ? P : never; }[keyof A & keyof B]; +// @internal +export const _convertPxToRem: (px: number) => number; + // @internal export const _convertRemToPx: (rem: number) => number; diff --git a/packages/acs-ui-common/review/stable/acs-ui-common.api.md b/packages/acs-ui-common/review/stable/acs-ui-common.api.md index 67fab56e7a0..e89cbb0d938 100644 --- a/packages/acs-ui-common/review/stable/acs-ui-common.api.md +++ b/packages/acs-ui-common/review/stable/acs-ui-common.api.md @@ -30,6 +30,9 @@ export type CommonProperties = { [P in keyof A & keyof B]: A[P] extends B[P] ? P : never; }[keyof A & keyof B]; +// @internal +export const _convertPxToRem: (px: number) => number; + // @internal export const _convertRemToPx: (rem: number) => number; diff --git a/packages/acs-ui-common/src/common.ts b/packages/acs-ui-common/src/common.ts index ea77236959b..885dfd59566 100644 --- a/packages/acs-ui-common/src/common.ts +++ b/packages/acs-ui-common/src/common.ts @@ -11,6 +11,16 @@ export const _convertRemToPx = (rem: number): number => { return rem * parseFloat(getComputedStyle(document.documentElement).fontSize); }; +/** + * @internal + * Converts units of pixels to units of rem + * @param px - units of px + * @returns units of rem + */ +export const _convertPxToRem = (px: number): number => { + return px / parseFloat(getComputedStyle(document.documentElement).fontSize); +}; + /** * @internal * Disable dismiss on resize to work around a couple Fluent UI bugs diff --git a/packages/acs-ui-common/src/index.ts b/packages/acs-ui-common/src/index.ts index dd3e8563a5e..c31c3ecb7c2 100644 --- a/packages/acs-ui-common/src/index.ts +++ b/packages/acs-ui-common/src/index.ts @@ -11,7 +11,7 @@ export { export { _getApplicationId } from './telemetry'; export { _formatString } from './localizationUtils'; export { _safeJSONStringify } from './safeStringify'; -export { _convertRemToPx, _preventDismissOnEvent } from './common'; +export { _convertPxToRem, _convertRemToPx, _preventDismissOnEvent } from './common'; export type { Common, CommonProperties } from './commonProperties'; export type { CallbackType, FunctionWithKey } from './memoizeFnAll'; diff --git a/packages/react-components/src/components/VideoGallery/DefaultLayout.tsx b/packages/react-components/src/components/VideoGallery/DefaultLayout.tsx index e78b6093417..fe52ebe976f 100644 --- a/packages/react-components/src/components/VideoGallery/DefaultLayout.tsx +++ b/packages/react-components/src/components/VideoGallery/DefaultLayout.tsx @@ -120,6 +120,7 @@ export const DefaultLayout = (props: DefaultLayoutProps): JSX.Element => { }} /* @conditional-compile-remove(gallery-layouts) */ layout={'default'} + parentWidth={parentWidth} /> ); }, [ @@ -129,7 +130,8 @@ export const DefaultLayout = (props: DefaultLayoutProps): JSX.Element => { styles?.horizontalGallery, /* @conditional-compile-remove(vertical-gallery) */ overflowGalleryPosition, setIndexesToRender, - /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery + /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery, + parentWidth ]); return ( diff --git a/packages/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx b/packages/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx index aab94ba72bf..cd7ad72a350 100644 --- a/packages/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx +++ b/packages/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx @@ -218,6 +218,7 @@ export const FloatingLocalVideoLayout = (props: FloatingLocalVideoLayoutProps): onChildrenPerPageChange={(n: number) => { childrenPerPage.current = n; }} + parentWidth={parentWidth} /> ); }, [ @@ -228,7 +229,8 @@ export const FloatingLocalVideoLayout = (props: FloatingLocalVideoLayoutProps): styles?.horizontalGallery, /* @conditional-compile-remove(vertical-gallery) */ overflowGalleryPosition, setIndexesToRender, - /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery + /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery, + parentWidth ]); return ( diff --git a/packages/react-components/src/components/VideoGallery/LargeGalleryLayout.tsx b/packages/react-components/src/components/VideoGallery/LargeGalleryLayout.tsx index 3e342a76c6f..81ce628d83a 100644 --- a/packages/react-components/src/components/VideoGallery/LargeGalleryLayout.tsx +++ b/packages/react-components/src/components/VideoGallery/LargeGalleryLayout.tsx @@ -131,6 +131,7 @@ export const LargeGalleryLayout = (props: LargeGalleryProps): JSX.Element => { onChildrenPerPageChange={(n: number) => { childrenPerPage.current = n; }} + parentWidth={parentWidth} /> ); }, [ @@ -140,7 +141,8 @@ export const LargeGalleryLayout = (props: LargeGalleryProps): JSX.Element => { styles?.horizontalGallery, /* @conditional-compile-remove(vertical-gallery) */ overflowGalleryPosition, setIndexesToRender, - /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery + /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery, + parentWidth ]); return ( diff --git a/packages/react-components/src/components/VideoGallery/OverflowGallery.tsx b/packages/react-components/src/components/VideoGallery/OverflowGallery.tsx index a601b1d1996..034a24e3feb 100644 --- a/packages/react-components/src/components/VideoGallery/OverflowGallery.tsx +++ b/packages/react-components/src/components/VideoGallery/OverflowGallery.tsx @@ -16,10 +16,13 @@ import { OverflowGalleryPosition } from '../VideoGallery'; import { VideoGalleryLayout } from '../VideoGallery'; import { ScrollableHorizontalGallery } from './ScrollableHorizontalGallery'; import { + SMALL_HORIZONTAL_GALLERY_TILE_SIZE_REM, horizontalGalleryContainerStyle, horizontalGalleryStyle } from './styles/VideoGalleryResponsiveHorizontalGallery.styles'; /* @conditional-compile-remove(vertical-gallery) */ +import { _convertPxToRem } from '@internal/acs-ui-common'; +import { SMALL_FLOATING_MODAL_SIZE_REM } from './styles/FloatingLocalVideo.styles'; import { verticalGalleryContainerStyle, verticalGalleryStyle @@ -45,6 +48,7 @@ export const OverflowGallery = (props: { onChildrenPerPageChange?: (childrenPerPage: number) => void; /* @conditional-compile-remove(gallery-layouts) */ layout?: VideoGalleryLayout; + parentWidth?: number; }): JSX.Element => { const { shouldFloatLocalVideo = false, @@ -56,7 +60,8 @@ export const OverflowGallery = (props: { horizontalGalleryStyles, /* @conditional-compile-remove(vertical-gallery) */ overflowGalleryPosition = 'horizontalBottom', /* @conditional-compile-remove(vertical-gallery) */ verticalGalleryStyles, - onChildrenPerPageChange + onChildrenPerPageChange, + parentWidth } = props; const containerStyles = useMemo(() => { @@ -86,6 +91,18 @@ export const OverflowGallery = (props: { /* @conditional-compile-remove(vertical-gallery) */ verticalGalleryStyles ]); + const scrollableHorizontalGalleryContainerStyles = useMemo(() => { + if (isNarrow && parentWidth) { + return { + width: + props.layout === 'default' + ? `${_convertPxToRem(parentWidth) - 1}rem` + : `${_convertPxToRem(parentWidth) - SMALL_FLOATING_MODAL_SIZE_REM.width - 1}rem` + }; + } + return undefined; + }, [isNarrow, parentWidth, props.layout]); + /* @conditional-compile-remove(vertical-gallery) */ if (overflowGalleryPosition === 'verticalRight') { return ( @@ -104,18 +121,20 @@ export const OverflowGallery = (props: { ); } + SMALL_HORIZONTAL_GALLERY_TILE_SIZE_REM; + /* @conditional-compile-remove(pinned-participants) */ if (isNarrow) { // There are no pages for ScrollableHorizontalGallery so we will approximate the first 3 remote // participant tiles are visible onChildrenPerPageChange?.(3); + return ( ]} onFetchTilesToRender={onFetchTilesToRender} key="scrollable-horizontal-gallery" - /* @conditional-compile-remove(gallery-layouts) */ - layout={props.layout} + containerStyles={scrollableHorizontalGalleryContainerStyles} /> ); } diff --git a/packages/react-components/src/components/VideoGallery/ScrollableHorizontalGallery.tsx b/packages/react-components/src/components/VideoGallery/ScrollableHorizontalGallery.tsx index 359774db0fc..455f21c06bf 100644 --- a/packages/react-components/src/components/VideoGallery/ScrollableHorizontalGallery.tsx +++ b/packages/react-components/src/components/VideoGallery/ScrollableHorizontalGallery.tsx @@ -1,15 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { Stack } from '@fluentui/react'; +import { IStyle, Stack, mergeStyles } from '@fluentui/react'; import React, { useEffect, useRef } from 'react'; import { useDraggable } from 'react-use-draggable-scroll'; import { scrollableHorizontalGalleryContainerStyles, scrollableHorizontalGalleryStyles } from './styles/ScrollableHorizontalGallery.style'; -/* @conditional-compile-remove(gallery-layouts) */ -import { VideoGalleryLayout } from '../VideoGallery'; /** * Component to display elements horizontally in a scrollable container @@ -18,17 +16,9 @@ import { VideoGalleryLayout } from '../VideoGallery'; export const ScrollableHorizontalGallery = (props: { horizontalGalleryElements?: JSX.Element[]; onFetchTilesToRender?: (indexes: number[]) => void; - /* @conditional-compile-remove(gallery-layouts) */ - layout?: VideoGalleryLayout; + containerStyles?: IStyle; }): JSX.Element => { - const { horizontalGalleryElements, onFetchTilesToRender, /* @conditional-compile-remove(gallery-layouts) */ layout } = - props; - - const useFullWidthTrampoline = (): boolean => { - /* @conditional-compile-remove(gallery-layouts) */ - return layout === 'default' ? true : false; - return false; - }; + const { horizontalGalleryElements, onFetchTilesToRender, containerStyles } = props; useEffect(() => { const indexesArray = [...Array(horizontalGalleryElements?.length).keys()]; @@ -44,7 +34,7 @@ export const ScrollableHorizontalGallery = (props: {
{ childrenPerPage.current = n; }} + parentWidth={parentWidth} /> ); }, [ @@ -203,7 +204,8 @@ export const SpeakerVideoLayout = (props: SpeakerVideoLayoutProps): JSX.Element styles?.horizontalGallery, /* @conditional-compile-remove(vertical-gallery) */ overflowGalleryPosition, setIndexesToRender, - /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery + /* @conditional-compile-remove(vertical-gallery) */ styles?.verticalGallery, + parentWidth ]); return ( diff --git a/packages/react-components/src/components/VideoGallery/styles/ScrollableHorizontalGallery.style.ts b/packages/react-components/src/components/VideoGallery/styles/ScrollableHorizontalGallery.style.ts index 73f609188ff..d45e685212f 100644 --- a/packages/react-components/src/components/VideoGallery/styles/ScrollableHorizontalGallery.style.ts +++ b/packages/react-components/src/components/VideoGallery/styles/ScrollableHorizontalGallery.style.ts @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { IStackStyles, mergeStyles } from '@fluentui/react'; -import { SMALL_FLOATING_MODAL_SIZE_REM } from './FloatingLocalVideo.styles'; +import { IStackStyles, IStyle } from '@fluentui/react'; import { SMALL_HORIZONTAL_GALLERY_TILE_SIZE_REM, SMALL_HORIZONTAL_GALLERY_TILE_STYLE @@ -23,14 +22,11 @@ export const scrollableHorizontalGalleryStyles: IStackStyles = { /** * @private */ -export const scrollableHorizontalGalleryContainerStyles = (fullWidth: boolean): string => { - return mergeStyles({ - display: 'flex', - width: fullWidth ? '100%' : `calc(100% - ${SMALL_FLOATING_MODAL_SIZE_REM.width}rem)`, - minHeight: `${SMALL_HORIZONTAL_GALLERY_TILE_SIZE_REM.height}rem`, - overflow: 'scroll', - '-ms-overflow-style': 'none', - 'scrollbar-width': 'none', - '::-webkit-scrollbar': { display: 'none' } - }); +export const scrollableHorizontalGalleryContainerStyles: IStyle = { + display: 'flex', + minHeight: `${SMALL_HORIZONTAL_GALLERY_TILE_SIZE_REM.height}rem`, + overflow: 'scroll', + '-ms-overflow-style': 'none', + 'scrollbar-width': 'none', + '::-webkit-scrollbar': { display: 'none' } };