Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 59 additions & 63 deletions packages/react-core/src/components/Alert/AlertGroup.tsx
Original file line number Diff line number Diff line change
@@ -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<React.HTMLProps<HTMLUListElement>, 'className'> {
/** Additional classes added to the AlertGroup */
Expand All @@ -26,78 +27,73 @@
'aria-label'?: string;
}

interface AlertGroupState {
container: HTMLElement;
}

class AlertGroup extends Component<AlertGroupProps, AlertGroupState> {
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<AlertGroupProps> = ({
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<HTMLElement | null>(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 = (
<AlertGroupInline
onOverflowClick={onOverflowClick}
className={className}
isToast={isToast}
isLiveRegion={isLiveRegion}
overflowMessage={overflowMessage}
aria-label={ariaLabel}
hasAnimations={hasAnimations}
{...props}
>
{children}
</AlertGroupInline>
);
if (!this.props.isToast) {
return alertGroup;
return () => {
if (containerRef.current) {
target.removeChild(containerRef.current);
containerRef.current = null;
}
setIsContainerReady(false);
};
}
}, [isToast, appendTo]);

Check warning on line 70 in packages/react-core/src/components/Alert/AlertGroup.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has a missing dependency: 'getTargetElement'. Either include it or remove the dependency array

const container = this.state.container;
const alertGroup = (
<AlertGroupInline
onOverflowClick={onOverflowClick}
className={className}
isToast={isToast}
isLiveRegion={isLiveRegion}
overflowMessage={overflowMessage}
aria-label={ariaLabel}
hasAnimations={hasAnimations}
{...props}
>
{children}
</AlertGroupInline>
);

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';
Original file line number Diff line number Diff line change
Expand Up @@ -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<AlertGroupProps> = ({
className,
children,
hasAnimations,
hasAnimations: hasAnimationsProp,
isToast,
isLiveRegion,
onOverflowClick,
overflowMessage,
...props
}: AlertGroupProps) => {
const hasAnimations = useHasAnimations(hasAnimationsProp);
const [handleTransitionEnd, setHandleTransitionEnd] = useState<() => void>(() => () => {});

const updateTransitionEnd = (onTransitionEnd: () => void) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
<AlertGroup isToast aria-label="group label">
<Alert variant="warning" title="alert title" />
</AlertGroup>
);

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 () => {
Expand All @@ -90,8 +93,11 @@ test('Calls the callback set by updateTransitionEnd when transition ends and ani
</AlertGroup>
);

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();
});
Expand Down Expand Up @@ -120,9 +126,54 @@ test('Does not call the callback set by updateTransitionEnd when transition ends
</AlertGroup>
);

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(
<AnimationsProvider config={{ hasAnimations: true }}>
<AlertGroup>
<Alert title="Test Alert" />
</AlertGroup>
</AnimationsProvider>
);

// 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(
<AnimationsProvider config={{ hasAnimations: true }}>
<AlertGroup hasAnimations={false}>
<Alert title="Test Alert" />
</AlertGroup>
</AnimationsProvider>
);

// 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(
<AlertGroup>
<Alert title="Test Alert" />
</AlertGroup>
);

// 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');
});
});
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -24,41 +24,34 @@ export interface DualListSelectorProps {
hasAnimations?: boolean;
}

class DualListSelector extends Component<DualListSelectorProps> {
static displayName = 'DualListSelector';
static defaultProps: PickOptional<DualListSelectorProps> = {
children: '',
isTree: false,
hasAnimations: false
};
export const DualListSelector: React.FunctionComponent<DualListSelectorProps> = ({
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 (
<DualListSelectorContext.Provider value={{ isTree, hasAnimations }}>
<GenerateId>
{(randomId) => (
<div
className={css(
styles.dualListSelector,
hasAnimations && isTree && styles.modifiers.animateExpand,
className
)}
id={id || randomId}
{...props}
>
{children}
</div>
)}
</GenerateId>
</DualListSelectorContext.Provider>
);
}
}

export { DualListSelector };
return (
<DualListSelectorContext.Provider value={{ isTree, hasAnimations }}>
<GenerateId>
{(randomId) => (
<div
className={css(
styles.dualListSelector,
hasAnimations && isTree && styles.modifiers.animateExpand,
className
)}
id={id || randomId}
{...props}
>
{children}
</div>
)}
</GenerateId>
</DualListSelectorContext.Provider>
);
};
DualListSelector.displayName = 'DualListSelector';
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLLIElement> {
/** Content rendered inside the dual list selector. */
Expand Down Expand Up @@ -58,14 +59,15 @@ const DualListSelectorTreeItemBase: React.FunctionComponent<DualListSelectorTree
badgeProps,
itemData,
isDisabled = false,
hasAnimations,
hasAnimations: hasAnimationsProp,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
useMemo,
...props
}: DualListSelectorTreeItemProps) => {
const ref = useRef(null);
const [isExpanded, setIsExpanded] = useState(defaultExpanded || false);
const { setFocusedOption } = useContext(DualListSelectorListContext);
const hasAnimations = useHasAnimations(hasAnimationsProp);

useEffect(() => {
setIsExpanded(defaultExpanded);
Expand Down
Loading
Loading