feat(AnimationsProvider): Add Support for AnimationsProvider #11990

jenny-s51 · 2025-09-04T14:49:18Z

What: Closes #11962

This PR enables support for AnimationsProvider.

PatternFly consumers will now be able to set animations for opt-in components once at the app level (instead of on every component via hasAnimations) while maintaining full backward compatibility.

Testing Instructions

To test the new context provider functionality:

Run yarn start and navigate to http://localhost:8002/components/alert#toast-alert-group

Copy and paste the code below into the demo TSX code:

Click to expand

 // eslint-disable-next-line no-restricted-imports
 import React, { Fragment, useState } from 'react';
 import {
   Alert,
   AlertProps,
   AlertGroup,
   AlertActionCloseButton,
   AlertVariant,
   InputGroup,
   InputGroupItem
 } from '@patternfly/react-core';
 import buttonStyles from '@patternfly/react-styles/css/components/Button/button';
 import { AnimationsProvider } from '@patternfly/react-core/dist/esm/helpers';
 export const AlertGroupToast: React.FunctionComponent = () => {
   const [alerts, setAlerts] = useState<Partial<AlertProps>[]>([]);
 
   const addAlert = (title: string, variant: AlertProps['variant'], key: React.Key) => {
     setAlerts((prevAlerts) => [{ title, variant, key }, ...prevAlerts]);
   };
 
   const removeAlert = (key: React.Key) => {
     setAlerts((prevAlerts) => [...prevAlerts.filter((alert) => alert.key !== key)]);
   };
 
   const btnClasses = [buttonStyles.button, buttonStyles.modifiers.secondary].join(' ');
 
   const getUniqueId = () => new Date().getTime();
 
   const addSuccessAlert = () => {
     addAlert('Toast success alert', 'success', getUniqueId());
   };
 
   const addDangerAlert = () => {
     addAlert('Toast danger alert', 'danger', getUniqueId());
   };
 
   const addInfoAlert = () => {
     addAlert('Toast info alert', 'info', getUniqueId());
   };
 
   return (
     <Fragment>
       <InputGroup style={{ marginBottom: '16px' }}>
         <InputGroupItem>
           <button onClick={addSuccessAlert} type="button" className={btnClasses}>
             Add toast success alert
           </button>
         </InputGroupItem>
         <InputGroupItem>
           <button onClick={addDangerAlert} type="button" className={btnClasses}>
             Add toast danger alert
           </button>
         </InputGroupItem>
         <InputGroupItem>
           <button onClick={addInfoAlert} type="button" className={btnClasses}>
             Add toast info alert
           </button>
         </InputGroupItem>
       </InputGroup>
       <AnimationsProvider config={{ hasAnimations: true }}>
         <AlertGroup isToast isLiveRegion>
           {alerts.map(({ key, variant, title }) => (
             <Alert
               variant={AlertVariant[variant]}
               title={title}
               actionClose={
                 <AlertActionCloseButton
                   title={title as string}
                   variantLabel={`${variant} alert`}
                   onClose={() => removeAlert(key)}
                 />
               }
               key={key}
             />
           ))}
         </AlertGroup>
       </AnimationsProvider>
     </Fragment>
   );
 };

Verify the animations are applied to the component via the context provider rather than with the hasAnimations prop.

Additional Changes & Fixes

Converted class components to functional components to enable React Hooks usage useHasAnimations hook integration:

AlertGroup.tsx - Required for useHasAnimations hook integration
DualListSelector.tsx - Required for useHasAnimations hook integration

Quick Verification

Go to any example for a component that uses the hasAnimations prop.
Wrap with <AnimationsProvider config={{ hasAnimations: true }}>
Remove the hasAnimations prop from the child component.
Verify animations work as expected.

Testing Note: Since PatternFly example files don't typically import from /helpers, the ESLint configs require a React import with the // eslint-disable-next-line no-restricted-imports comment at the top of the file for local testing. Real consumers won't have this issue.

Success Criteria

✅ Component hasAnimations prop overrides provider when set
✅ Existing examples work unchanged without provider
✅ No console errors or TypeScript issues

patternfly-build · 2025-09-04T14:53:09Z

Preview: https://pf-react-pr-11990.surge.sh

A11y report: https://pf-react-pr-11990-a11y.surge.sh

…ons, add AnimationsProvider to helpers, add md files for documentation fix(components): update hasAnimations to fall back to context fix(md): remove cssPrefix fix(alertgroup): fix tests

wise-king-sullyman

Great work on this! I have a handful of comments/change requests but they're mostly fairly small things.

