fix: contain activation to header content

adobe · May 12, 2022 · 10183ce · 10183ce
1 parent 70e57ce
commit 10183ce
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 22 deletions.
diff --git a/packages/accordion/src/AccordionItem.ts b/packages/accordion/src/AccordionItem.ts
@@ -46,22 +46,6 @@ export class AccordionItem extends Focusable {
         return this.shadowRoot.querySelector('#header') as HTMLElement;
     }
 
-    constructor() {
-        super();
-        this.addEventListener('keydown', this.onKeyDown);
-    }
-
-    private onKeyDown(event: KeyboardEvent): void {
-        /* c8 ignore next 3 */
-        if (this.disabled) {
-            return;
-        }
-        if (event.code === 'Enter' || event.code === 'Space') {
-            event.preventDefault();
-            this.toggle();
-        }
-    }
-
     private onClick(): void {
         /* c8 ignore next 3 */
         if (this.disabled) {
@@ -92,6 +76,7 @@ export class AccordionItem extends Focusable {
                     @click=${this.onClick}
                     aria-expanded=${this.open}
                     aria-controls="content"
+                    ?disabled=${this.disabled}
                 >
                     ${this.label}
                 </button>

diff --git a/packages/accordion/stories/accordion.stories.ts b/packages/accordion/stories/accordion.stories.ts
@@ -14,6 +14,7 @@ import { html, TemplateResult } from '@spectrum-web-components/base';
 
 import '../sp-accordion.js';
 import '../sp-accordion-item.js';
+import '@spectrum-web-components/link/sp-link.js';
 
 export default {
     title: 'Accordion',
@@ -61,7 +62,20 @@ const Template = (
             <sp-accordion-item label="Heading 2" ?open=${open}>
                 Item 2
             </sp-accordion-item>
-            <sp-accordion-item label="Heading 3">Item 3</sp-accordion-item>
+            <sp-accordion-item label="Heading 3">
+                <p>
+                    This is content that has a
+                    <sp-link
+                        href="http://opensource.adobe.com/spectrum-web-components"
+                        target="_blank"
+                    >
+                        link back to Spectrum Web Components
+                    </sp-link>
+                    so that it is easy to test that "Space" and "Enter"
+                    interactions on focusable content does NOT toggle the
+                    Accordion Item.
+                </p>
+            </sp-accordion-item>
         </sp-accordion>
     `;
 };

diff --git a/packages/accordion/test/accordion-item.test.ts b/packages/accordion/test/accordion-item.test.ts
@@ -15,7 +15,7 @@ import { spy } from 'sinon';
 
 import '../sp-accordion-item.js';
 import { AccordionItem } from '../src/AccordionItem';
-import { enterEvent, spaceEvent } from '../../../test/testing-helpers.js';
+import { sendKeys } from '@web/test-runner-commands';
 
 describe('Accordion Item', () => {
     it('can exist with no parent accessibly', async () => {
@@ -88,7 +88,10 @@ describe('Accordion Item', () => {
 
         expect(open).to.be.false;
 
-        el.dispatchEvent(enterEvent());
+        el.focus();
+        await sendKeys({
+            press: 'Enter',
+        });
 
         await elementUpdated(el);
 
@@ -97,7 +100,10 @@ describe('Accordion Item', () => {
         el.disabled = false;
         await elementUpdated(el);
 
-        el.dispatchEvent(enterEvent());
+        el.focus();
+        await sendKeys({
+            press: 'Enter',
+        });
 
         await elementUpdated(el);
 
@@ -124,7 +130,10 @@ describe('Accordion Item', () => {
 
         expect(open).to.be.false;
 
-        el.dispatchEvent(spaceEvent());
+        el.focus();
+        await sendKeys({
+            press: 'Space',
+        });
 
         await elementUpdated(el);
 
@@ -133,10 +142,58 @@ describe('Accordion Item', () => {
         el.disabled = false;
         await elementUpdated(el);
 
-        el.dispatchEvent(spaceEvent());
+        el.focus();
+        await sendKeys({
+            press: 'Space',
+        });
 
         await elementUpdated(el);
 
         expect(open).to.be.true;
     });
+
+    it('does not dispatch toggle events on key events in Item content', async () => {
+        let closed = false;
+        const onAccordionToggle = (): void => {
+            closed = true;
+        };
+        const el = await fixture<AccordionItem>(
+            html`
+                <sp-accordion-item
+                    open
+                    @sp-accordion-item-toggle=${onAccordionToggle}
+                >
+                    <div>
+                        <button>Test Button</button>
+                    </div>
+                </sp-accordion-item>
+            `
+        );
+
+        const button = el.querySelector('button') as HTMLButtonElement;
+        await elementUpdated(el);
+
+        expect(el.open).to.be.true;
+        expect(closed).to.be.false;
+
+        button.focus();
+        await sendKeys({
+            press: 'Space',
+        });
+
+        await elementUpdated(el);
+
+        expect(closed).to.be.false;
+
+        await elementUpdated(el);
+
+        await sendKeys({
+            press: 'Enter',
+        });
+
+        await elementUpdated(el);
+
+        expect(closed).to.be.false;
+        expect(el.open).to.be.true;
+    });
 });