chore(react-tree): stop unnecessary re-rendering when no actions are available

change/@fluentui-react-tree-b6e16366-ba82-402d-b48d-bf2d6449bb78.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "chore: stop unnecessary re-rendering when no actions are available",
+  "packageName": "@fluentui/react-tree",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/react-components/react-tree/etc/react-tree.api.md

@@ -177,6 +177,7 @@ export type TreeItemContextValue = {
     expandIconRef: React_2.Ref<HTMLDivElement>;
     layoutRef: React_2.Ref<HTMLDivElement>;
     subtreeRef: React_2.Ref<HTMLDivElement>;
+    treeItemRef?: React_2.RefObject<HTMLDivElement>;
     itemType: TreeItemType;
     value: TreeItemValue;
     open: boolean;

packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx

@@ -3,10 +3,10 @@ import * as ReactDOM from 'react-dom';
 import {
   getIntrinsicElementProps,
   useId,
-  useMergedRefs,
   useEventCallback,
   slot,
   elementContains,
+  useMergedRefs,
 } from '@fluentui/react-utilities';
 import type { TreeItemProps, TreeItemState, TreeItemValue } from './TreeItem.types';
 import { Space } from '@fluentui/keyboard-keys';
@@ -51,18 +51,12 @@ export function useTreeItem_unstable(props: TreeItemProps, ref: React.Ref<HTMLDi
     ...rest
   } = props;
 
-  const [isActionsVisible, setActionsVisible] = React.useState(false);
-  const [isAsideVisible, setAsideVisible] = React.useState(true);
-
-  const handleActionsRef = React.useCallback((actionsElement: HTMLDivElement | null) => {
-    setAsideVisible(actionsElement === null);
-  }, []);
-
   const actionsRef = React.useRef<HTMLDivElement>(null);
   const expandIconRef = React.useRef<HTMLDivElement>(null);
   const layoutRef = React.useRef<HTMLDivElement>(null);
   const subtreeRef = React.useRef<HTMLDivElement>(null);
   const selectionRef = React.useRef<HTMLInputElement>(null);
+  const treeItemRef = React.useRef<HTMLDivElement>(null);
 
   const open = useTreeContext_unstable(ctx => props.open ?? ctx.openItems.has(value));
   const selectionMode = useTreeContext_unstable(ctx => ctx.selectionMode);
@@ -200,51 +194,6 @@ export function useTreeItem_unstable(props: TreeItemProps, ref: React.Ref<HTMLDi
     }
   });
 
