fix(core): call onEnter/onExit when using useFocusContainer

Author: mlaursen

packages/core/src/dialog/Dialog.tsx

@@ -287,13 +287,21 @@ export const Dialog = forwardRef<HTMLDivElement, DialogProps>(
     const { eventHandlers, transitionOptions } = useFocusContainer({
       nodeRef: ref,
       activate: visible,
+      onEnter(appearing) {
+        onEnter(appearing);
+        setChildVisible(type !== "full-page");
+      },
       onEntered(appear) {
         onEntered(appear);
         // this needs to be called onEnter and onEntered just in case the
         // transition is disabled
         setChildVisible(type !== "full-page");
       },
       onEntering,
+      onExit() {
+        onExit();
+        setChildVisible(false);
+      },
       onExiting,
       onExited() {
         onExited();
@@ -334,14 +342,6 @@ export const Dialog = forwardRef<HTMLDivElement, DialogProps>(
       appear: appear && !disableTransition && !ssr,
       enter: enter && !disableTransition,
       exit: exit && !disableTransition,
-      onEnter(appearing) {
-        onEnter(appearing);
-        setChildVisible(type !== "full-page");
-      },
-      onExit() {
-        onExit();
-        setChildVisible(false);
-      },
       temporary,
       exitedHidden,
       disablePortal: propDisablePortal,

packages/core/src/focus/__tests__/useFocusContainer.tsx

@@ -70,29 +70,37 @@ describe("useFocusContainer", () => {
     TRANSITION_CONFIG.disabled = false;
 
     const user = userEvent.setup();
+    const onEnter = jest.fn();
     const onEntering = jest.fn();
     const onEntered = jest.fn();
+    const onExit = jest.fn();
     const onExiting = jest.fn();
     const onExited = jest.fn();
     rmdRender(
       <Test
+        onEnter={onEnter}
         onEntering={onEntering}
         onEntered={onEntered}
+        onExit={onExit}
         onExiting={onExiting}
         onExited={onExited}
       />
     );
 
+    expect(onEnter).not.toHaveBeenCalled();
     expect(onEntering).not.toHaveBeenCalled();
     expect(onEntered).not.toHaveBeenCalled();
+    expect(onExit).not.toHaveBeenCalled();
     expect(onExiting).not.toHaveBeenCalled();
     expect(onExited).not.toHaveBeenCalled();
 
     await user.click(screen.getByRole("button", { name: "Show" }));
     await waitFor(() => {
       expect(onEntering).toHaveBeenCalled();
     });
+    expect(onEnter).toHaveBeenCalled();
     expect(onEntered).not.toHaveBeenCalled();
+    expect(onExit).not.toHaveBeenCalled();
     expect(onExiting).not.toHaveBeenCalled();
     expect(onExited).not.toHaveBeenCalled();
 
@@ -101,8 +109,10 @@ describe("useFocusContainer", () => {
     });
 
     await user.click(screen.getByRole("button", { name: "Button 1" }));
+    expect(onEnter).toHaveBeenCalled();
     expect(onEntering).toHaveBeenCalled();
     expect(onEntered).toHaveBeenCalled();
+    expect(onExit).toHaveBeenCalled();
     expect(onExiting).toHaveBeenCalled();
     expect(onExited).not.toHaveBeenCalled();
 

packages/core/src/focus/useFocusContainer.ts

@@ -39,15 +39,19 @@ const noop = (): void => {
  */
 export type FocusType = "mount" | "unmount" | "keyboard";
 
-/** @since 6.0.0 */
-export type FocusContainerTransitionCallbacks = Pick<
-  TransitionCallbacks,
-  "onEntering" | "onEntered" | "onExiting" | "onExited"
->;
+/**
+ * @since 6.0.0
+ * @deprecated Use `TransitionCallbacks` instead.
+ */
+export type FocusContainerTransitionCallbacks = TransitionCallbacks;
 
-/** @since 6.0.0 */
+/**
+ * @since 6.0.0
+ * @since 6.3.2 Fixed by extending `TransitionCallbacks` after the
+ * `onEnteredOnce` and `onExitedOnce` support was added to CSS transitions.
+ */
 export interface FocusContainerTransitionOptions<E extends HTMLElement>
-  extends FocusContainerTransitionCallbacks {
+  extends TransitionCallbacks {
   /**
    * An optional ref that will be merged with the
    * {@link FocusContainerImplementation.nodeRef}
@@ -155,10 +159,12 @@ export function useFocusContainer<E extends HTMLElement>(
   const {
     nodeRef,
     activate,
-    onEntering = noop,
-    onEntered = noop,
-    onExiting = noop,
-    onExited = noop,
+    onEnter,
+    onEntering,
+    onEntered,
+    onExit,
+    onExiting,
+    onExited,
     onKeyDown = noop,
     programmatic = false,
     disableTransition = false,
@@ -181,6 +187,7 @@ export function useFocusContainer<E extends HTMLElement>(
     transitionOptions: {
       nodeRef: refCallback,
       ...getTransitionCallbacks({
+        onEnter,
         onEnterOnce: () => {
           const instance = ref.current;
           if (
@@ -205,6 +212,7 @@ export function useFocusContainer<E extends HTMLElement>(
             prevFocus.current?.focus();
           });
         },
+        onExit,
         onExiting,
         onExited,
         disableTransition,

packages/core/src/menu/Menu.tsx

@@ -420,6 +420,7 @@ export const Menu = forwardRef<HTMLDivElement, LabelRequiredForA11y<MenuProps>>(
             break;
         }
       },
+      onEnter,
       onEntering(appearing) {
         onEntering(appearing);
         entered.current = true;
@@ -430,11 +431,12 @@ export const Menu = forwardRef<HTMLDivElement, LabelRequiredForA11y<MenuProps>>(
         cancelUnmountFocus.current = false;
         animatedOnceRef.current = true;
       },
+      onExit,
+      onExiting,
       onExited() {
         onExited();
         entered.current = false;
       },
-      onExiting,
       disableTransition,
       isFocusTypeDisabled(type) {
         if (role === "listbox") {
@@ -461,7 +463,6 @@ export const Menu = forwardRef<HTMLDivElement, LabelRequiredForA11y<MenuProps>>(
     const { ref, style, callbacks, updateStyle } = useFixedPositioning({
       ...transitionOptions,
       disabled: disableFixedPositioning,
-      onEnter,
       style: isSheet ? propStyle : menuStyle,
       fixedTo,
       anchor: getDefaultAnchor({
@@ -491,7 +492,6 @@ export const Menu = forwardRef<HTMLDivElement, LabelRequiredForA11y<MenuProps>>(
       },
     });
     const { rendered, disablePortal, elementProps } = useScaleTransition({
-      nodeRef: ref,
       className: cnb(!isSheet && menuClassName, className),
       transitionIn: visible,
       vertical: !horizontal,
@@ -501,10 +501,12 @@ export const Menu = forwardRef<HTMLDivElement, LabelRequiredForA11y<MenuProps>>(
       appear,
       enter,
       exit,
-      onExit,
-      onExiting: transitionOptions.onExiting,
       exitedHidden: true,
+      // merge the transition callbacks
+      ...transitionOptions,
       ...callbacks,
+      // but prefer the latest defined ref
+      nodeRef: ref,
     });
     useScrollLock(visible && preventScroll);