From e10f49c43daa11fe7deb5a9b9bfd34897542b6b1 Mon Sep 17 00:00:00 2001
From: Shawn Taylor <shawn@ionic.io>
Date: Wed, 24 Jan 2024 14:57:36 -0500
Subject: [PATCH] fix(accordion): prevent opening of readonly accordion using
 keyboard (#28865)

Issue number: resolves #28344

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
When an Accordion is inside an Accordion Group, and the Accordion Group
is enabled and not readonly but the Accordion is disabled and/or
readonly, it is possible to navigate to the accordion and open it using
the keyboard. This should not be allowed, just like opening it on click
is not allowed.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- A disabled Accordion inside an Accordion Group may not be opened via
the keyboard.
- A readonly Accordion inside an Accordion Group may not be opened via
the keyboard.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
---
 core/src/components/accordion/accordion.tsx   |   5 +-
 .../accordion/test/accordion.e2e.ts           |  49 -------
 .../accordion/test/disabled/accordion.e2e.ts  | 137 ++++++++++++++++++
 .../accordion/test/disabled/index.html        |  91 ++++++++++++
 .../accordion/test/readonly/accordion.e2e.ts  | 137 ++++++++++++++++++
 .../accordion/test/readonly/index.html        |  91 ++++++++++++
 6 files changed, 460 insertions(+), 50 deletions(-)
 delete mode 100644 core/src/components/accordion/test/accordion.e2e.ts
 create mode 100644 core/src/components/accordion/test/disabled/accordion.e2e.ts
 create mode 100644 core/src/components/accordion/test/disabled/index.html
 create mode 100644 core/src/components/accordion/test/readonly/accordion.e2e.ts
 create mode 100644 core/src/components/accordion/test/readonly/index.html

diff --git a/core/src/components/accordion/accordion.tsx b/core/src/components/accordion/accordion.tsx
index eab83209cd5..92d28848d20 100644
--- a/core/src/components/accordion/accordion.tsx
+++ b/core/src/components/accordion/accordion.tsx
@@ -382,7 +382,10 @@ export class Accordion implements ComponentInterface {
   };
 
   private toggleExpanded() {
-    const { accordionGroupEl, value, state } = this;
+    const { accordionGroupEl, disabled, readonly, value, state } = this;
+
+    if (disabled || readonly) return;
+
     if (accordionGroupEl) {
       /**
        * Because the accordion group may or may
diff --git a/core/src/components/accordion/test/accordion.e2e.ts b/core/src/components/accordion/test/accordion.e2e.ts
deleted file mode 100644
index f09aa4b1a2f..00000000000
--- a/core/src/components/accordion/test/accordion.e2e.ts
+++ /dev/null
@@ -1,49 +0,0 @@
-import { expect } from '@playwright/test';
-import { configs, test } from '@utils/test/playwright';
-
-configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ config, title }) => {
-  test.describe(title('accordion: states'), () => {
-    test.beforeEach(async ({ page }) => {
-      await page.setContent(
-        `
-      <ion-accordion-group animated="false">
-        <ion-accordion>
-          <ion-item slot="header">Label</ion-item>
-          <div slot="content">Content</div>
-        </ion-accordion>
-      </ion-accordion-group>
-    `,
-        config
-      );
-    });
-    test('should properly set readonly on child accordions', async ({ page }) => {
-      const accordionGroup = page.locator('ion-accordion-group');
-      const accordion = page.locator('ion-accordion');
-
-      await expect(accordion).toHaveJSProperty('readonly', false);
-
-      await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => {
-        el.readonly = true;
-      });
-
-      await page.waitForChanges();
-
-      await expect(accordion).toHaveJSProperty('readonly', true);
-    });
-
-    test('should properly set disabled on child accordions', async ({ page }) => {
-      const accordionGroup = page.locator('ion-accordion-group');
-      const accordion = page.locator('ion-accordion');
-
-      await expect(accordion).toHaveJSProperty('disabled', false);
-
-      await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => {
-        el.disabled = true;
-      });
-
-      await page.waitForChanges();
-
-      await expect(accordion).toHaveJSProperty('disabled', true);
-    });
-  });
-});
diff --git a/core/src/components/accordion/test/disabled/accordion.e2e.ts b/core/src/components/accordion/test/disabled/accordion.e2e.ts
new file mode 100644
index 00000000000..3857a1d7b26
--- /dev/null
+++ b/core/src/components/accordion/test/disabled/accordion.e2e.ts
@@ -0,0 +1,137 @@
+import { expect } from '@playwright/test';
+import { configs, test } from '@utils/test/playwright';
+
+// NOTE: these tests cannot be re-written as spec tests because the `getAccordions` method in accordion-group.tsx uses a `:scope` selector
+configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ config, title }) => {
+  test.describe(title('accordion: disabled'), () => {
+    test('should properly set disabled on child accordions', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordionGroup = page.locator('ion-accordion-group');
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveJSProperty('disabled', false);
+
+      await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => {
+        el.disabled = true;
+      });
+
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveJSProperty('disabled', true);
+    });
+
+    test('should not open accordion on click when group is disabled', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false" disabled>
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      accordion.click();
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion on click when accordion is disabled', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion disabled>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      accordion.click();
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion via keyboard navigation when group is disabled', async ({ page, browserName }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false" disabled>
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+      const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      await page.keyboard.press(tabKey);
+      await page.waitForChanges();
+
+      await page.keyboard.press('Enter');
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion via keyboard navigation when accordion is disabled', async ({
+      page,
+      browserName,
+    }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion disabled>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+      const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      await page.keyboard.press(tabKey);
+      await page.waitForChanges();
+
+      await page.keyboard.press('Enter');
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+  });
+});
diff --git a/core/src/components/accordion/test/disabled/index.html b/core/src/components/accordion/test/disabled/index.html
new file mode 100644
index 00000000000..38452b7adec
--- /dev/null
+++ b/core/src/components/accordion/test/disabled/index.html
@@ -0,0 +1,91 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Accordion - Disabled</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0" />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+    <style>
+      .grid {
+        display: grid;
+        grid-template-columns: repeat(auto-fill, minmax(250px, 1fr));
+        grid-row-gap: 20px;
+        grid-column-gap: 20px;
+      }
+    </style>
+  </head>
+  <body>
+    <ion-app>
+      <ion-header translucent="true">
+        <ion-toolbar>
+          <ion-title>Accordion - Disabled</ion-title>
+        </ion-toolbar>
+      </ion-header>
+      <ion-content fullscreen="true" color="light">
+        <ion-header collapse="condense">
+          <ion-toolbar color="light">
+            <ion-title size="large">Accordion - Disabled</ion-title>
+          </ion-toolbar>
+        </ion-header>
+
+        <div class="grid ion-padding">
+          <div class="grid-item">
+            <ion-accordion-group>
+              <ion-accordion value="first">
+                <ion-item slot="header" color="light">
+                  <ion-label>First Accordion</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">First Content</div>
+              </ion-accordion>
+              <ion-accordion value="second" disabled="true">
+                <ion-item slot="header" color="light">
+                  <ion-label>Second Accordion (Disabled)</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Second Content</div>
+              </ion-accordion>
+              <ion-accordion value="third">
+                <ion-item slot="header" color="light">
+                  <ion-label>Third Accordion</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Third Content</div>
+              </ion-accordion>
+            </ion-accordion-group>
+          </div>
+          <div class="grid-item">
+            <ion-accordion-group disabled="true">
+              <ion-accordion value="first">
+                <ion-item slot="header" color="light">
+                  <ion-label>First Accordion in Disabled Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">First Content</div>
+              </ion-accordion>
+              <ion-accordion value="second">
+                <ion-item slot="header" color="light">
+                  <ion-label>Second Accordion in Disabled Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Second Content</div>
+              </ion-accordion>
+              <ion-accordion value="third">
+                <ion-item slot="header" color="light">
+                  <ion-label>Third Accordion in Disabled Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Third Content</div>
+              </ion-accordion>
+            </ion-accordion-group>
+          </div>
+          <div class="grid-item">
+            <ion-accordion value="second" disabled="true">
+              <ion-item slot="header" color="light">
+                <ion-label>Accordion Without Group (Disabled)</ion-label>
+              </ion-item>
+              <div class="ion-padding" slot="content">Second Content</div>
+            </ion-accordion>
+          </div>
+        </div>
+      </ion-content>
+    </ion-app>
+  </body>
+</html>
diff --git a/core/src/components/accordion/test/readonly/accordion.e2e.ts b/core/src/components/accordion/test/readonly/accordion.e2e.ts
new file mode 100644
index 00000000000..00795030468
--- /dev/null
+++ b/core/src/components/accordion/test/readonly/accordion.e2e.ts
@@ -0,0 +1,137 @@
+import { expect } from '@playwright/test';
+import { configs, test } from '@utils/test/playwright';
+
+// NOTE: these tests cannot be re-written as spec tests because the `getAccordions` method in accordion-group.tsx uses a `:scope` selector
+configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ config, title }) => {
+  test.describe(title('accordion: readonly'), () => {
+    test('should properly set readonly on child accordions', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordionGroup = page.locator('ion-accordion-group');
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveJSProperty('readonly', false);
+
+      await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => {
+        el.readonly = true;
+      });
+
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveJSProperty('readonly', true);
+    });
+
+    test('should not open accordion on click when group is readonly', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false" readonly>
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      accordion.click();
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion on click when accordion is readonly', async ({ page }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion readonly>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      accordion.click();
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion via keyboard navigation when group is readonly', async ({ page, browserName }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false" readonly>
+        <ion-accordion>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+      const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      await page.keyboard.press(tabKey);
+      await page.waitForChanges();
+
+      await page.keyboard.press('Enter');
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+
+    test('should not open accordion via keyboard navigation when accordion is readonly', async ({
+      page,
+      browserName,
+    }) => {
+      await page.setContent(
+        `
+      <ion-accordion-group animated="false">
+        <ion-accordion readonly>
+          <ion-item slot="header">Label</ion-item>
+          <div slot="content">Content</div>
+        </ion-accordion>
+      </ion-accordion-group>
+    `,
+        config
+      );
+
+      const accordion = page.locator('ion-accordion');
+      const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+
+      await page.keyboard.press(tabKey);
+      await page.waitForChanges();
+
+      await page.keyboard.press('Enter');
+      await page.waitForChanges();
+
+      await expect(accordion).toHaveClass(/accordion-collapsed/);
+    });
+  });
+});
diff --git a/core/src/components/accordion/test/readonly/index.html b/core/src/components/accordion/test/readonly/index.html
new file mode 100644
index 00000000000..7b1a9ac84c6
--- /dev/null
+++ b/core/src/components/accordion/test/readonly/index.html
@@ -0,0 +1,91 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Accordion - Readonly</title>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0" />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+    <style>
+      .grid {
+        display: grid;
+        grid-template-columns: repeat(auto-fill, minmax(250px, 1fr));
+        grid-row-gap: 20px;
+        grid-column-gap: 20px;
+      }
+    </style>
+  </head>
+  <body>
+    <ion-app>
+      <ion-header translucent="true">
+        <ion-toolbar>
+          <ion-title>Accordion - Readonly</ion-title>
+        </ion-toolbar>
+      </ion-header>
+      <ion-content fullscreen="true" color="light">
+        <ion-header collapse="condense">
+          <ion-toolbar color="light">
+            <ion-title size="large">Accordion - Readonly</ion-title>
+          </ion-toolbar>
+        </ion-header>
+
+        <div class="grid ion-padding">
+          <div class="grid-item">
+            <ion-accordion-group>
+              <ion-accordion value="first">
+                <ion-item slot="header" color="light">
+                  <ion-label>First Accordion</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">First Content</div>
+              </ion-accordion>
+              <ion-accordion value="second" readonly="true">
+                <ion-item slot="header" color="light">
+                  <ion-label>Second Accordion (Readonly)</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Second Content</div>
+              </ion-accordion>
+              <ion-accordion value="third">
+                <ion-item slot="header" color="light">
+                  <ion-label>Third Accordion</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Third Content</div>
+              </ion-accordion>
+            </ion-accordion-group>
+          </div>
+          <div class="grid-item">
+            <ion-accordion-group readonly="true">
+              <ion-accordion value="first">
+                <ion-item slot="header" color="light">
+                  <ion-label>First Accordion in Readonly Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">First Content</div>
+              </ion-accordion>
+              <ion-accordion value="second">
+                <ion-item slot="header" color="light">
+                  <ion-label>Second Accordion in Readonly Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Second Content</div>
+              </ion-accordion>
+              <ion-accordion value="third">
+                <ion-item slot="header" color="light">
+                  <ion-label>Third Accordion in Readonly Group</ion-label>
+                </ion-item>
+                <div class="ion-padding" slot="content">Third Content</div>
+              </ion-accordion>
+            </ion-accordion-group>
+          </div>
+          <div class="grid-item">
+            <ion-accordion value="second" readonly="true">
+              <ion-item slot="header" color="light">
+                <ion-label>Accordion Without Group (Readonly)</ion-label>
+              </ion-item>
+              <div class="ion-padding" slot="content">Second Content</div>
+            </ion-accordion>
+          </div>
+        </div>
+      </ion-content>
+    </ion-app>
+  </body>
+</html>