diff --git a/packages/react-core/src/components/Alert/AlertGroup.tsx b/packages/react-core/src/components/Alert/AlertGroup.tsx index 26028c1e16b..9780c1e5908 100644 --- a/packages/react-core/src/components/Alert/AlertGroup.tsx +++ b/packages/react-core/src/components/Alert/AlertGroup.tsx @@ -1,7 +1,8 @@ -import { Component } from 'react'; +import { useEffect, useRef, useState } from 'react'; import * as ReactDOM from 'react-dom'; import { canUseDOM } from '../../helpers'; import { AlertGroupInline } from './AlertGroupInline'; +import { useHasAnimations } from '../../helpers'; export interface AlertGroupProps extends Omit, 'className'> { /** Additional classes added to the AlertGroup */ @@ -26,78 +27,73 @@ export interface AlertGroupProps extends Omit, 'aria-label'?: string; } -interface AlertGroupState { - container: HTMLElement; -} - -class AlertGroup extends Component { - static displayName = 'AlertGroup'; - state = { - container: undefined - } as AlertGroupState; - - componentDidMount() { - const container = document.createElement('div'); - const target: HTMLElement = this.getTargetElement(); - this.setState({ container }); - target.appendChild(container); - } +export const AlertGroup: React.FunctionComponent = ({ + className, + children, + hasAnimations: hasAnimationsProp, + isToast, + isLiveRegion, + onOverflowClick, + overflowMessage, + 'aria-label': ariaLabel, - componentWillUnmount() { - const target: HTMLElement = this.getTargetElement(); - if (this.state.container) { - target.removeChild(this.state.container); - } - } + appendTo, + ...props +}: AlertGroupProps) => { + const containerRef = useRef(null); + const [isContainerReady, setIsContainerReady] = useState(false); + const hasAnimations = useHasAnimations(hasAnimationsProp); - getTargetElement() { - const appendTo = this.props.appendTo; + const getTargetElement = () => { if (typeof appendTo === 'function') { return appendTo(); } return appendTo || document.body; - } + }; + + useEffect(() => { + if (isToast && canUseDOM) { + const container = document.createElement('div'); + const target = getTargetElement(); + containerRef.current = container; + target.appendChild(container); + setIsContainerReady(true); - render() { - const { - className, - children, - hasAnimations = false, - isToast, - isLiveRegion, - onOverflowClick, - overflowMessage, - 'aria-label': ariaLabel, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - appendTo, // do not pass down to ul - ...props - } = this.props; - const alertGroup = ( - - {children} - - ); - if (!this.props.isToast) { - return alertGroup; + return () => { + if (containerRef.current) { + target.removeChild(containerRef.current); + containerRef.current = null; + } + setIsContainerReady(false); + }; } + }, [isToast, appendTo]); - const container = this.state.container; + const alertGroup = ( + + {children} + + ); - if (!canUseDOM || !container) { - return null; - } + if (!isToast) { + return alertGroup; + } + + const container = containerRef.current; - return ReactDOM.createPortal(alertGroup, container); + if (!canUseDOM || !container || !isContainerReady) { + return null; } -} -export { AlertGroup }; + return ReactDOM.createPortal(alertGroup, container); +}; +AlertGroup.displayName = 'AlertGroup'; diff --git a/packages/react-core/src/components/Alert/AlertGroupInline.tsx b/packages/react-core/src/components/Alert/AlertGroupInline.tsx index 95622162b11..390a78771da 100644 --- a/packages/react-core/src/components/Alert/AlertGroupInline.tsx +++ b/packages/react-core/src/components/Alert/AlertGroupInline.tsx @@ -4,17 +4,19 @@ import styles from '@patternfly/react-styles/css/components/Alert/alert-group'; import { AlertGroupProps } from './AlertGroup'; import { AlertProps } from '../Alert'; import { AlertGroupContext } from './AlertGroupContext'; +import { useHasAnimations } from '../../helpers'; export const AlertGroupInline: React.FunctionComponent = ({ className, children, - hasAnimations, + hasAnimations: hasAnimationsProp, isToast, isLiveRegion, onOverflowClick, overflowMessage, ...props }: AlertGroupProps) => { + const hasAnimations = useHasAnimations(hasAnimationsProp); const [handleTransitionEnd, setHandleTransitionEnd] = useState<() => void>(() => () => {}); const updateTransitionEnd = (onTransitionEnd: () => void) => { diff --git a/packages/react-core/src/components/Alert/__tests__/AlertGroup.test.tsx b/packages/react-core/src/components/Alert/__tests__/AlertGroup.test.tsx index 4b5bade997a..03255da4a9c 100644 --- a/packages/react-core/src/components/Alert/__tests__/AlertGroup.test.tsx +++ b/packages/react-core/src/components/Alert/__tests__/AlertGroup.test.tsx @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event'; import { Alert } from '../../Alert'; import { AlertGroup } from '../../Alert'; import { AlertActionCloseButton } from '../../../components/Alert/AlertActionCloseButton'; +import { AnimationsProvider } from '../../../helpers/AnimationsProvider'; test('Alert Group renders without children', () => { render( @@ -56,14 +57,16 @@ test('Standard Alert Group is not a toast alert group', () => { expect(screen.getByText('alert title').parentElement).not.toHaveClass('pf-m-toast'); }); -test('Toast Alert Group contains expected modifier class', () => { +test('Toast Alert Group contains expected modifier class', async () => { render( ); - expect(screen.getByLabelText('group label')).toHaveClass('pf-m-toast'); + // Wait for the portal to be created and rendered + const alertGroup = await screen.findByLabelText('group label'); + expect(alertGroup).toHaveClass('pf-m-toast'); }); test('Calls the callback set by updateTransitionEnd when transition ends and animations are enabled', async () => { @@ -90,8 +93,11 @@ test('Calls the callback set by updateTransitionEnd when transition ends and ani ); - await user.click(screen.getByLabelText('Close')); + const closeButton = await screen.findByLabelText('Close'); + await user.click(closeButton); expect(mockCallback).not.toHaveBeenCalled(); + // fireEvent is needed here because transitionEnd is a browser event that occurs automatically + // when CSS transitions complete, not a user interaction that userEvent can simulate fireEvent.transitionEnd(screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item') as HTMLElement); expect(mockCallback).toHaveBeenCalled(); }); @@ -120,9 +126,54 @@ test('Does not call the callback set by updateTransitionEnd when transition ends ); - await user.click(screen.getByLabelText('Close')); + const closeButton = await screen.findByLabelText('Close'); + await user.click(closeButton); expect(mockCallback).toHaveBeenCalledTimes(1); // The transitionend event firing should not cause the callback to be called again + // fireEvent is needed here because transitionEnd is a browser event that occurs automatically + // when CSS transitions complete, not a user interaction that userEvent can simulate fireEvent.transitionEnd(screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item') as HTMLElement); expect(mockCallback).toHaveBeenCalledTimes(1); }); + +describe('Animation context behavior', () => { + test('respects AnimationsProvider context when no local hasAnimations prop', () => { + render( + + + + + + ); + + // Should apply animation class when animations are enabled via context + const alertGroupItem = screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item'); + expect(alertGroupItem).toHaveClass('pf-m-offstage-top'); + }); + + test('local hasAnimations prop takes precedence over context', () => { + render( + + + + + + ); + + // Should not apply animation class when local hasAnimations=false overrides context + const alertGroupItem = screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item'); + expect(alertGroupItem).not.toHaveClass('pf-m-offstage-top'); + }); + + test('works without AnimationsProvider (backward compatibility)', () => { + render( + + + + ); + + // Should not apply animation class when no context and no local hasAnimations + const alertGroupItem = screen.getByText('Test Alert').closest('.pf-v6-c-alert-group__item'); + expect(alertGroupItem).not.toHaveClass('pf-m-offstage-top'); + }); +}); diff --git a/packages/react-core/src/components/DualListSelector/DualListSelector.tsx b/packages/react-core/src/components/DualListSelector/DualListSelector.tsx index a8eed78a257..29b6b9d5fd8 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelector.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelector.tsx @@ -1,8 +1,8 @@ -import { Component } from 'react'; import styles from '@patternfly/react-styles/css/components/DualListSelector/dual-list-selector'; import { css } from '@patternfly/react-styles'; -import { GenerateId, PickOptional } from '../../helpers'; +import { GenerateId } from '../../helpers'; import { DualListSelectorContext } from './DualListSelectorContext'; +import { useHasAnimations } from '../../helpers'; /** Acts as a container for all other DualListSelector sub-components when using a * composable dual list selector. @@ -24,41 +24,34 @@ export interface DualListSelectorProps { hasAnimations?: boolean; } -class DualListSelector extends Component { - static displayName = 'DualListSelector'; - static defaultProps: PickOptional = { - children: '', - isTree: false, - hasAnimations: false - }; +export const DualListSelector: React.FunctionComponent = ({ + className, + children, + id, + isTree = false, + hasAnimations: hasAnimationsProp, + ...props +}: DualListSelectorProps) => { + const hasAnimations = useHasAnimations(hasAnimationsProp); - constructor(props: DualListSelectorProps) { - super(props); - } - - render() { - const { className, children, id, isTree, hasAnimations, ...props } = this.props; - - return ( - - - {(randomId) => ( -
- {children} -
- )} -
-
- ); - } -} - -export { DualListSelector }; + return ( + + + {(randomId) => ( +
+ {children} +
+ )} +
+
+ ); +}; +DualListSelector.displayName = 'DualListSelector'; diff --git a/packages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsx b/packages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsx index 94141f9f38e..33c3539090d 100644 --- a/packages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsx +++ b/packages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsx @@ -6,6 +6,7 @@ import { Badge } from '../Badge'; import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon'; import { flattenTree } from './treeUtils'; import { DualListSelectorListContext } from './DualListSelectorContext'; +import { useHasAnimations } from '../../helpers'; export interface DualListSelectorTreeItemProps extends React.HTMLProps { /** Content rendered inside the dual list selector. */ @@ -58,7 +59,7 @@ const DualListSelectorTreeItemBase: React.FunctionComponent { setIsExpanded(defaultExpanded); diff --git a/packages/react-core/src/components/DualListSelector/__tests__/DualListSelector.test.tsx b/packages/react-core/src/components/DualListSelector/__tests__/DualListSelector.test.tsx index 3d59432fac2..3d3601fe8a1 100644 --- a/packages/react-core/src/components/DualListSelector/__tests__/DualListSelector.test.tsx +++ b/packages/react-core/src/components/DualListSelector/__tests__/DualListSelector.test.tsx @@ -3,6 +3,7 @@ import styles from '@patternfly/react-styles/css/components/DualListSelector/dua import { DualListSelector } from '../DualListSelector'; import { DualListSelectorPane } from '../DualListSelectorPane'; import { SearchInput } from '../../SearchInput'; +import { AnimationsProvider } from '../../../helpers/AnimationsProvider'; // The following tests can be removed as part of https://github.com/patternfly/patternfly-react/issues/11838 describe('Opt-in animations', () => { @@ -29,6 +30,33 @@ describe('Opt-in animations', () => { expect(screen.getByTestId('test-id')).toHaveClass(styles.modifiers.animateExpand); }); + + // Animation context tests + test('respects AnimationsProvider context when no local hasAnimations prop', () => { + render( + + + + ); + + expect(screen.getByTestId('test-id')).toHaveClass(styles.modifiers.animateExpand); + }); + + test('local hasAnimations prop takes precedence over context', () => { + render( + + + + ); + + expect(screen.getByTestId('test-id')).not.toHaveClass(styles.modifiers.animateExpand); + }); + + test('works without AnimationsProvider (backward compatibility)', () => { + render(); + + expect(screen.getByTestId('test-id')).not.toHaveClass(styles.modifiers.animateExpand); + }); }); // Following tests should be moved to a separate DualListSelectorPane test file diff --git a/packages/react-core/src/components/Form/FormFieldGroupExpandable.tsx b/packages/react-core/src/components/Form/FormFieldGroupExpandable.tsx index 369a5adf3fc..b5285b5e758 100644 --- a/packages/react-core/src/components/Form/FormFieldGroupExpandable.tsx +++ b/packages/react-core/src/components/Form/FormFieldGroupExpandable.tsx @@ -1,5 +1,6 @@ import { useState } from 'react'; import { InternalFormFieldGroup } from './InternalFormFieldGroup'; +import { useHasAnimations } from '../../helpers'; export interface FormFieldGroupExpandableProps extends Omit, 'onToggle'> { /** Anything that can be rendered as form field group content. */ @@ -25,10 +26,11 @@ export const FormFieldGroupExpandable: React.FunctionComponent { const [localIsExpanded, setIsExpanded] = useState(isExpanded); + const hasAnimations = useHasAnimations(hasAnimationsProp); return ( , 'label' | 'onToggle'> { /** Anything that can be rendered as form field group content. */ @@ -33,9 +34,10 @@ export const InternalFormFieldGroup: React.FunctionComponent { + const hasAnimations = useHasAnimations(hasAnimationsProp); const headerTitleText = header ? header.props.titleText : null; if (isExpandable && !toggleAriaLabel && !headerTitleText) { // eslint-disable-next-line no-console diff --git a/packages/react-core/src/components/Form/__tests__/FormFieldGroupExpandable.test.tsx b/packages/react-core/src/components/Form/__tests__/FormFieldGroupExpandable.test.tsx index bca95890361..b5d96014ff0 100644 --- a/packages/react-core/src/components/Form/__tests__/FormFieldGroupExpandable.test.tsx +++ b/packages/react-core/src/components/Form/__tests__/FormFieldGroupExpandable.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react'; import { FormFieldGroupExpandable } from '../FormFieldGroupExpandable'; +import { AnimationsProvider } from '../../../helpers/AnimationsProvider'; import styles from '@patternfly/react-styles/css/components/Form/form'; test('Does not render children by default', () => { @@ -53,3 +54,32 @@ test(`Renders with class ${styles.modifiers.expandable} when hasAnimations is tr expect(screen.getByRole('group')).toHaveClass(styles.modifiers.expandable); }); + +// Regression tests for AnimationsProvider context +test('respects AnimationsProvider context when no local hasAnimations prop', () => { + render( + + Child content + + ); + + expect(screen.getByRole('group')).toHaveClass(styles.modifiers.expandable); +}); + +test('local hasAnimations prop takes precedence over context', () => { + render( + + + Child content + + + ); + + expect(screen.getByRole('group')).not.toHaveClass(styles.modifiers.expandable); +}); + +test('works without AnimationsProvider (backward compatibility)', () => { + render(Child content); + + expect(screen.getByRole('group')).not.toHaveClass(styles.modifiers.expandable); +}); diff --git a/packages/react-core/src/components/SearchInput/SearchInput.tsx b/packages/react-core/src/components/SearchInput/SearchInput.tsx index ddb9f96a1dd..d61f0ff3f99 100644 --- a/packages/react-core/src/components/SearchInput/SearchInput.tsx +++ b/packages/react-core/src/components/SearchInput/SearchInput.tsx @@ -13,6 +13,7 @@ import { AdvancedSearchMenu } from './AdvancedSearchMenu'; import { TextInputGroup, TextInputGroupMain, TextInputGroupUtilities } from '../TextInputGroup'; import { InputGroup, InputGroupItem } from '../InputGroup'; import { Popper } from '../../helpers'; +import { useHasAnimations } from '../../helpers'; import textInputGroupStyles from '@patternfly/react-styles/css/components/TextInputGroup/text-input-group'; import inputGroupStyles from '@patternfly/react-styles/css/components/InputGroup/input-group'; @@ -180,7 +181,8 @@ const SearchInputBase: React.FunctionComponent = ({ const popperRef = useRef(null); const [focusAfterExpandChange, setFocusAfterExpandChange] = useState(false); - const { isExpanded, onToggleExpand, toggleAriaLabel, hasAnimations } = expandableInput || {}; + const { isExpanded, onToggleExpand, toggleAriaLabel, hasAnimations: hasAnimationsProp } = expandableInput || {}; + const hasAnimations = useHasAnimations(hasAnimationsProp); useEffect(() => { // this effect and the focusAfterExpandChange variable are needed to focus the input/toggle as needed when the diff --git a/packages/react-core/src/components/SearchInput/__tests__/SearchInput.test.tsx b/packages/react-core/src/components/SearchInput/__tests__/SearchInput.test.tsx index 5ffbfd717d8..a545ebe7579 100644 --- a/packages/react-core/src/components/SearchInput/__tests__/SearchInput.test.tsx +++ b/packages/react-core/src/components/SearchInput/__tests__/SearchInput.test.tsx @@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event'; import { SearchInput } from '../SearchInput'; import { FormGroup } from '../../Form'; import { Button } from '../../Button'; +import { AnimationsProvider } from '../../../helpers/AnimationsProvider'; import ExternalLinkSquareAltIcon from '@patternfly/react-icons/dist/esm/icons/external-link-square-alt-icon'; import badgeStyles from '@patternfly/react-styles/css/components/Badge/badge'; import textInputGroupStyles from '@patternfly/react-styles/css/components/TextInputGroup/text-input-group'; @@ -363,3 +364,54 @@ test('Additional props are spread when inputProps prop is passed', () => { render(); expect(screen.getByLabelText('Test input')).toHaveAttribute('autofocus', 'true'); }); + +// Animation context tests +test('respects AnimationsProvider context when no local hasAnimations prop for expandable input', () => { + render( + + {}, + toggleAriaLabel: 'Test label' + }} + /> + + ); + + expect(screen.getByTestId('test-id')).toHaveClass(`${inputGroupStyles.modifiers.searchExpandable}`); +}); + +test('local hasAnimations prop takes precedence over context for expandable input', () => { + render( + + {}, + toggleAriaLabel: 'Test label' + }} + /> + + ); + + expect(screen.getByTestId('test-id')).not.toHaveClass(`${inputGroupStyles.modifiers.searchExpandable}`); +}); + +test('works without AnimationsProvider for expandable input (backward compatibility)', () => { + render( + {}, + toggleAriaLabel: 'Test label' + }} + /> + ); + + expect(screen.getByTestId('test-id')).not.toHaveClass(`${inputGroupStyles.modifiers.searchExpandable}`); +}); diff --git a/packages/react-core/src/components/TreeView/TreeView.tsx b/packages/react-core/src/components/TreeView/TreeView.tsx index 890a86782ba..b6cd5a0c950 100644 --- a/packages/react-core/src/components/TreeView/TreeView.tsx +++ b/packages/react-core/src/components/TreeView/TreeView.tsx @@ -1,6 +1,7 @@ import { TreeViewList } from './TreeViewList'; import { TreeViewCheckProps, TreeViewListItem } from './TreeViewListItem'; import { TreeViewRoot } from './TreeViewRoot'; +import { useHasAnimations } from '../../helpers'; /** Properties that make up a tree view data item. These properties should be passed in as an * object to one of the various tree view component properties which accept TreeViewDataItem as @@ -135,9 +136,10 @@ export const TreeView: React.FunctionComponent = ({ useMemo, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, - hasAnimations, + hasAnimations: hasAnimationsProp, ...props }: TreeViewProps) => { + const hasAnimations = useHasAnimations(hasAnimationsProp); const treeViewList = ( >, 'checked'> { checked?: boolean | null; @@ -102,10 +103,11 @@ const TreeViewListItemBase: React.FunctionComponent = ({ expandedIcon, action, compareItems, - hasAnimations, + hasAnimations: hasAnimationsProp, // eslint-disable-next-line @typescript-eslint/no-unused-vars useMemo }: TreeViewListItemProps) => { + const hasAnimations = useHasAnimations(hasAnimationsProp); const [internalIsExpanded, setIsExpanded] = useState(defaultExpanded); useEffect(() => { if (isExpanded !== undefined && isExpanded !== null) { diff --git a/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx b/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx index 4143d8e09c0..766ba8b3d94 100644 --- a/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx +++ b/packages/react-core/src/components/TreeView/__tests__/TreeView.test.tsx @@ -1,6 +1,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { TreeView } from '../TreeView'; +import { AnimationsProvider } from '../../../helpers/AnimationsProvider'; jest.mock('../TreeViewList', () => ({ TreeViewList: ({ @@ -293,6 +294,33 @@ test('Passes hasAnimations to TreeViewListItem', () => { expect(screen.getByText('TreeViewListItem hasAnimations: true')).toBeVisible(); }); + +// Animation context tests +test('respects AnimationsProvider context when no local hasAnimations prop', () => { + render( + + + + ); + + expect(screen.getByText('TreeViewListItem hasAnimations: true')).toBeVisible(); +}); + +test('local hasAnimations prop takes precedence over context', () => { + render( + + + + ); + + expect(screen.getByText('TreeViewListItem hasAnimations: false')).toBeVisible(); +}); + +test('works without AnimationsProvider (backward compatibility)', () => { + render(); + + expect(screen.getByText('TreeViewListItem hasAnimations: false')).toBeVisible(); +}); test('Passes data.children to TreeViewListItem', () => { render(); diff --git a/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap b/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap index 8b523ae077c..6f0bf3de7c5 100644 --- a/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap +++ b/packages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeView.test.tsx.snap @@ -105,7 +105,7 @@ exports[`Matches snapshot 1`] = ` TreeViewListItem useMemo: undefined

- TreeViewListItem hasAnimations: undefined + TreeViewListItem hasAnimations: false