Fix VideoEffects Pane reseting focus when an item is selected (#4618)

Author: JamesBurnside

change-beta/@azure-communication-react-95da9d89-f77e-4f64-a28f-ef0f67889d41.json

@@ -0,0 +1,9 @@
+{
+  "type": "patch",
+  "area": "fix",
+  "workstream": "",
+  "comment": "Fix VideoEffects Pane reseting focus when an item is selected",
+  "packageName": "@azure/communication-react",
+  "email": "2684369+JamesBurnside@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

change/@azure-communication-react-95da9d89-f77e-4f64-a28f-ef0f67889d41.json

@@ -0,0 +1,9 @@
+{
+  "type": "patch",
+  "area": "fix",
+  "workstream": "",
+  "comment": "Fix VideoEffects Pane reseting focus when an item is selected",
+  "packageName": "@azure/communication-react",
+  "email": "2684369+JamesBurnside@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

packages/react-components/src/components/VideoEffects/VideoBackgroundEffectsPicker.test.tsx

@@ -45,25 +45,19 @@ const TEST_PICKER_OPTIONS = [
 
 describe('Background Picker should have the correct number of tiles', () => {
   test('Background Picker should have 7 tiles when items are set to wrap', async () => {
-    const { container } = render(
-      <_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow="wrap" ignoreFocusOnMount={true} />
-    );
+    const { container } = render(<_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow="wrap" />);
     expect(container.querySelectorAll('[data-ui-id="video-effects-item"]').length).toBe(TEST_PICKER_OPTIONS.length);
   });
 
   test('Background Picker should have 7 tiles and 2 hidden tiles spanning 3 rows when items per row is set to 3', async () => {
-    const { container } = render(
-      <_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow={3} ignoreFocusOnMount={true} />
-    );
+    const { container } = render(<_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow={3} />);
     expect(container.querySelectorAll('[data-ui-id="video-effects-item"]').length).toBe(7);
     expect(container.querySelectorAll('[data-ui-id="video-effects-hidden-item"]').length).toBe(2);
     expect(container.querySelectorAll('[data-ui-id="video-effects-picker-row"]').length).toBe(3);
   });
 
   test('Background Picker should have 7 tiles and 1 hidden tile spanning 4 rows when items per row is set to 2', async () => {
-    const { container } = render(
-      <_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow={2} ignoreFocusOnMount={true} />
-    );
+    const { container } = render(<_VideoBackgroundEffectsPicker options={TEST_PICKER_OPTIONS} itemsPerRow={2} />);
     expect(container.querySelectorAll('[data-ui-id="video-effects-item"]').length).toBe(7);
     expect(container.querySelectorAll('[data-ui-id="video-effects-hidden-item"]').length).toBe(1);
     expect(container.querySelectorAll('[data-ui-id="video-effects-picker-row"]').length).toBe(4);

packages/react-components/src/components/VideoEffects/VideoBackgroundEffectsPicker.tsx

@@ -54,13 +54,11 @@ export interface _VideoBackgroundEffectsPickerProps {
   styles?: _VideoBackgroundEffectsPickerStyles;
 
   /**
-   * Used only for jest tests. If `true`, the autofocus-on-mount will be ignored when the component mounts.
-   * If not, jest tries to cleanup after the first render pass, but the component throws an error because
-   * it tries to display a tooltip after that render pass and after the component is cleaned up.
-   *
-   * This can be removed once: https://github.com/microsoft/fluentui/issues/30896 is fixed.
+   * Imperative handle for calling focus()
    */
-  ignoreFocusOnMount?: boolean;
+  componentRef?: React.RefObject<{
+    focus: () => void;
+  }>;
 }
 
 /**
@@ -151,7 +149,7 @@ export const _VideoBackgroundEffectsPicker = (props: _VideoBackgroundEffectsPick
                   {...option}
                   itemKey={option.itemKey}
                   key={option.itemKey}
-                  focusOnMount={true && !props.ignoreFocusOnMount}
+                  componentRef={props.componentRef}
                 />
               );
             }

packages/react-components/src/components/VideoEffects/VideoEffectsItem.tsx

@@ -14,7 +14,7 @@ import {
   TooltipHost,
   useTheme
 } from '@fluentui/react';
-import React, { useCallback, useEffect } from 'react';
+import React, { useCallback } from 'react';
 import { videoEffectsItemContainerStyles } from './VideoEffectsItem.styles';
 
 /**
@@ -87,9 +87,11 @@ export interface _VideoEffectsItemProps {
   styles?: _VideoEffectsItemStyles;
 
   /**
-   * Should focus on mounting of the picker item
+   * Imperative handle for calling focus()
    */
-  focusOnMount?: boolean;
+  componentRef?: React.RefObject<{
+    focus: () => void;
+  }>;
 }
 
 /**
@@ -138,14 +140,6 @@ export const _VideoEffectsItem = (props: _VideoEffectsItemProps): JSX.Element =>
     [backgroundImage, disabled, isSelected, theme]
   );
 
-  const componentRef = React.createRef<IButton>();
-
-  useEffect(() => {
-    if (props.focusOnMount && componentRef.current) {
-      componentRef.current.focus();
-    }
-  }, [componentRef, props.focusOnMount]);
-
   return (
     <TooltipHost {...props.tooltipProps}>
       <Stack
@@ -160,8 +154,7 @@ export const _VideoEffectsItem = (props: _VideoEffectsItemProps): JSX.Element =>
         <DefaultButton
           styles={containerStyles()}
           onClick={disabled ? undefined : () => props.onSelect?.(props.itemKey)}
-          componentRef={componentRef}
-          autoFocus={props.focusOnMount}
+          componentRef={props.componentRef as React.RefObject<IButton>}
         >
           <Stack horizontalAlign={'center'}>
             {props.iconProps && (

packages/react-composites/src/composites/CallComposite/components/SidePane/useVideoEffectsPane.tsx

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import React, { useCallback, useEffect, useMemo } from 'react';
+import React, { createRef, useCallback, useEffect, useMemo } from 'react';
 import { SidePaneRenderer, useIsParticularSidePaneOpen } from './SidePaneProvider';
 import { SidePaneHeader } from '../../../common/SidePaneHeader';
 
@@ -52,6 +52,7 @@ export const useVideoEffectsPane = (
   }, [closePane, locale.strings, mobileView]);
 
   const latestVideoEffectError = latestErrors.find((error) => error.type === 'unableToStartVideoEffect');
+  const updateFocusHandle = useMemo(() => createRef<{ focus: () => void }>(), []);
 
   const onRenderContent = useCallback((): JSX.Element => {
     return (
@@ -63,9 +64,10 @@ export const useVideoEffectsPane = (
 
           latestVideoEffectError && onDismissError?.(latestVideoEffectError);
         }}
+        updateFocusHandle={updateFocusHandle}
       />
     );
-  }, [latestVideoEffectError, onDismissError]);
+  }, [latestVideoEffectError, onDismissError, updateFocusHandle]);
 
   const sidePaneRenderer: SidePaneRenderer = useMemo(
     () => ({
@@ -78,7 +80,10 @@ export const useVideoEffectsPane = (
 
   const openPane = useCallback(() => {
     updateSidePaneRenderer(sidePaneRenderer);
-  }, [sidePaneRenderer, updateSidePaneRenderer]);
+
+    // Run in a setTimeout as it must be called only once the imperative handle is available
+    setTimeout(() => updateFocusHandle.current?.focus(), 0);
+  }, [sidePaneRenderer, updateSidePaneRenderer, updateFocusHandle]);
 
   const isOpen = useIsParticularSidePaneOpen(VIDEO_EFFECTS_SIDE_PANE_ID);
 

packages/react-composites/src/composites/common/VideoEffectsPane.tsx

@@ -37,6 +37,9 @@ export const VideoEffectsPaneContent = (props: {
   activeVideoEffectError?: ActiveErrorMessage;
   onDismissError: (error: ActiveErrorMessage) => void;
   activeVideoEffectChange: (effect: ActiveVideoEffect) => void;
+  updateFocusHandle: React.RefObject<{
+    focus: () => void;
+  }>;
 }): JSX.Element => {
   const {
     onDismissError,
@@ -140,18 +143,21 @@ export const VideoEffectsPaneContent = (props: {
     };
     adapter.updateSelectedVideoBackgroundEffect(noneEffect);
   }
+
   return VideoEffectsPaneTrampoline(
     onDismissError,
+    props.updateFocusHandle,
     activeVideoEffectError,
-
     selectableVideoEffects,
-
     onEffectChange
   );
 };
 
 const VideoEffectsPaneTrampoline = (
   onDismissError: (error: ActiveErrorMessage) => void,
+  updateFocusHandle: React.RefObject<{
+    focus: () => void;
+  }>,
   activeVideoEffectError?: ActiveErrorMessage,
   selectableVideoEffects?: _VideoEffectsItemProps[],
   onEffectChange?: (effectKey: string) => Promise<void>
@@ -182,6 +188,7 @@ const VideoEffectsPaneTrampoline = (
         options={selectableVideoEffects ?? []}
         onChange={onEffectChange}
         selectedEffectKey={selectedEffect}
+        componentRef={updateFocusHandle}
       />
     </Stack>
   );