fix(accordion-group): do not adjust incorrect values (#26086)

BREAKING CHANGE:

Accordion Group no longer automatically adjusts the `value` property when passed an array and `multiple="false"`. Developers should update their apps to ensure they are using the API correctly.
This commit is contained in:
Liam DeBeasi
2022-10-10 09:20:52 -05:00
committed by GitHub
parent 34c4137868
commit e2cbeeb8ac
5 changed files with 24 additions and 75 deletions

View File

@ -63,7 +63,9 @@ This section details the desktop browser, JavaScript framework, and mobile platf
<h4 id="version-7x-accordion-group">Accordion Group</h4> <h4 id="version-7x-accordion-group">Accordion Group</h4>
`ionChange` is no longer emitted when the `value` of `ion-accordion-group` is modified externally. `ionChange` is only emitted from user committed changes, such as clicking or tapping the accordion header. -`ionChange` is no longer emitted when the `value` of `ion-accordion-group` is modified externally. `ionChange` is only emitted from user committed changes, such as clicking or tapping the accordion header.
- Accordion Group no longer automatically adjusts the `value` property when passed an array and `multiple="false"`. Developers should update their apps to ensure they are using the API correctly.
<h4 id="version-7x-checkbox">Checkbox</h4> <h4 id="version-7x-checkbox">Checkbox</h4>

View File

@ -73,7 +73,7 @@ export namespace Components {
*/ */
"requestAccordionToggle": (accordionValue: string | undefined, accordionExpand: boolean) => Promise<void>; "requestAccordionToggle": (accordionValue: string | undefined, accordionExpand: boolean) => Promise<void>;
/** /**
* The value of the accordion group. * The value of the accordion group. This controls which accordions are expanded. This should be an array of strings only when `multiple="true"`
*/ */
"value"?: string | string[] | null; "value"?: string | string[] | null;
} }
@ -3814,7 +3814,7 @@ declare namespace LocalJSX {
*/ */
"readonly"?: boolean; "readonly"?: boolean;
/** /**
* The value of the accordion group. * The value of the accordion group. This controls which accordions are expanded. This should be an array of strings only when `multiple="true"`
*/ */
"value"?: string | string[] | null; "value"?: string | string[] | null;
} }

View File

@ -3,6 +3,7 @@ import { Component, Element, Event, Host, Listen, Method, Prop, Watch, h } from
import { getIonMode } from '../../global/ionic-global'; import { getIonMode } from '../../global/ionic-global';
import type { AccordionGroupChangeEventDetail } from '../../interface'; import type { AccordionGroupChangeEventDetail } from '../../interface';
import { printIonWarning } from '../../utils/logging';
/** /**
* @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use. * @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use.
@ -32,7 +33,9 @@ export class AccordionGroup implements ComponentInterface {
@Prop() multiple?: boolean; @Prop() multiple?: boolean;
/** /**
* The value of the accordion group. * The value of the accordion group. This controls which
* accordions are expanded.
* This should be an array of strings only when `multiple="true"`
*/ */
@Prop({ mutable: true }) value?: string | string[] | null; @Prop({ mutable: true }) value?: string | string[] | null;
@ -74,16 +77,22 @@ export class AccordionGroup implements ComponentInterface {
valueChanged() { valueChanged() {
const { value, multiple } = this; const { value, multiple } = this;
/**
* If accordion group does not
* let multiple accordions be open
* at once, but user passes an array
* just grab the first value.
* This should emit ionChange because
* we are updating the value internally.
*/
if (!multiple && Array.isArray(value)) { if (!multiple && Array.isArray(value)) {
this.setValue(value[0]); /**
* We do some processing on the `value` array so
* that it looks more like an array when logged to
* the console.
* Example given ['a', 'b']
* Default toString() behavior: a,b
* Custom behavior: ['a', 'b']
*/
printIonWarning(
`ion-accordion-group was passed an array of values, but multiple="false". This is incorrect usage and may result in unexpected behaviors. To dismiss this warning, pass a string to the "value" property when multiple="false".
Value Passed: [${value.map((v) => `'${v}'`).join(', ')}]
`,
this.el
);
} }
/** /**

View File

@ -40,42 +40,6 @@ it('should open correct accordions', async () => {
expect(accordions[2].classList.contains('accordion-collapsed')).toEqual(true); expect(accordions[2].classList.contains('accordion-collapsed')).toEqual(true);
}); });
it('should not open more than one accordion when multiple="false"', async () => {
const page = await newSpecPage({
components: [Item, Accordion, AccordionGroup],
html: `
<ion-accordion-group animated="false">
<ion-accordion value="first">
<ion-item slot="header">Label</ion-item>
<div slot="content">Content</div>
</ion-accordion>
<ion-accordion value="second">
<ion-item slot="header">Label</ion-item>
<div slot="content">Content</div>
</ion-accordion>
<ion-accordion value="third">
<ion-item slot="header">Label</ion-item>
<div slot="content">Content</div>
</ion-accordion>
</ion-accordion-group>
`,
});
const accordionGroup = page.body.querySelector('ion-accordion-group');
const accordions = accordionGroup.querySelectorAll('ion-accordion');
accordions.forEach((accordion) => {
expect(accordion.classList.contains('accordion-collapsed')).toEqual(true);
});
accordionGroup.value = ['first', 'second'];
await page.waitForChanges();
expect(accordions[0].classList.contains('accordion-collapsed')).toEqual(false);
expect(accordions[1].classList.contains('accordion-collapsed')).toEqual(true);
expect(accordions[2].classList.contains('accordion-collapsed')).toEqual(true);
});
it('should open more than one accordion when multiple="true"', async () => { it('should open more than one accordion when multiple="true"', async () => {
const page = await newSpecPage({ const page = await newSpecPage({
components: [Item, Accordion, AccordionGroup], components: [Item, Accordion, AccordionGroup],

View File

@ -74,30 +74,4 @@ test.describe('accordion: ionChange', () => {
await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => (el.value = 'second')); await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => (el.value = 'second'));
await expect(ionChange).not.toHaveReceivedEvent(); await expect(ionChange).not.toHaveReceivedEvent();
}); });
test('should fire ionChange setting array of values on a single selection accordion', async ({ page }) => {
await page.setContent(`
<ion-accordion-group>
<ion-accordion value="first">
<button slot="header">Header</button>
<div slot="content">First Content</div>
</ion-accordion>
<ion-accordion value="second">
<button slot="header">Header</button>
<div slot="content">Second Content</div>
</ion-accordion>
<ion-accordion value="third">
<button slot="header">Header</button>
<div slot="content">Third Content</div>
</ion-accordion>
</ion-accordion-group>
`);
const ionChange = await page.spyOnEvent('ionChange');
const accordionGroup = page.locator('ion-accordion-group');
await accordionGroup.evaluate((el: HTMLIonAccordionGroupElement) => (el.value = ['second', 'third']));
await expect(ionChange).toHaveReceivedEventDetail({ value: 'second' });
});
}); });