Feedback from Austin

Author: Eric Olkowski

packages/react-core/src/components/Alert/AlertGroupInline.tsx

@@ -19,6 +19,24 @@ export const AlertGroupInline: React.FunctionComponent<AlertGroupProps> = ({
   const updateTransitionEnd = (onTransitionEnd: () => void) => {
     setHandleTransitionEnd(() => onTransitionEnd);
   };
+
+  const onTransitionEnd = (event: React.TransitionEvent<HTMLLIElement>) => {
+    if (!hasAnimations) {
+      return;
+    }
+
+    const prefersReducedMotion = !window.matchMedia('(prefers-reduced-motion: no-preference)')?.matches;
+    if (
+      // If a user has no motion preference, we want to target the grid template rows transition
+      // so that the onClose is called after the "slide up" animation of other alerts finishes.
+      // If they have motion preference, we don't need to check for a specific transition since only opacity should fire.
+      (prefersReducedMotion || (!prefersReducedMotion && event.propertyName === 'grid-template-rows')) &&
+      (event.target as HTMLElement).className.includes(styles.modifiers.offstageRight)
+    ) {
+      handleTransitionEnd();
+    }
+  };
+
   return (
     <AlertGroupContext.Provider value={{ hasAnimations, updateTransitionEnd }}>
       <ul
@@ -30,22 +48,7 @@ export const AlertGroupInline: React.FunctionComponent<AlertGroupProps> = ({
       >
         {Children.toArray(children).map((alert, index) => (
           <li
-            onTransitionEnd={(event: React.TransitionEvent<HTMLLIElement>) => {
-              if (!hasAnimations) {
-                return;
-              }
-
-              const prefersReducedMotion = !window.matchMedia('(prefers-reduced-motion: no-preference)')?.matches;
-              if (
-                // If a user has no motion preference, we want to target the grid template rows transition
-                // so that the onClose is called after the "slide up" animation of other alerts finishes.
-                // If they have motion preference, we don't need to check for a specific transition since only opacity should fire.
-                (prefersReducedMotion || (!prefersReducedMotion && event.propertyName === 'grid-template-rows')) &&
-                (event.target as HTMLElement).className.includes(styles.modifiers.offstageRight)
-              ) {
-                handleTransitionEnd();
-              }
-            }}
+            onTransitionEnd={onTransitionEnd}
             className={css(styles.alertGroupItem, hasAnimations && styles.modifiers.offstageTop)}
             key={
               (alert as React.ReactElement<AlertProps>).props?.id ||

packages/react-core/src/components/Alert/__tests__/AlertActionCloseButton.test.tsx

@@ -59,8 +59,8 @@ test('Does not call the callback provided via onClose when it is not clicked', (
 });
 
 test('Calls the callback provided via onClose when clicked', async () => {
-  const onCloseMock = jest.fn();
   const user = userEvent.setup();
+  const onCloseMock = jest.fn();
 
   render(
     <AlertContext.Provider value={{ title: 'title', variantLabel: 'variantLabel' }}>
@@ -74,8 +74,8 @@ test('Calls the callback provided via onClose when clicked', async () => {
 });
 
 test('Calls updateTransitionEnd with onClose when animations are enabled', async () => {
-  const onClose = jest.fn();
   const user = userEvent.setup();
+  const onClose = jest.fn();
   const updateMock = jest.fn();
   render(
     <AlertGroupContext.Provider value={{ hasAnimations: true, updateTransitionEnd: updateMock }}>
@@ -85,6 +85,7 @@ test('Calls updateTransitionEnd with onClose when animations are enabled', async
     </AlertGroupContext.Provider>
   );
 
+  expect(updateMock).not.toHaveBeenCalled();
   await user.click(screen.getByRole('button'));
   expect(updateMock).toHaveBeenCalledWith(onClose);
   expect(updateMock).toHaveBeenCalledTimes(1);

packages/react-core/src/components/Alert/__tests__/AlertGroup.test.tsx

@@ -71,11 +71,11 @@ test('Calls the callback set by updateTransitionEnd when transition ends and ani
     matches: false,
     media: query,
     onchange: null,
-    addListener: jest.fn(), // deprecated
-    removeListener: jest.fn(), // deprecated
-    addEventListener: jest.fn(),
-    removeEventListener: jest.fn(),
-    dispatchEvent: jest.fn()
+    addListener: () => {}, // deprecated
+    removeListener: () => {}, // deprecated
+    addEventListener: () => {},
+    removeEventListener: () => {},
+    dispatchEvent: () => true
   });
   const mockCallback = jest.fn();
   const user = userEvent.setup();
@@ -101,11 +101,11 @@ test('Does not call the callback set by updateTransitionEnd when transition ends
     matches: false,
     media: query,
     onchange: null,
-    addListener: jest.fn(), // deprecated
-    removeListener: jest.fn(), // deprecated
-    addEventListener: jest.fn(),
-    removeEventListener: jest.fn(),
-    dispatchEvent: jest.fn()
+    addListener: () => {}, // deprecated
+    removeListener: () => {}, // deprecated
+    addEventListener: () => {},
+    removeEventListener: () => {},
+    dispatchEvent: () => true
   });
   const mockCallback = jest.fn();
   const user = userEvent.setup();