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
Azure · Nov 5, 2023 · 6798e07 · 6798e07
1 parent 84b4c01
commit 6798e07
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 34 deletions.
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
@@ -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": "[email protected]",
+  "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
@@ -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": "[email protected]",
+  "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
@@ -30,6 +30,9 @@ export type CommonProperties<A, B> = {
     [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
@@ -30,6 +30,9 @@ export type CommonProperties<A, B> = {
     [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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 (
       <ScrollableHorizontalGallery
         horizontalGalleryElements={overflowGalleryElements ? overflowGalleryElements : [<></>]}
         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
@@ -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: {
     <div
       ref={ref}
       {...dragabbleEvents}
-      className={scrollableHorizontalGalleryContainerStyles(useFullWidthTrampoline())}
+      className={mergeStyles(scrollableHorizontalGalleryContainerStyles, containerStyles)}
     >
       <Stack
         data-ui-id="scrollable-horizontal-gallery"

diff --git a/packages/react-components/src/components/VideoGallery/SpeakerVideoLayout.tsx b/packages/react-components/src/components/VideoGallery/SpeakerVideoLayout.tsx
@@ -193,6 +193,7 @@ export const SpeakerVideoLayout = (props: SpeakerVideoLayoutProps): JSX.Element
         onChildrenPerPageChange={(n: number) => {
           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/.../react-components/src/components/VideoGallery/styles/ScrollableHorizontalGallery.style.ts b/.../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' }
 };