This review didn't include looking at changing AlertGroup and DualListSelector from class based to function components that was needed as unfortunately I ran out of time. I wouldn't say to redo anything, but just for the sake of the future if a similar situation arises I would advocate for splitting that work into a distinct PR just to make review/testing easier, but that's just my $0.02.

wise-king-sullyman · 2025-09-16T21:05:05Z

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

+  );
+
+  const closeButton = screen.getByLabelText('Close');
+  fireEvent.click(closeButton);


Unless there's a particular reason to need fireEvent we should default to userEvent per the suggestions of the RTL team.

If there is a particular reason fireEvent is needed please add a comment explaining why.

wise-king-sullyman · 2025-09-16T21:10:03Z

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

+
+  // Should call callback on transition end when animations are enabled via context
+  fireEvent.transitionEnd(screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item') as HTMLElement);
+  expect(mockCallback).toHaveBeenCalled();


Instead of checking for the onClose callback to be called/not called would it make more sense to test for animation related classes on the element?

Yes, really good call here - updated

wise-king-sullyman · 2025-09-16T21:14:59Z

packages/react-core/src/components/DualListSelector/DualListSelector.tsx

+  children,
+  id,
+  isTree = false,
+  hasAnimations: localHasAnimations,


Nit: I feel like hasAnimationsProp or something would maybe be a little clearer of a deconstructed name? I don't have strong feelings on that though and it might just be a matter of preference.

We may not be consistent with it, but we do have instances where in this context we've used somethingProp, while for variables inside the component we use localSomething. Again we may not be consistent on this, but I'd probably lean towards using hasAnimationsProp instead here, and reserving localHasAnimations for a variable/state name inside the component code.

@wise-king-sullyman @thatblindgeye thank you, great points, I like hasAnimationsProp here too, it's clearer and follows our existing conventions better - updated it.

wise-king-sullyman · 2025-09-16T21:18:57Z

packages/react-core/src/helpers/AnimationsProvider/AnimationsProvider.tsx

+export interface AnimationsConfig {
+  /** Whether animations are enabled globally */
+  hasAnimations?: boolean;
+}
+
+/** Props for the AnimationsProvider component */
+export interface AnimationsProviderProps {
+  /** Animation configuration settings */
+  config?: AnimationsConfig;


Should these be required? I'm not sure what use case there would be for someone to use the provider without setting a config with a hasAnimations value.

wise-king-sullyman · 2025-09-16T21:28:17Z

packages/react-core/src/helpers/AnimationsProvider/AnimationsProvider.tsx

+
+  // Local prop takes precedence when explicitly set (including false)
+  // If local prop is undefined, fall back to context
+  return localHasAnimations !== undefined ? localHasAnimations : (contextHasAnimations ?? false);


Could this be simplified to just localHasAnimations ?? contextHasAnimations ?? false instead of a ternary with a nullish coalescing operator nested inside it?

wise-king-sullyman · 2025-09-16T21:29:53Z

packages/react-core/src/helpers/AnimationsProvider/__tests__/AnimationsProvider.test.tsx

+  return <div data-testid="has-animations">{hasAnimations ? 'animations-enabled' : 'animations-disabled'}</div>;
+};
+
+describe('AnimationsProvider', () => {


Nit: can you remove this nesting describe block? We default to not using them around whole suites.

Also if you're not aware of our testing styles/guidelines you might want to give that article in our wiki a look 🙂

packages/react-core/src/helpers/AnimationsProvider/__tests__/AnimationsProvider.test.tsx

wise-king-sullyman · 2025-09-16T21:38:06Z

packages/react-core/src/helpers/AnimationsProvider/examples/AnimationsProvider.md

@@ -0,0 +1,161 @@
+---
+id: Animations provider  
+section: helpers


Are we going to be adding a helpers section to the site?

Maybe a topic to bring up in a platform topics meeting @jenny-s51 . I don't believe we really document helpers anywhere on Org currently, and I would have thought communication about this new context would be housed in something like release highlights/notes. A decision on this could mean having to remove the new examples that you added here.

@wise-king-sullyman @thatblindgeye yep great callout, I think it makes sense to delete it here and include this md file as part of patternfly/patternfly-org#4783 - removed it from this PR and will add it there as a follow-up.

wise-king-sullyman · 2025-09-16T21:38:39Z

packages/react-core/src/helpers/AnimationsProvider/examples/AnimationsProviderBasic.tsx

@@ -0,0 +1,69 @@
+import { TreeView } from '@patternfly/react-core';


Is this file being used anywhere?

Nice catch, deleted it

thatblindgeye · 2025-09-17T13:31:44Z

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

 });
+
+// Animation context tests
+test('respects AnimationsProvider context when no local hasAnimations prop', () => {


Kind of the opposite of Austin's comment about regarding a describe block, I personally might wrap these tests in a describe block specifically for the context tests. Not a blocker and depending how quickly we want to get this merged in, would like to hear @wise-king-sullyman opinion here.

I think so too @thatblindgeye - based on the RTL guide this seems like a good use case for a grouped describe() block, I've updated the tests to include it, but will also defer to @wise-king-sullyman here for the final word on whether or not we'd like to keep it.

I'm not against grouping associated things like the animation tests together in a describe block in an otherwise broader test suite, just wrapping full test suites in a describe block 🙂

thatblindgeye · 2025-09-17T13:33:51Z

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

-      target.removeChild(this.state.container);
-    }
-  }
+  appendTo, // do not pass down to ul


Nit: it's pre-existing but I think we could remove this comment now that it's a functional component.

Nice catch, removed

thatblindgeye · 2025-09-17T13:38:05Z

packages/react-core/src/components/DualListSelector/DualListSelector.tsx

+  children,
+  id,
+  isTree = false,
+  hasAnimations: localHasAnimations,


We may not be consistent with it, but we do have instances where in this context we've used somethingProp, while for variables inside the component we use localSomething. Again we may not be consistent on this, but I'd probably lean towards using hasAnimationsProp instead here, and reserving localHasAnimations for a variable/state name inside the component code.

thatblindgeye · 2025-09-17T13:39:08Z

packages/react-core/src/components/SearchInput/SearchInput.tsx

  const [focusAfterExpandChange, setFocusAfterExpandChange] = useState(false);

-  const { isExpanded, onToggleExpand, toggleAriaLabel, hasAnimations } = expandableInput || {};
+  const { isExpanded, onToggleExpand, toggleAriaLabel, hasAnimations: localHasAnimations } = expandableInput || {};


Just as a followup to my above reply to Austin about the prop name, I think this makes sense to call it localHasAnimations.

Just following up here to confirm based on the discussion in the thread here, I renamed it to hasAnimations for both the destructed and internal variable for consistency with our existing conventions for destructured props, for example JumpLinks uses activeIndex: activeIndexProp and isExpanded: isExpandedProp. What are your thoughts on using hasAnimationsProp here?

…e prop to hasAnimationsProp to match existing conventions

jenny-s51 marked this pull request as draft September 4, 2025 14:49

jenny-s51 marked this pull request as ready for review September 4, 2025 16:17

feat(AnimationsProvider): update opt-in components with useHasAnimati…

49bcb04

…ons, add AnimationsProvider to helpers, add md files for documentation fix(components): update hasAnimations to fall back to context fix(md): remove cssPrefix fix(alertgroup): fix tests

jenny-s51 force-pushed the animationContext branch from 855d346 to 49bcb04 Compare September 4, 2025 16:18

chore(AnimationsProvider): Add tests

751cb84

jenny-s51 requested review from thatblindgeye, kmcfaul and wise-king-sullyman September 10, 2025 14:24

jenny-s51 mentioned this pull request Sep 11, 2025

AnimationsProvider - add documentation that AnimationsProvider is now supported patternfly/pf-codemods#856

Open

wise-king-sullyman requested changes Sep 16, 2025

View reviewed changes

thatblindgeye reviewed Sep 17, 2025

View reviewed changes

jenny-s51 added 3 commits September 18, 2025 14:56

feat(AnimationsProvider): update tests, set config to required, renam…

85d2847

…e prop to hasAnimationsProp to match existing conventions

feat(AnimationsProvider): delete AnimationsProvider.md

207bcad

remove comment in AlertGroup for appendTo

c4a3b80

jenny-s51 requested review from wise-king-sullyman and thatblindgeye September 23, 2025 14:59

		@@ -0,0 +1,69 @@
		import { TreeView } from '@patternfly/react-core';

feat(AnimationsProvider): Add Support for AnimationsProvider #11990

Are you sure you want to change the base?

feat(AnimationsProvider): Add Support for AnimationsProvider #11990

Conversation

jenny-s51 commented Sep 4, 2025

Testing Instructions

Additional Changes & Fixes

Quick Verification

Success Criteria

Uh oh!

patternfly-build commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wise-king-sullyman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenny-s51 Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenny-s51 Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented Sep 4, 2025 •

edited

Loading

jenny-s51 Sep 18, 2025 •

edited

Loading

thatblindgeye Sep 17, 2025 •

edited

Loading

jenny-s51 Sep 18, 2025 •

edited

Loading