[EuiAccordion] Do not set arrowDisplay to none if the buttonElement is a div.

[alexwizp]

To address the accessibility issue described in https://github.com/elastic/security-team/issues/8611 , we intended to set the buttonElement to div and hide the default arrow by setting arrowDisplay to none. However, this approach is ineffective because the arrowDisplay option only works when buttonElement is set to button.

EUI code:

Our layout:

Please provide advice on how to resolve this issue.

[1Copenut]

@alexwizp Can we use the EuiAccordion - interactive content in trigger guidance to make the outer container a DIV, keep the arrow button, and have the link as a sibling element? We need to retain the arrow button as a visual cue and keyboard hook, and making the wrapper a DIV should remove the issue.

Holler at me if this is not workable or I'm missing context. Thank you!

[alexwizp]

@cee-chen @1Copenut
In our current design, the buttonContent property for EuiAccordion uses EuiPanel, inside of which there is a trigger arrow EuiButtonIcon. From a design perspective, the button is located inside the panel!

Now, I will try setting buttonElement="div" according to the guide you provided. In this case, the button is displayed outside the panel, which does not align with the initial design.

See:

before:

after:
Here I removed our EuiButtonIcon trigger button

Is my question still the same: do we have any technical reason for that condition in the EUI code, or can we remove it?

[1Copenut]

Looking at the description it seems the nesting order needs refactored. @cee-chen stop me if I'm wrong, but wouldn't it make more sense to:

1. EuiPanel as the outer container
2. EuiAccordion as a DIV inside the panel
3. Button, link, and metadata as sibling inline elements in the accordion

[alexwizp]

@1Copenut I would like to focus only on the EuiAccordion component. From the component's API perspective, we have the ability to place any ReactNode in the buttonContent property. The question is: can we use arrowDisplay === "none" in the case where buttonElement is set to div or not?

If you confirm that this case is not supported, I think I can recommend team to move the arrow button from the EuiPanel, like this:

[cee-chen]

Is my question still the same: do we have any technical reason for that condition in the EUI code, or can we remove it?

The reason for that condition is an accessibility/cognition one. Even if not a technical reason, it's still valid and shouldn't be removed as a default.

Now, I will try setting buttonElement="div" according to the guide you provided. In this case, the button is displayed outside the panel, which does not align with the initial design.

This is solvable by not using an EuiPanel but instead adding some custom CSS to the .euiAccordion__triggerWrapper DOM element/wrapper, e.g.:

import { EuiAccordion, euiShadow } from '@elastic/eui';

<EuiAccordion
  buttonElement="div"
  css={(euiThemeContext) => css`
    .euiAccordion__triggerWrapper {
      ${euiShadow(euiThemeContext, 'm')}
      border-radius: ${euiThemeContext.euiTheme.border.radius.medium};
      padding: ${euiThemeContext.euiTheme.size.base};
    }
  `}
  // ... other props
>
  ...
</EuiAccordion>

Example codesandbox here.

While custom CSS is not ideal, in this case it should solve the accessibility issue for now while maintaining the existing design. For a longer term solution, EUI could consider a couple options:

1. Updating the triggerWrapper element to allow custom props to be passed to it
2. Adding a custom prop/config, e.g. buttonProps: { hasShadow: true } that automatically adds a panel with shadows and/or allows consumers to configure a shadow on the button/trigger only

[github-actions]

👋 This issue hasn't seen activity in 3 days, so we're automatically closing this issue as answered. Please leave a comment if that's not the case, or if you have any remaining questions or issues.