feat(radio-group): ionChange will only emit from user committed changes (#26223)

This commit is contained in:
Sean Perkins
2022-11-07 18:03:08 -05:00
committed by GitHub
parent 003de44d92
commit c299d3666a
10 changed files with 189 additions and 10 deletions

View File

@ -21,6 +21,7 @@ This is a comprehensive list of the breaking changes introduced in the major ver
- [Input](#version-7x-input) - [Input](#version-7x-input)
- [Modal](#version-7x-modal) - [Modal](#version-7x-modal)
- [Overlays](#version-7x-overlays) - [Overlays](#version-7x-overlays)
- [Radio Group](#version-7x-radio-group)
- [Range](#version-7x-range) - [Range](#version-7x-range)
- [Searchbar](#version-7x-searchbar) - [Searchbar](#version-7x-searchbar)
- [Segment](#version-7x-segment) - [Segment](#version-7x-segment)
@ -107,6 +108,10 @@ This section details the desktop browser, JavaScript framework, and mobile platf
Ionic now listens on the `keydown` event instead of the `keyup` event when determining when to dismiss overlays via the "Escape" key. Any applications that were listening on `keyup` to suppress this behavior should listen on `keydown` instead. Ionic now listens on the `keydown` event instead of the `keyup` event when determining when to dismiss overlays via the "Escape" key. Any applications that were listening on `keyup` to suppress this behavior should listen on `keydown` instead.
<h4 id="version-7x-radio-group">Radio Group</h4>
- `ionChange` is no longer emitted when the `value` of `ion-radio-group` is modified externally. `ionChange` is only emitted from user committed changes, such as clicking or tapping an `ion-radio` in the group.
<h4 id="version-7x-range">Range</h4> <h4 id="version-7x-range">Range</h4>
- Range is updated to align with the design specification for supported modes. - Range is updated to align with the design specification for supported modes.

View File

@ -5908,6 +5908,10 @@ declare namespace LocalJSX {
* Emitted when the value has changed. * Emitted when the value has changed.
*/ */
"onIonChange"?: (event: IonRadioGroupCustomEvent<RadioGroupChangeEventDetail>) => void; "onIonChange"?: (event: IonRadioGroupCustomEvent<RadioGroupChangeEventDetail>) => void;
/**
* Emitted when the `value` property has changed. This is used to ensure that `ion-radio` can respond to any value property changes from the group.
*/
"onIonValueChange"?: (event: IonRadioGroupCustomEvent<RadioGroupChangeEventDetail>) => void;
/** /**
* the value of the radio group. * the value of the radio group.
*/ */

View File

@ -1,5 +1,6 @@
export interface RadioGroupChangeEventDetail<T = any> { export interface RadioGroupChangeEventDetail<T = any> {
value: T; value: T;
event?: Event;
} }
export interface RadioGroupCustomEvent<T = any> extends CustomEvent { export interface RadioGroupCustomEvent<T = any> extends CustomEvent {

View File

@ -38,8 +38,7 @@ export class RadioGroup implements ComponentInterface {
@Watch('value') @Watch('value')
valueChanged(value: any | undefined) { valueChanged(value: any | undefined) {
this.setRadioTabindex(value); this.setRadioTabindex(value);
this.ionValueChange.emit({ value });
this.ionChange.emit({ value });
} }
/** /**
@ -47,6 +46,15 @@ export class RadioGroup implements ComponentInterface {
*/ */
@Event() ionChange!: EventEmitter<RadioGroupChangeEventDetail>; @Event() ionChange!: EventEmitter<RadioGroupChangeEventDetail>;
/**
* Emitted when the `value` property has changed.
* This is used to ensure that `ion-radio` can respond
* to any value property changes from the group.
*
* @internal
*/
@Event() ionValueChange!: EventEmitter<RadioGroupChangeEventDetail>;
componentDidLoad() { componentDidLoad() {
this.setRadioTabindex(this.value); this.setRadioTabindex(this.value);
} }
@ -88,6 +96,17 @@ export class RadioGroup implements ComponentInterface {
return Array.from(this.el.querySelectorAll('ion-radio')); return Array.from(this.el.querySelectorAll('ion-radio'));
} }
/**
* Emits an `ionChange` event.
*
* This API should be called for user committed changes.
* This API should not be used for external value changes.
*/
private emitValueChange(event?: Event) {
const { value } = this;
this.ionChange.emit({ value, event });
}
private onClick = (ev: Event) => { private onClick = (ev: Event) => {
ev.preventDefault(); ev.preventDefault();
@ -97,17 +116,19 @@ export class RadioGroup implements ComponentInterface {
const newValue = selectedRadio.value; const newValue = selectedRadio.value;
if (newValue !== currentValue) { if (newValue !== currentValue) {
this.value = newValue; this.value = newValue;
this.emitValueChange(ev);
} else if (this.allowEmptySelection) { } else if (this.allowEmptySelection) {
this.value = undefined; this.value = undefined;
this.emitValueChange(ev);
} }
} }
}; };
@Listen('keydown', { target: 'document' }) @Listen('keydown', { target: 'document' })
onKeydown(ev: any) { onKeydown(ev: KeyboardEvent) {
const inSelectPopover = !!this.el.closest('ion-select-popover'); const inSelectPopover = !!this.el.closest('ion-select-popover');
if (ev.target && !this.el.contains(ev.target)) { if (ev.target && !this.el.contains(ev.target as HTMLElement)) {
return; return;
} }
@ -116,7 +137,7 @@ export class RadioGroup implements ComponentInterface {
const radios = this.getRadios().filter((radio) => !radio.disabled); const radios = this.getRadios().filter((radio) => !radio.disabled);
// Only move the radio if the current focus is in the radio group // Only move the radio if the current focus is in the radio group
if (ev.target && radios.includes(ev.target)) { if (ev.target && radios.includes(ev.target as HTMLIonRadioElement)) {
const index = radios.findIndex((radio) => radio === ev.target); const index = radios.findIndex((radio) => radio === ev.target);
const current = radios[index]; const current = radios[index];
@ -139,13 +160,24 @@ export class RadioGroup implements ComponentInterface {
if (!inSelectPopover) { if (!inSelectPopover) {
this.value = next.value; this.value = next.value;
this.emitValueChange(ev);
} }
} }
// Update the radio group value when a user presses the // Update the radio group value when a user presses the
// space bar on top of a selected radio // space bar on top of a selected radio
if (['Space'].includes(ev.code)) { if (['Space'].includes(ev.code)) {
const previousValue = this.value;
this.value = this.allowEmptySelection && this.value !== undefined ? undefined : current.value; this.value = this.allowEmptySelection && this.value !== undefined ? undefined : current.value;
if (previousValue !== this.value || this.allowEmptySelection) {
/**
* Value change should only be emitted if the value is different,
* such as selecting a new radio with the space bar or if
* the radio group allows for empty selection and the user
* is deselecting a checked radio.
*/
this.emitValueChange(ev);
}
// Prevent browsers from jumping // Prevent browsers from jumping
// to the bottom of the screen // to the bottom of the screen

View File

@ -76,6 +76,38 @@ test.describe('radio-group: interaction', () => {
await radioFixture.checkRadio('mouse'); await radioFixture.checkRadio('mouse');
await radioFixture.expectChecked(false); await radioFixture.expectChecked(false);
}); });
test('programmatically assigning a value should update the checked radio', async ({ page }) => {
await page.setContent(`
<ion-radio-group value="1">
<ion-item>
<ion-label>Item 1</ion-label>
<ion-radio value="1" slot="start"></ion-radio>
</ion-item>
<ion-item>
<ion-label>Item 2</ion-label>
<ion-radio value="2" slot="start"></ion-radio>
</ion-item>
<ion-item>
<ion-label>Item 3</ion-label>
<ion-radio value="3" slot="start"></ion-radio>
</ion-item>
</ion-radio-group>
`);
const radioGroup = page.locator('ion-radio-group');
const radioOne = page.locator('ion-radio[value="1"]');
const radioTwo = page.locator('ion-radio[value="2"]');
await radioGroup.evaluate((el: HTMLIonRadioGroupElement) => (el.value = '2'));
await page.waitForChanges();
expect(radioOne).not.toHaveClass(/radio-checked/);
expect(radioTwo).toHaveClass(/radio-checked/);
});
}); });
class RadioFixture { class RadioFixture {

View File

@ -16,7 +16,7 @@ test.describe('radio-group: form', () => {
await griffRadio.click(); await griffRadio.click();
await page.waitForChanges(); await page.waitForChanges();
await expect(ionChange).toHaveReceivedEventDetail({ value: 'griff' }); await expect(ionChange).toHaveReceivedEventDetail({ value: 'griff', event: { isTrusted: true } });
}); });
test('selecting a disabled option should not update the value', async ({ page }) => { test('selecting a disabled option should not update the value', async ({ page }) => {

View File

@ -0,0 +1,104 @@
import { expect } from '@playwright/test';
import { test } from '@utils/test/playwright';
test.describe('radio group: events: ionChange', () => {
test.beforeEach(({ skip }) => {
skip.rtl();
skip.mode('md');
});
test('should emit when selecting an unchecked radio', async ({ page }) => {
await page.setContent(`
<ion-radio-group value="1">
<ion-radio value="1"></ion-radio>
<ion-radio value="2"></ion-radio>
<ion-radio value="3"></ion-radio>
</ion-radio-group>
`);
const ionChangeSpy = await page.spyOnEvent('ionChange');
await page.click('ion-radio[value="2"]');
await ionChangeSpy.next();
expect(ionChangeSpy).toHaveReceivedEventTimes(1);
expect(ionChangeSpy).toHaveReceivedEventDetail({ value: '2', event: { isTrusted: true } });
});
test('should emit when the radio group does not have an initial value', async ({ page }) => {
await page.setContent(`
<ion-radio-group>
<ion-radio value="1"></ion-radio>
<ion-radio value="2"></ion-radio>
<ion-radio value="3"></ion-radio>
</ion-radio-group>
`);
const ionChangeSpy = await page.spyOnEvent('ionChange');
await page.click('ion-radio[value="2"]');
await ionChangeSpy.next();
expect(ionChangeSpy).toHaveReceivedEventTimes(1);
expect(ionChangeSpy).toHaveReceivedEventDetail({ value: '2', event: { isTrusted: true } });
});
test('should not emit when selecting a checked radio', async ({ page }) => {
await page.setContent(`
<ion-radio-group value="1">
<ion-radio value="1"></ion-radio>
<ion-radio value="2"></ion-radio>
<ion-radio value="3"></ion-radio>
</ion-radio-group>
`);
const ionChangeSpy = await page.spyOnEvent('ionChange');
await page.click('ion-radio[value="1"]');
await page.waitForChanges();
expect(ionChangeSpy).toHaveReceivedEventTimes(0);
});
test('should not emit if the value is set programmatically', async ({ page }) => {
await page.setContent(`
<ion-radio-group value="1">
<ion-radio value="1"></ion-radio>
<ion-radio value="2"></ion-radio>
<ion-radio value="3"></ion-radio>
</ion-radio-group>
`);
const radioGroup = page.locator('ion-radio-group');
const ionChangeSpy = await page.spyOnEvent('ionChange');
await radioGroup.evaluate((el: HTMLIonRadioGroupElement) => (el.value = '2'));
expect(ionChangeSpy).toHaveReceivedEventTimes(0);
expect(await radioGroup.evaluate((el: HTMLIonRadioGroupElement) => el.value)).toBe('2');
});
test.describe('allowEmptySelection', () => {
test('should emit when selecting a checked radio', async ({ page }) => {
await page.setContent(`
<ion-radio-group allow-empty-selection="true" value="1">
<ion-radio value="1"></ion-radio>
<ion-radio value="2"></ion-radio>
<ion-radio value="3"></ion-radio>
</ion-radio-group>
`);
const ionChangeSpy = await page.spyOnEvent('ionChange');
await page.click('ion-radio[value="1"]');
await ionChangeSpy.next();
expect(ionChangeSpy).toHaveReceivedEventTimes(1);
expect(ionChangeSpy).toHaveReceivedEventDetail({ value: undefined, event: { isTrusted: true } });
});
});
});

View File

@ -98,14 +98,14 @@ export class Radio implements ComponentInterface {
const radioGroup = (this.radioGroup = this.el.closest('ion-radio-group')); const radioGroup = (this.radioGroup = this.el.closest('ion-radio-group'));
if (radioGroup) { if (radioGroup) {
this.updateState(); this.updateState();
addEventListener(radioGroup, 'ionChange', this.updateState); addEventListener(radioGroup, 'ionValueChange', this.updateState);
} }
} }
disconnectedCallback() { disconnectedCallback() {
const radioGroup = this.radioGroup; const radioGroup = this.radioGroup;
if (radioGroup) { if (radioGroup) {
removeEventListener(radioGroup, 'ionChange', this.updateState); removeEventListener(radioGroup, 'ionValueChange', this.updateState);
this.radioGroup = null; this.radioGroup = null;
} }
} }

View File

@ -149,7 +149,7 @@ test.describe('select: ionChange', () => {
await radioButtons.nth(0).click(); await radioButtons.nth(0).click();
await ionChange.next(); await ionChange.next();
expect(ionChange).toHaveReceivedEventDetail({ value: 'apple' }); expect(ionChange).toHaveReceivedEventDetail({ value: 'apple', event: { isTrusted: true } });
}); });
test('should fire ionChange when confirming a value from an action sheet', async ({ page }) => { test('should fire ionChange when confirming a value from an action sheet', async ({ page }) => {

View File

@ -580,7 +580,8 @@ export const IonRadioGroup = /*@__PURE__*/ defineContainer<JSX.IonRadioGroup>('i
'allowEmptySelection', 'allowEmptySelection',
'name', 'name',
'value', 'value',
'ionChange' 'ionChange',
'ionValueChange'
], ],
'value', 'v-ion-change', 'ionChange'); 'value', 'v-ion-change', 'ionChange');