fix(accordion): add transition styles back (#17418)

* fix(accordion): add transition styles back * chore: fix test * chore: snapshot and test story * chore: snapshot * chore: testing * chore: update test * chore: typo
carbon-design-system · Oct 14, 2024 · 4a7d935 · 4a7d935
1 parent badbac2
commit 4a7d935
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 29 deletions.
diff --git a/packages/react/src/components/Accordion/AccordionItem.tsx b/packages/react/src/components/Accordion/AccordionItem.tsx
@@ -196,7 +196,6 @@ function AccordionItem({
       </Toggle>
       <div
         ref={content}
-        hidden={!isOpen}
         className={`${prefix}--accordion__wrapper`}
         onTransitionEnd={onAnimationEnd}>
         <div id={id} className={`${prefix}--accordion__content`}>

diff --git a/packages/react/src/components/Accordion/__tests__/Accordion-test.js b/packages/react/src/components/Accordion/__tests__/Accordion-test.js
@@ -5,7 +5,6 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import '../../../../scss/components/accordion/_index.scss';
 import Button from '../../Button';
 import React from 'react';
 import { default as Accordion, AccordionItem } from '../';
@@ -233,25 +232,27 @@ describe('Accordion', () => {
     it('should expand All on click to button', async () => {
       render(<ControlledAccordion />);
 
-      // click to open
       await userEvent.click(screen.getByText('Click to expand all'));
 
-      // test when open
-      expect(screen.getByText('Panel A')).toBeVisible();
-      expect(screen.getByText('Panel B')).toBeVisible();
-      expect(screen.getByText('Panel C')).toBeVisible();
+      // Check if the class `cds--accordion__item--active` is added to all items
+      const items = screen.getAllByRole('listitem');
+      items.forEach((item) => {
+        expect(item).toHaveClass('cds--accordion__item--active');
+      });
     });
 
     it('should Collapse All on click to button', async () => {
       render(<ControlledAccordion />);
 
-      // click to close
+      await userEvent.click(screen.getByText('Click to expand all'));
+
       await userEvent.click(screen.getByText('Click to collapse all'));
 
-      // test when close
-      expect(screen.getByText('Panel A')).not.toBeVisible();
-      expect(screen.getByText('Panel B')).not.toBeVisible();
-      expect(screen.getByText('Panel C')).not.toBeVisible();
+      // Check if the class `cds--accordion__item--active` is removed from all items
+      const items = screen.getAllByRole('listitem');
+      items.forEach((item) => {
+        expect(item).not.toHaveClass('cds--accordion__item--active');
+      });
     });
   });
   describe('Ordered List', () => {

diff --git a/packages/react/src/components/Accordion/__tests__/__snapshots__/Accordion-test.js.snap b/packages/react/src/components/Accordion/__tests__/__snapshots__/Accordion-test.js.snap
@@ -38,7 +38,6 @@ exports[`Accordion should render 1`] = `
       </button>
       <div
         class="cds--accordion__wrapper"
-        hidden=""
       >
         <div
           class="cds--accordion__content"
@@ -81,7 +80,6 @@ exports[`Accordion should render 1`] = `
       </button>
       <div
         class="cds--accordion__wrapper"
-        hidden=""
       >
         <div
           class="cds--accordion__content"
@@ -124,7 +122,6 @@ exports[`Accordion should render 1`] = `
       </button>
       <div
         class="cds--accordion__wrapper"
-        hidden=""
       >
         <div
           class="cds--accordion__content"

diff --git a/packages/react/src/components/Accordion/__tests__/__snapshots__/AccordionItem-test.js.snap b/packages/react/src/components/Accordion/__tests__/__snapshots__/AccordionItem-test.js.snap
@@ -35,7 +35,6 @@ exports[`AccordionItem renders as expected - Component API should render and mat
     </button>
     <div
       class="cds--accordion__wrapper"
-      hidden=""
     >
       <div
         class="cds--accordion__content"

diff --git a/packages/styles/scss/components/accordion/_accordion.scss b/packages/styles/scss/components/accordion/_accordion.scss
@@ -176,11 +176,13 @@ $content-padding: 0 0 0 $spacing-05 !default;
 
   .#{$prefix}--accordion__wrapper {
     // Properties for when the accordion is closed
+    display: none;
     overflow: hidden;
     padding: 0;
     max-block-size: 0;
     opacity: 0;
-    transition: all $duration-fast-02 motion(entrance, productive);
+    transition: all $duration-fast-02 motion(entrance, productive)
+      allow-discrete;
     writing-mode: horizontal-tb;
   }
 
@@ -231,6 +233,7 @@ $content-padding: 0 0 0 $spacing-05 !default;
 
     > .#{$prefix}--accordion__wrapper {
       // Properties for when the accordion is open
+      display: block;
       overflow: visible;
       max-block-size: fit-content;
       opacity: 1;
@@ -245,6 +248,21 @@ $content-padding: 0 0 0 $spacing-05 !default;
       }
     }
   }
+  /** starting style also not supported widely
+    * https://caniuse.com/mdn-css_at-rules_starting-style
+    */
+  /* Needs to be after the previous accordion__item--active rule
+     to take effect, as the specificity is the same */
+  /* stylelint-disable-next-line scss/at-rule-no-unknown */
+  @starting-style {
+    .#{$prefix}--accordion__item--active {
+      > .#{$prefix}--accordion__wrapper {
+        padding: 0;
+        max-block-size: 0;
+        opacity: 0;
+      }
+    }
+  }
 
   // flush
   .#{$prefix}--accordion--flush .#{$prefix}--accordion__item {
@@ -383,15 +401,3 @@ $content-padding: 0 0 0 $spacing-05 !default;
     padding-inline-start: 0;
   }
 }
-
-@mixin -content-visible {
-  block-size: 100%;
-  opacity: 1;
-  visibility: inherit;
-}
-
-@mixin -content-hidden {
-  block-size: 0;
-  opacity: 0;
-  visibility: hidden;
-}