Skip to content

[EuiAccordion] Migrate from class to function component #9558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
antonbc wants to merge 3 commits into
base: main
Choose a base branch
from
+96 −130
Open

[EuiAccordion] Migrate from class to function component #9558

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b82d6b8
refactor: convert EuiAccordion to function component with useState an…
antonbc Apr 4, 2026
1962403
chore: add changelog for #9558
antonbc Apr 4, 2026
087711b
chore: remove unnecessary changelog entry for #9558
antonbc Apr 7, 2026
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
226 changes: 96 additions & 130 deletions packages/eui/src/components/accordion/accordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
* Side Public License, v 1.
*/

import React, { Component, HTMLAttributes, ReactNode } from 'react';
import React, {
FunctionComponent,
HTMLAttributes,
ReactNode,
useState,
} from 'react';
import classNames from 'classnames';

import {
htmlIdGenerator,
withEuiTheme,
WithEuiThemeProps,
} from '../../services';
import { useEuiTheme, useGeneratedHtmlId } from '../../services';
import { CommonProps } from '../common';
import { EuiLoadingSpinner } from '../loading';
import type { EuiButtonIconProps } from '../button';
Expand Down Expand Up @@ -114,133 +115,98 @@ export type EuiAccordionProps = CommonProps &
isDisabled?: boolean;
};

type EuiAccordionState = {
isOpen: boolean;
};

