[EuiAccordion] Fix accordion right arrow display when combined with extraAction (#3971)

Author: TAYTS

CHANGELOG.md

@@ -5,6 +5,7 @@
 **Bug fixes**
 
 - Fix incorrect `euiCodeBlockNameColor` variable usage for `.hljs-name` in SCSS ([#3991](https://github.com/elastic/eui/pull/3991))
+- Fixed bug in `EuiAccordion` where the `arrowDisplay="right"` is ignored when `extraAction` is configured ([#3971](https://github.com/elastic/eui/pull/3971))
 
 ## [`28.3.1`](https://github.com/elastic/eui/tree/v28.3.1)
 

src-docs/src/views/accordion/accordion_extra.js

@@ -1,13 +1,26 @@
 import React from 'react';
 
-import { EuiAccordion, EuiButton } from '../../../../src/components';
+import { EuiAccordion, EuiButton, EuiSpacer } from '../../../../src/components';
 
 export default () => (
-  <EuiAccordion
-    id="accordionExtra"
-    buttonContent="Click to open"
-    extraAction={<EuiButton size="s">Extra action!</EuiButton>}
-    paddingSize="l">
-    <div>Opened content.</div>
-  </EuiAccordion>
+  <>
+    <EuiAccordion
+      id="accordionExtraWithLeftArrow"
+      buttonContent="Click to open (Arrow on the left)"
+      extraAction={<EuiButton size="s">Extra action!</EuiButton>}
+      paddingSize="l">
+      <div>Opened content.</div>
+    </EuiAccordion>
+
+    <EuiSpacer />
+
+    <EuiAccordion
+      id="accordionExtraWithRightArrow"
+      arrowDisplay="right"
+      buttonContent="Click to open (Arrow on the right)"
+      extraAction={<EuiButton size="s">Extra action!</EuiButton>}
+      paddingSize="l">
+      <div>Opened content.</div>
+    </EuiAccordion>
+  </>
 );

src/components/accordion/__snapshots__/accordion.test.tsx.snap

@@ -19,6 +19,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
         aria-controls="12"
         aria-expanded={false}
         className="euiAccordion__button"
+        id="generated-id"
         onClick={[Function]}
         type="button"
       >
@@ -79,6 +80,7 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
         aria-controls="11"
         aria-expanded={true}
         className="euiAccordion__button"
+        id="generated-id"
         onClick={[Function]}
         type="button"
       >
@@ -133,6 +135,7 @@ exports[`EuiAccordion is rendered 1`] = `
       aria-controls="0"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -172,6 +175,7 @@ exports[`EuiAccordion props arrowDisplay none is rendered 1`] = `
       aria-controls="6"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -207,6 +211,7 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = `
       aria-controls="5"
       aria-expanded="false"
       class="euiAccordion__button euiAccordion__buttonReverse"
+      id="generated-id"
       type="button"
     >
       <span
@@ -250,6 +255,7 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
       aria-controls="2"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -293,6 +299,7 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
       aria-controls="1"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -332,6 +339,7 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
       aria-controls="3"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -378,6 +386,7 @@ exports[`EuiAccordion props forceState is rendered 1`] = `
       aria-controls="7"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -421,6 +430,7 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
       aria-controls="4"
       aria-expanded="true"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -464,6 +474,7 @@ exports[`EuiAccordion props isLoading is rendered 1`] = `
       aria-controls="9"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span
@@ -514,6 +525,7 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = `
       aria-controls="10"
       aria-expanded="false"
       class="euiAccordion__button"
+      id="generated-id"
       type="button"
     >
       <span

src/components/accordion/_accordion.scss

@@ -19,8 +19,7 @@
 
   &:focus {
     .euiAccordion__iconWrapper {
-      @include euiFocusRing;
-      color: $euiColorPrimary;
+      @include euiAccordionIconFocus;
     }
   }
 }
@@ -31,16 +30,14 @@
   justify-content: space-between;
 
   .euiAccordion__iconWrapper {
-    margin-left: $euiSizeS;
-    margin-right: $euiSizeXS;
+    @include euiAccordionIconMargin(right);
   }
 }
 
 .euiAccordion__iconWrapper {
   @include size($euiSize);
+  @include euiAccordionIconMargin;
   border-radius: $euiBorderRadius;
-  margin-right: $euiSizeS;
-  margin-left: $euiSizeXS;
   flex-shrink: 0;
 
   // Nested to override EuiIcon
@@ -54,6 +51,14 @@
   }
 }
 
+.euiAccordion__iconButton {
+  @include euiAccordionIconMargin(right);
+
+  &:focus {
+    @include euiAccordionIconFocus;
+  }
+}
+
 .euiAccordion__optionalAction {
   flex-shrink: 0;
 }
@@ -101,4 +106,4 @@ $paddingSizes: (
   .euiAccordion__spinner {
     margin-right: $euiSizeXS;
   }
-}
+}

src/components/accordion/_index.scss

@@ -1,2 +1,3 @@
+@import 'mixins';
 @import 'accordion';
 @import 'accordion_form';

src/components/accordion/_mixins.scss

@@ -0,0 +1,14 @@
+@mixin euiAccordionIconFocus {
+  @include euiFocusRing;
+  color: $euiColorPrimary;
+}
+
+@mixin euiAccordionIconMargin($align: left) {
+  @if $align == left {
+    margin-left: $euiSizeXS;
+    margin-right: $euiSizeS;
+  } @else {
+    margin-left: $euiSizeS;
+    margin-right: $euiSizeXS;
+  }
+}

src/components/accordion/accordion.test.tsx

@@ -23,6 +23,10 @@ import { requiredProps } from '../../test/required_props';
 
 import { EuiAccordion } from './accordion';
 
+jest.mock('./../../services/accessibility', () => ({
+  htmlIdGenerator: () => () => 'generated-id',
+}));
+
 let id = 0;
 const getId = () => `${id++}`;
 

src/components/accordion/accordion.tsx

@@ -26,6 +26,7 @@ import { EuiIcon } from '../icon';
 import { EuiLoadingSpinner } from '../loading';
 import { EuiResizeObserver } from '../observer/resize_observer';
 import { EuiI18n } from '../i18n';
+import { htmlIdGenerator } from '../../services';
 
 const paddingSizeToClassNameMap = {
   none: '',
@@ -72,7 +73,6 @@ export type EuiAccordionProps = CommonProps &
     paddingSize?: EuiAccordionSize;
     /**
      * Placement of the arrow indicator, or 'none' to hide it.
-     * Placing on the `right` doesn't work with `extraAction` and so it will be ignored
      */
     arrowDisplay?: 'left' | 'right' | 'none';
     /**
@@ -200,13 +200,32 @@ export class EuiAccordion extends Component<
       'euiAccordion__icon-isOpen': isOpen,
     });
 
-    let icon;
+    const iconWrapperClasses = classNames('euiAccordion__iconWrapper', {
+      euiAccordion__iconButton: extraAction && arrowDisplay === 'right',
+    });
+
+    let baseIcon;
     if (arrowDisplay !== 'none') {
-      icon = (
-        <span className="euiAccordion__iconWrapper">
-          <EuiIcon className={iconClasses} type="arrowRight" size="m" />
-        </span>
+      baseIcon = <EuiIcon className={iconClasses} type="arrowRight" size="m" />;
+    }
+
+    let icon;
+    let iconButton;
+    const buttonId = htmlIdGenerator()();
+    if (extraAction && arrowDisplay === 'right') {
+      iconButton = (
+        <button
+          aria-controls={id}
+          aria-expanded={isOpen}
+          aria-labelledby={buttonId}
+          tabIndex={-1}
+          className={iconWrapperClasses}
+          onClick={this.onToggle}>
+          {baseIcon}
+        </button>
       );
+    } else if (arrowDisplay !== 'none') {
+      icon = <span className={iconWrapperClasses}>{baseIcon}</span>;
     }
 
     let optionalAction = null;
@@ -245,6 +264,7 @@ export class EuiAccordion extends Component<
       <div className={classes} {...rest}>
         <div className="euiAccordion__triggerWrapper">
           <button
+            id={buttonId}
             aria-controls={id}
             aria-expanded={isOpen}
             onClick={this.onToggle}
@@ -260,6 +280,7 @@ export class EuiAccordion extends Component<
             </span>
           </button>
           {optionalAction}
+          {iconButton}
         </div>
 
         <div

src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap

@@ -246,6 +246,7 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true will render an accord
       aria-controls="id"
       aria-expanded="false"
       class="euiAccordion__button euiAccordion__buttonReverse euiCollapsibleNavGroup__heading"
+      id="generated-id"
       type="button"
     >
       <span

src/components/collapsible_nav/collapsible_nav_group/collapsible_nav_group.test.tsx

@@ -23,6 +23,10 @@ import { requiredProps } from '../../../test/required_props';
 
 import { EuiCollapsibleNavGroup, BACKGROUNDS } from './collapsible_nav_group';
 
+jest.mock('./../../../services/accessibility', () => ({
+  htmlIdGenerator: () => () => 'generated-id',
+}));
+
 describe('EuiCollapsibleNavGroup', () => {
   test('is rendered', () => {
     const component = render(