Skip to content

Commit

Permalink
feat(radio-group): ionChange will only emit from user committed chang…
Browse files Browse the repository at this point in the history
…es (#26223)
  • Loading branch information
sean-perkins committed Nov 7, 2022
1 parent 003de44 commit c299d36
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 10 deletions.
5 changes: 5 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This is a comprehensive list of the breaking changes introduced in the major ver
- [Input](#version-7x-input)
- [Modal](#version-7x-modal)
- [Overlays](#version-7x-overlays)
- [Radio Group](#version-7x-radio-group)
- [Range](#version-7x-range)
- [Searchbar](#version-7x-searchbar)
- [Segment](#version-7x-segment)
Expand Down Expand Up @@ -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.

<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>

- Range is updated to align with the design specification for supported modes.
Expand Down
4 changes: 4 additions & 0 deletions core/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5908,6 +5908,10 @@ declare namespace LocalJSX {
* Emitted when the value has changed.
*/
"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.
*/
Expand Down
1 change: 1 addition & 0 deletions core/src/components/radio-group/radio-group-interface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export interface RadioGroupChangeEventDetail<T = any> {
value: T;
event?: Event;
}

export interface RadioGroupCustomEvent<T = any> extends CustomEvent {
Expand Down
42 changes: 37 additions & 5 deletions core/src/components/radio-group/radio-group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,23 @@ export class RadioGroup implements ComponentInterface {
@Watch('value')
valueChanged(value: any | undefined) {
this.setRadioTabindex(value);

this.ionChange.emit({ value });
this.ionValueChange.emit({ value });
}

/**
* Emitted when the value has changed.
*/
@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() {
this.setRadioTabindex(this.value);
}
Expand Down Expand Up @@ -88,6 +96,17 @@ export class RadioGroup implements ComponentInterface {
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) => {
ev.preventDefault();

Expand All @@ -97,17 +116,19 @@ export class RadioGroup implements ComponentInterface {
const newValue = selectedRadio.value;
if (newValue !== currentValue) {
this.value = newValue;
this.emitValueChange(ev);
} else if (this.allowEmptySelection) {
this.value = undefined;
this.emitValueChange(ev);
}
}
};

@Listen('keydown', { target: 'document' })
onKeydown(ev: any) {
onKeydown(ev: KeyboardEvent) {
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;
}

Expand All @@ -116,7 +137,7 @@ export class RadioGroup implements ComponentInterface {
const radios = this.getRadios().filter((radio) => !radio.disabled);

// 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 current = radios[index];

Expand All @@ -139,13 +160,24 @@ export class RadioGroup implements ComponentInterface {

if (!inSelectPopover) {
this.value = next.value;
this.emitValueChange(ev);
}
}

// Update the radio group value when a user presses the
// space bar on top of a selected radio
if (['Space'].includes(ev.code)) {
const previousValue = this.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
// to the bottom of the screen
Expand Down
32 changes: 32 additions & 0 deletions core/src/components/radio-group/test/basic/radio-group.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,38 @@ test.describe('radio-group: interaction', () => {
await radioFixture.checkRadio('mouse');
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test.describe('radio-group: form', () => {
await griffRadio.click();
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 }) => {
Expand Down
104 changes: 104 additions & 0 deletions core/src/components/radio-group/test/radio-group-events.e2e.ts
Original file line number Diff line number Diff line change
@@ -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 } });
});
});
});
4 changes: 2 additions & 2 deletions core/src/components/radio/radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ export class Radio implements ComponentInterface {
const radioGroup = (this.radioGroup = this.el.closest('ion-radio-group'));
if (radioGroup) {
this.updateState();
addEventListener(radioGroup, 'ionChange', this.updateState);
addEventListener(radioGroup, 'ionValueChange', this.updateState);
}
}

disconnectedCallback() {
const radioGroup = this.radioGroup;
if (radioGroup) {
removeEventListener(radioGroup, 'ionChange', this.updateState);
removeEventListener(radioGroup, 'ionValueChange', this.updateState);
this.radioGroup = null;
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/select/test/basic/select.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ test.describe('select: ionChange', () => {
await radioButtons.nth(0).click();

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 }) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/vue/src/proxies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,8 @@ export const IonRadioGroup = /*@__PURE__*/ defineContainer<JSX.IonRadioGroup>('i
'allowEmptySelection',
'name',
'value',
'ionChange'
'ionChange',
'ionValueChange'
],
'value', 'v-ion-change', 'ionChange');

Expand Down

0 comments on commit c299d36

Please sign in to comment.