export class EuiAccordionClass extends Component<
WithEuiThemeProps & EuiAccordionProps,
EuiAccordionState
> {
static defaultProps = {
initialIsOpen: false,
borders: 'none' as const,
paddingSize: 'none' as const,
arrowDisplay: 'left' as const,
isLoading: false,
isDisabled: false,
isLoadingMessage: false,
element: 'div' as const,
buttonElement: 'button' as const,
role: 'group' as const,
};

state = {
isOpen: this.props.forceState
? this.props.forceState === 'open'
: this.props.initialIsOpen!,
};

get isOpen() {
return this.props.forceState
? this.props.forceState === 'open'
: this.state.isOpen;
}

onToggle = () => {
const { forceState } = this.props;
export const EuiAccordionClass: FunctionComponent<EuiAccordionProps> = ({
Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

We should export it directly as EuiAccordion, not EuiAccordionClass because that's no longer true.

Suggested change
export const EuiAccordionClass: FunctionComponent<EuiAccordionProps> = ({
export const EuiAccordion: FunctionComponent<EuiAccordionProps> = ({

children,
className,
id,
role = 'group',
element: Element = 'div',
buttonElement = 'button',
buttonProps,
buttonClassName,
buttonContentClassName,
buttonContent,
arrowDisplay = 'left',
arrowProps,
extraAction,
paddingSize = 'none',
borders = 'none',
initialIsOpen = false,
forceState,
isLoading = false,
isLoadingMessage = false,
isDisabled = false,
onToggle,
...rest
}) => {
const [isOpenState, setIsOpenState] = useState(
forceState ? forceState === 'open' : initialIsOpen
);

const isOpen = forceState ? forceState === 'open' : isOpenState;

const onAccordionToggle = () => {
if (forceState) {
const nextState = !this.isOpen;
this.props.onToggle?.(nextState);
onToggle?.(!isOpen);
} else {
this.setState(
(prevState) => ({
isOpen: !prevState.isOpen,
}),
() => {
this.props.onToggle?.(this.state.isOpen);
}
);
setIsOpenState((prevIsOpen) => {
const nextState = !prevIsOpen;
onToggle?.(nextState);
return nextState;
});
Comment on lines +152 to +156
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onToggle is invoked inside the setIsOpenState functional updater. React may call state updater functions more than once (e.g., StrictMode/dev, interrupted renders), which can lead to onToggle firing multiple times for a single user interaction. Consider computing nextState in the click handler using the current isOpenState, calling setIsOpenState(nextState), and then calling onToggle(nextState) outside the updater (or triggering the callback from an effect that only runs for user-initiated toggles).

Suggested change
setIsOpenState((prevIsOpen) => {
const nextState = !prevIsOpen;
onToggle?.(nextState);
return nextState;
});
const nextState = !isOpenState;
setIsOpenState(nextState);
onToggle?.(nextState);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's please address this.

}
};

generatedId = htmlIdGenerator()();

render() {
const {
children,
className,
id,
role,
element: Element = 'div',
buttonElement,
buttonProps,
buttonClassName,
buttonContentClassName,
buttonContent,
arrowDisplay,
arrowProps,
extraAction,
paddingSize,
borders,
initialIsOpen,
forceState,
isLoading,
isLoadingMessage,
isDisabled,
theme,
...rest
} = this.props;

const classes = classNames(
'euiAccordion',
{ 'euiAccordion-isOpen': this.isOpen },
className
);

const styles = euiAccordionStyles(theme);
const cssStyles = [
styles.euiAccordion,
borders !== 'none' && styles.borders.borders,
borders !== 'none' && styles.borders[borders!],
];

const buttonId = buttonProps?.id ?? this.generatedId;

return (
<Element className={classes} css={cssStyles} {...rest}>
<EuiAccordionTrigger
ariaControlsId={id}
buttonId={buttonId}
// Force button element to be a legend if the element is a fieldset
buttonElement={Element === 'fieldset' ? 'legend' : buttonElement}
buttonClassName={buttonClassName}
buttonContent={buttonContent}
buttonContentClassName={buttonContentClassName}
buttonProps={buttonProps}
arrowProps={arrowProps}
arrowDisplay={arrowDisplay}
isDisabled={isDisabled}
isOpen={this.isOpen}
onToggle={this.onToggle}
extraAction={isLoading ? <EuiLoadingSpinner /> : extraAction}
/>

<EuiAccordionChildren
role={role}
id={id}
aria-labelledby={buttonId}
paddingSize={paddingSize}
isLoading={isLoading}
isLoadingMessage={isLoadingMessage}
isOpen={this.isOpen}
initialIsOpen={initialIsOpen}
>
{children}
</EuiAccordionChildren>
</Element>
);
}
}
const generatedId = useGeneratedHtmlId();
const buttonId = buttonProps?.id ?? generatedId;

const euiTheme = useEuiTheme();
const classes = classNames(
'euiAccordion',
{ 'euiAccordion-isOpen': isOpen },
className
);

const styles = euiAccordionStyles(euiTheme);
Comment on lines +163 to +170
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEuiTheme() returns a theme object (with euiTheme, colorMode, etc.), but the local variable is named euiTheme, which reads like it contains only the euiTheme property. Renaming this variable to theme (or destructuring { euiTheme } and adjusting the style call accordingly) would avoid confusion and reduce the chance of misusing the hook return value later.

Suggested change
const euiTheme = useEuiTheme();
const classes = classNames(
'euiAccordion',
{ 'euiAccordion-isOpen': isOpen },
className
);
const styles = euiAccordionStyles(euiTheme);
const theme = useEuiTheme();
const classes = classNames(
'euiAccordion',
{ 'euiAccordion-isOpen': isOpen },
className
);
const styles = euiAccordionStyles(theme);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use euiThemeContext:

const euiThemeContext = useEuiTheme();
const styles = euiAccordionStyles(euiThemeContext);

Copy link
Copy Markdown
Contributor

@weronikaolejniczak weronikaolejniczak Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not part of the task but we could use useEuiMemoizedStyles

const cssStyles = [
styles.euiAccordion,
borders !== 'none' && styles.borders.borders,
borders !== 'none' && styles.borders[borders],
];

return (
<Element className={classes} css={cssStyles} {...rest}>
<EuiAccordionTrigger
ariaControlsId={id}
buttonId={buttonId}
// Force button element to be a legend if the element is a fieldset
buttonElement={Element === 'fieldset' ? 'legend' : buttonElement}
buttonClassName={buttonClassName}
buttonContent={buttonContent}
buttonContentClassName={buttonContentClassName}
buttonProps={buttonProps}
arrowProps={arrowProps}
arrowDisplay={arrowDisplay}
isDisabled={isDisabled}
isOpen={isOpen}
onToggle={onAccordionToggle}
extraAction={isLoading ? <EuiLoadingSpinner /> : extraAction}
/>

<EuiAccordionChildren
role={role}
id={id}
aria-labelledby={buttonId}
paddingSize={paddingSize}
isLoading={isLoading}
isLoadingMessage={isLoadingMessage}
isOpen={isOpen}
initialIsOpen={initialIsOpen}
>
{children}
</EuiAccordionChildren>
</Element>
);
};

export const EuiAccordion = withEuiTheme<EuiAccordionProps>(EuiAccordionClass);
export const EuiAccordion = EuiAccordionClass;