Skip to content

Commit

Permalink
fix(accordion): prevent opening of readonly accordion using keyboard (#…
Browse files Browse the repository at this point in the history
…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. -->
  • Loading branch information
mapsandapps committed Jan 24, 2024
1 parent e41a1a1 commit e10f49c
Show file tree
Hide file tree
Showing 6 changed files with 460 additions and 50 deletions.
5 changes: 4 additions & 1 deletion core/src/components/accordion/accordion.tsx
Expand Up @@ -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
Expand Down
49 changes: 0 additions & 49 deletions core/src/components/accordion/test/accordion.e2e.ts

This file was deleted.

137 changes: 137 additions & 0 deletions 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/);
});
});
});
91 changes: 91 additions & 0 deletions 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>

0 comments on commit e10f49c

Please sign in to comment.