fix(accordion): panel checkbox collapsing panel on check (#2538)

* fix(accordion): updating collapse to check for index value before closing panel

* chore: changeset

* fix(accordion): typecheck in onchange function, removing deprecated code

* fix(accordion): instanceof check for change events

* docs: update .changeset/neat-colts-jam.md

---------

Co-authored-by: Benny Powers <[email protected]>
Co-authored-by: Benny Powers <[email protected]>

Author: brianferry

.changeset/neat-colts-jam.md

@@ -0,0 +1,5 @@
+---
+"@patternfly/elements": patch
+---
+
+`<pf-accordion>`: fixed bug which would collapse a panel if it contained a checkbox which got toggled.

elements/pf-accordion/BaseAccordion.ts

@@ -48,6 +48,10 @@ export abstract class BaseAccordion extends LitElement {
     return target instanceof BaseAccordionPanel;
   }
 
+  static #isAccordionChangeEvent(event: Event): event is AccordionHeaderChangeEvent {
+    return event instanceof AccordionHeaderChangeEvent;
+  }
+
   #headerIndex = new RovingTabindexController<BaseAccordionHeader>(this);
 
   #expandedIndex: number[] = [];
@@ -265,51 +269,14 @@ export abstract class BaseAccordion extends LitElement {
   }
 
   #onChange(event: AccordionHeaderChangeEvent) {
-    if (this.classList.contains('animating')) {
-      return;
-    }
-
-    const index = this.#getIndex(event.target as Element);
-
-    if (event.expanded) {
-      this.expand(index, event.accordion);
-    } else {
-      this.collapse(index);
-    }
-  }
-
-  /**
-   * @see https://www.w3.org/TR/wai-aria-practices/#accordion
-   */
-  async #onKeydown(evt: KeyboardEvent) {
-    const currentHeader = evt.target as Element;
-
-    if (!BaseAccordion.isHeader(currentHeader)) {
-      return;
-    }
-
-    let newHeader: BaseAccordionHeader | undefined;
-
-    switch (evt.key) {
-      case 'ArrowDown':
-        evt.preventDefault();
-        newHeader = this.#nextHeader();
-        break;
-      case 'ArrowUp':
-        evt.preventDefault();
-        newHeader = this.#previousHeader();
-        break;
-      case 'Home':
-        evt.preventDefault();
-        newHeader = this.#firstHeader();
-        break;
-      case 'End':
-        evt.preventDefault();
-        newHeader = this.#lastHeader();
-        break;
+    if (BaseAccordion.#isAccordionChangeEvent(event) && !this.classList.contains('animating')) {
+      const index = this.#getIndex(event.target);
+      if (event.expanded) {
+        this.expand(index, event.accordion);
+      } else {
+        this.collapse(index);
+      }
     }
-
-    newHeader?.focus?.();
   }
 
   #allHeaders(accordion: BaseAccordion = this): BaseAccordionHeader[] {
@@ -320,26 +287,6 @@ export abstract class BaseAccordion extends LitElement {
     return Array.from(accordion.children).filter(BaseAccordion.isPanel);
   }
 
-  #previousHeader() {
-    const { headers } = this;
-    const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) - 1;
-    return headers[(newIndex + headers.length) % headers.length];
-  }
-
-  #nextHeader() {
-    const { headers } = this;
-    const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) + 1;
-    return headers[newIndex % headers.length];
-  }
-
-  #firstHeader() {
-    return this.headers.at(0);
-  }
-
-  #lastHeader() {
-    return this.headers.at(-1);
-  }
-
   #getIndex(el: Element | null) {
     if (BaseAccordion.isHeader(el)) {
       return this.headers.findIndex(header => header.id === el.id);
@@ -386,10 +333,6 @@ export abstract class BaseAccordion extends LitElement {
    * Accepts an optional parent accordion to search for headers and panels.
    */
   public async expand(index: number, parentAccordion?: BaseAccordion) {
-    if (index === -1) {
-      return;
-    }
-
     const allHeaders: Array<BaseAccordionHeader> = this.#allHeaders(parentAccordion);
 
     const header = allHeaders[index];

elements/pf-accordion/BaseAccordionHeader.ts

@@ -15,6 +15,7 @@ const isPorHeader =
     el instanceof HTMLElement && !!el.tagName.match(/P|^H[1-6]/);
 
 export class AccordionHeaderChangeEvent extends ComposedEvent {
+  declare target: BaseAccordionHeader;
   constructor(
     public expanded: boolean,
     public toggle: BaseAccordionHeader,

elements/pf-accordion/test/pf-accordion.spec.ts

@@ -1,11 +1,10 @@
-import type { ReactiveElement } from 'lit';
-
 import { expect, html, aTimeout, nextFrame } from '@open-wc/testing';
 import { createFixture } from '@patternfly/pfe-tools/test/create-fixture.js';
 import { sendKeys } from '@web/test-runner-commands';
 
 // Import the element we're testing.
 import { PfAccordion, PfAccordionPanel, PfAccordionHeader } from '@patternfly/elements/pf-accordion/pf-accordion.js';
+import { PfSwitch } from '@patternfly/elements/pf-switch/pf-switch.js';
 
 import { Logger } from '@patternfly/pfe-core/controllers/logger.js';
 
@@ -1321,4 +1320,55 @@ describe('<pf-accordion>', function() {
       });
     });
   });
+
+  describe('with a single expanded header and panel containing a checkbox and a switch', function() {
+    let element: PfAccordion;
+    let headers: NodeListOf<PfAccordionHeader>;
+    let panels: NodeListOf<PfAccordionPanel>;
+    let checkbox: HTMLInputElement;
+    let pfswitch: PfSwitch;
+    let accordionPanelOne: PfAccordionPanel;
+
+    beforeEach(async function() {
+      element = await createFixture<PfAccordion>(html`
+          <pf-accordion>
+            <pf-accordion-header expanded id="header-1-1" data-index="0"></pf-accordion-header>
+            <pf-accordion-panel id="panel-1-1" data-index="0">
+              <pf-switch></pf-switch>
+              <input type="checkbox">
+            </pf-accordion-panel>
+          </pf-accordion>
+        `);
+      headers = document.querySelectorAll('pf-accordion-header');
+      panels = document.querySelectorAll('pf-accordion-panel');
+      checkbox = element.querySelector('input')!;
+      pfswitch = element.querySelector('pf-switch')!;
+      expect(checkbox).to.be.ok;
+      expect(pfswitch).to.be.ok;
+      [accordionPanelOne] = panels;
+    });
+
+    describe('clicking the checkbox', function() {
+      beforeEach(async function() {
+        checkbox.click();
+        await element.updateComplete;
+      });
+      it('does not collapse the panel', function() {
+        expect(accordionPanelOne.expanded).to.be.true;
+      });
+    });
+
+    describe('clicking the switch', function() {
+      beforeEach(async function() {
+        const { checked } = pfswitch;
+        pfswitch.click();
+        await element.updateComplete;
+        await pfswitch.updateComplete;
+        expect(pfswitch.checked).to.not.equal(checked);
+      });
+      it('does not collapse the panel', function() {
+        expect(accordionPanelOne.expanded).to.be.true;
+      });
+    });
+  });
 });