-  const setActionsVisibleIfNotFromSubtree = React.useCallback((event: React.SyntheticEvent<HTMLDivElement>) => {
-    const isTargetFromSubtree = Boolean(
-      subtreeRef.current && elementContains(subtreeRef.current, event.target as Node),
-    );
-    if (!isTargetFromSubtree) {
-      setActionsVisible(true);
-    }
-  }, []);
-  const setActionsInvisibleIfNotFromSubtree = React.useCallback(
-    (event: React.MouseEvent<HTMLDivElement> | React.FocusEvent<HTMLDivElement>) => {
-      const isTargetFromSubtree = Boolean(
-        subtreeRef.current && elementContains(subtreeRef.current, event.target as Node),
-      );
-      const isRelatedTargetFromActions = Boolean(
-        actionsRef.current && elementContains(actionsRef.current, event.relatedTarget as Node),
-      );
-      if (isRelatedTargetFromActions) {
-        return setActionsVisible(true);
-      }
-      if (!isTargetFromSubtree) {
-        return setActionsVisible(false);
-      }
-    },
-    [],
-  );
-
-  const handleMouseOver = useEventCallback((event: React.MouseEvent<HTMLDivElement>) => {
-    onMouseOver?.(event);
-    setActionsVisibleIfNotFromSubtree(event);
-  });
-
-  const handleFocus = useEventCallback((event: React.FocusEvent<HTMLDivElement>) => {
-    onFocus?.(event);
-    setActionsVisibleIfNotFromSubtree(event);
-  });
-
-  const handleMouseOut = useEventCallback((event: React.MouseEvent<HTMLDivElement>) => {
-    onMouseOut?.(event);
-    setActionsInvisibleIfNotFromSubtree(event);
-  });
-  const handleBlur = useEventCallback((event: React.FocusEvent<HTMLDivElement>) => {
-    onBlur?.(event);
-    setActionsInvisibleIfNotFromSubtree(event);
-  });
-
   const handleChange = useEventCallback((event: React.ChangeEvent<HTMLInputElement>) => {
     onChange?.(event);
     if (event.isDefaultPrevented()) {
@@ -273,20 +222,23 @@ export function useTreeItem_unstable(props: TreeItemProps, ref: React.Ref<HTMLDi
     layoutRef,
     selectionRef,
     expandIconRef,
-    actionsRef: useMergedRefs(handleActionsRef, actionsRef),
+    treeItemRef,
+    actionsRef,
     itemType,
     level,
     components: {
       root: 'div',
     },
-    isAsideVisible,
-    isActionsVisible,
+    // FIXME: this property is not necessary anymore, but as removing it would be a breaking change, we need to keep it as false
+    isAsideVisible: false,
+    // FIXME: this property is not necessary anymore, but as removing it would be a breaking change, we need to keep it as false
+    isActionsVisible: false,
     root: slot.always(
       getIntrinsicElementProps(as, {
         tabIndex: -1,
         [dataTreeItemValueAttrName]: value,
         ...rest,
-        ref,
+        ref: useMergedRefs(ref, treeItemRef),
         role: 'treeitem',
         'aria-level': level,
         'aria-checked': selectionMode === 'multiselect' ? checked : undefined,
@@ -297,10 +249,6 @@ export function useTreeItem_unstable(props: TreeItemProps, ref: React.Ref<HTMLDi
         'aria-expanded': itemType === 'branch' ? open : undefined,
         onClick: handleClick,
         onKeyDown: handleKeyDown,
-        onMouseOver: handleMouseOver,
-        onFocus: handleFocus,
-        onMouseOut: handleMouseOut,
-        onBlur: handleBlur,
         onChange: handleChange,
       } as const),
       { elementType: 'div' },

packages/react-components/react-tree/src/components/TreeItem/useTreeItemContextValues.ts

@@ -10,7 +10,10 @@ export function useTreeItemContextValues_unstable(state: TreeItemState): TreeIte
     open,
     expandIconRef,
     actionsRef,
+    treeItemRef,
+    // eslint-disable-next-line deprecation/deprecation
     isActionsVisible,
+    // eslint-disable-next-line deprecation/deprecation
     isAsideVisible,
     selectionRef,
     checked,
@@ -31,6 +34,7 @@ export function useTreeItemContextValues_unstable(state: TreeItemState): TreeIte
     isActionsVisible,
     isAsideVisible,
     actionsRef,
+    treeItemRef,
     expandIconRef,
   };
 

packages/react-components/react-tree/src/components/TreeItemLayout/TreeItemLayout.test.tsx

@@ -13,6 +13,7 @@ const Wrapper: React.FC = ({ children }) => (
       subtreeRef: React.createRef(),
       actionsRef: React.createRef(),
       expandIconRef: React.createRef(),
+      treeItemRef: React.createRef(),
       isActionsVisible: true,
       isAsideVisible: true,
       itemType: 'leaf',
@@ -29,6 +30,11 @@ describe('TreeItemLayout', () => {
     Component: TreeItemLayout,
     renderOptions: { wrapper: Wrapper },
     displayName: 'TreeItemLayout',
+    disabledTests: [
+      // This is disabled as aside and actions cannot be visible at the same time
+      'component-has-static-classnames',
+      'component-has-static-classnames-object',
+    ],
     requiredProps: {
       iconAfter: 'iconAfter',
       iconBefore: 'iconBefore',
@@ -39,8 +45,6 @@ describe('TreeItemLayout', () => {
     },
   });
 
-  // TODO add more tests here, and create visual regression tests in /apps/vr-tests
-
   it('renders a default state', () => {
     const result = render(<TreeItemLayout>Default TreeItemLayout</TreeItemLayout>);
     expect(result.container).toMatchSnapshot();

packages/react-components/react-tree/src/components/TreeItemLayout/useTreeItemLayout.tsx

@@ -1,10 +1,12 @@
 import * as React from 'react';
 import {
-  ExtractSlotProps,
   getIntrinsicElementProps,
   isResolvedShorthand,
   useMergedRefs,
   slot,
+  useEventCallback,
+  elementContains,
+  useControllableState,
 } from '@fluentui/react-utilities';
 import { useTreeItemContext_unstable, useTreeContext_unstable } from '../../contexts';
 import type { TreeItemLayoutProps, TreeItemLayoutSlots, TreeItemLayoutState } from './TreeItemLayout.types';
@@ -31,20 +33,63 @@ export const useTreeItemLayout_unstable = (
   const layoutRef = useTreeItemContext_unstable(ctx => ctx.layoutRef);
   const selectionMode = useTreeContext_unstable(ctx => ctx.selectionMode);
 
-  const [isActionsVisibleExternal, actionsShorthand]: [boolean | undefined, TreeItemLayoutSlots['actions']] =
+  const [isActionsVisibleFromProps, actionsShorthand]: [boolean | undefined, TreeItemLayoutSlots['actions']] =
     isResolvedShorthand(props.actions)
       ? // .visible prop should not be propagated to the DOM
         [props.actions.visible, { ...props.actions, visible: undefined }]
       : [undefined, props.actions];
 
-  const isActionsVisible = useTreeItemContext_unstable(ctx => ctx.isActionsVisible) || isActionsVisibleExternal;
-  const isAsideVisible = useTreeItemContext_unstable(ctx => ctx.isAsideVisible);
+  const [isActionsVisible, setIsActionsVisible] = useControllableState({
+    state: isActionsVisibleFromProps,
+    initialState: false,
+  });
+
   const selectionRef = useTreeItemContext_unstable(ctx => ctx.selectionRef);
   const expandIconRef = useTreeItemContext_unstable(ctx => ctx.expandIconRef);
   const actionsRef = useTreeItemContext_unstable(ctx => ctx.actionsRef);
+  const actionsRefInternal = React.useRef<HTMLDivElement>(null);
+  const treeItemRef = useTreeItemContext_unstable(ctx => ctx.treeItemRef);
+  const subtreeRef = useTreeItemContext_unstable(ctx => ctx.subtreeRef);
   const checked = useTreeItemContext_unstable(ctx => ctx.checked);
   const isBranch = useTreeItemContext_unstable(ctx => ctx.itemType === 'branch');
 
+  // FIXME: Asserting is required here, as converting this to RefObject on context type would be a breaking change
+  assertIsRefObject(treeItemRef);
+  // FIXME: Asserting is required here, as converting this to RefObject on context type would be a breaking change
+  assertIsRefObject(subtreeRef);
+
+  const setActionsVisibleIfNotFromSubtree = React.useCallback(
+    (event: MouseEvent | FocusEvent) => {
+      const isTargetFromSubtree = Boolean(
+        subtreeRef.current && elementContains(subtreeRef.current, event.target as Node),
+      );
+      if (!isTargetFromSubtree) {
+        setIsActionsVisible(true);
+      }
+    },
+    [subtreeRef, setIsActionsVisible],
+  );
+
+  const setActionsInvisibleIfNotFromSubtree = React.useCallback(
+    (event: FocusEvent | MouseEvent) => {
+      const isRelatedTargetFromActions = Boolean(
+        actionsRefInternal.current && elementContains(actionsRefInternal.current, event.relatedTarget as Node),
+      );
+      if (isRelatedTargetFromActions) {
+        setIsActionsVisible(true);
+        return;
+      }
+      const isTargetFromSubtree = Boolean(
+        subtreeRef.current && elementContains(subtreeRef.current, event.target as Node),
+      );
+      if (!isTargetFromSubtree) {
+        setIsActionsVisible(false);
+        return;
+      }
+    },
+    [subtreeRef, setIsActionsVisible],
+  );
+
   const expandIcon = slot.optional(props.expandIcon, {
     renderByDefault: isBranch,
     defaultProps: {
@@ -60,14 +105,54 @@ export const useTreeItemLayout_unstable = (
   const arrowNavigationProps = useArrowNavigationGroup({ circular: true, axis: 'horizontal' });
   const actions = isActionsVisible
     ? slot.optional(actionsShorthand, {
-        defaultProps: { ...arrowNavigationProps, role: 'toolbar' } as ExtractSlotProps<TreeItemLayoutSlots['actions']>,
+        defaultProps: { ...arrowNavigationProps, role: 'toolbar' },
         elementType: 'div',
       })
     : undefined;
-  const actionsRefs = useMergedRefs(actions?.ref, actionsRef);
+  const actionsRefs = useMergedRefs(actions?.ref, actionsRef, actionsRefInternal);
+  const handleActionsBlur = useEventCallback((event: React.FocusEvent<HTMLDivElement>) => {
+    if (isResolvedShorthand(actionsShorthand)) {
+      actionsShorthand.onBlur?.(event);
+    }
+    const isRelatedTargetFromActions = Boolean(elementContains(event.currentTarget, event.relatedTarget as Node));
+    setIsActionsVisible(isRelatedTargetFromActions);
+  });
   if (actions) {
     actions.ref = actionsRefs;
+    actions.onBlur = handleActionsBlur;
   }
+
+  const hasActions = Boolean(actionsShorthand);
+
+  React.useEffect(() => {
+    if (treeItemRef.current && hasActions && isActionsVisibleFromProps === undefined) {
+      const treeItemElement = treeItemRef.current;
+
+      const handleMouseOver = setActionsVisibleIfNotFromSubtree;
+      const handleMouseOut = setActionsInvisibleIfNotFromSubtree;
+      const handleFocus = setActionsVisibleIfNotFromSubtree;
+      const handleBlur = setActionsInvisibleIfNotFromSubtree;
+
+      treeItemElement.addEventListener('mouseover', handleMouseOver);
+      treeItemElement.addEventListener('mouseout', handleMouseOut);
+      treeItemElement.addEventListener('focus', handleFocus);
+      treeItemElement.addEventListener('blur', handleBlur);
+
+      return () => {
+        treeItemElement.removeEventListener('mouseover', handleMouseOver);
+        treeItemElement.removeEventListener('mouseout', handleMouseOut);
+        treeItemElement.removeEventListener('focus', handleFocus);
+        treeItemElement.removeEventListener('blur', handleBlur);
+      };
+    }
+  }, [
+    hasActions,
+    treeItemRef,
+    isActionsVisibleFromProps,
+    setActionsVisibleIfNotFromSubtree,
+    setActionsInvisibleIfNotFromSubtree,
+  ]);
+
   return {
     components: {
       root: 'div',
@@ -96,7 +181,7 @@ export const useTreeItemLayout_unstable = (
     iconBefore: slot.optional(iconBefore, { defaultProps: { 'aria-hidden': true }, elementType: 'div' }),
     main: slot.always(main, { elementType: 'div' }),
     iconAfter: slot.optional(iconAfter, { defaultProps: { 'aria-hidden': true }, elementType: 'div' }),
-    aside: isAsideVisible
+    aside: !isActionsVisible
       ? slot.optional(props.aside, { defaultProps: { 'aria-hidden': true }, elementType: 'div' })
       : undefined,
     actions,
@@ -118,3 +203,14 @@ export const useTreeItemLayout_unstable = (
     }),
   };
 };
+
+function assertIsRefObject<Value>(ref?: React.Ref<Value>): asserts ref is React.RefObject<Value> {
+  if (process.env.NODE_ENV !== 'production') {
+    if (typeof ref !== 'object' || ref === null || !('current' in ref)) {
+      throw new Error(`
+        @fluentui/react-tree [${useTreeItemLayout_unstable.name}]:
+        Internal Error: contextual ref is not a RefObject! Please report this bug immediately, as contextual refs should be RefObjects.
+      `);
+    }
+  }
+}

packages/react-components/react-tree/src/components/TreeItemPersonaLayout/TreeItemPersonaLayout.test.tsx

@@ -13,6 +13,7 @@ const Wrapper: React.FC = ({ children }) => (
       subtreeRef: React.createRef(),
       actionsRef: React.createRef(),
       expandIconRef: React.createRef(),
+      treeItemRef: React.createRef(),
       isActionsVisible: true,
       isAsideVisible: true,
       itemType: 'leaf',
@@ -29,6 +30,11 @@ describe('TreeItemPersonaLayout', () => {
     Component: TreeItemPersonaLayout,
     renderOptions: { wrapper: Wrapper },
     displayName: 'TreeItemPersonaLayout',
+    disabledTests: [
+      // This is disabled as aside and actions cannot be visible at the same time
+      'component-has-static-classnames',
+      'component-has-static-classnames-object',
+    ],
     requiredProps: {
       description: 'description',
       expandIcon: 'expandIcon',