Skip to content

Commit

Permalink
feat(select): ionChange will only emit from user committed changes (#…
Browse files Browse the repository at this point in the history
…26066)

BREAKING CHANGE:

`ionChange` is no longer emitted when the `value` of `ion-select` is modified externally. `ionChange` is only emitted from user committed changes, such as confirming a selected option in the select's overlay.
  • Loading branch information
liamdebeasi committed Oct 10, 2022
1 parent b052d3b commit 34c4137
Show file tree
Hide file tree
Showing 12 changed files with 153 additions and 24 deletions.
5 changes: 5 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This is a comprehensive list of the breaking changes introduced in the major ver
- [Range](#version-7x-range)
- [Searchbar](#version-7x-searchbar)
- [Segment](#version-7x-segment)
- [Select](#version-7x-select)
- [Slides](#version-7x-slides)
- [Textarea](#version-7x-textarea)
- [Virtual Scroll](#version-7x-virtual-scroll)
Expand Down Expand Up @@ -119,6 +120,10 @@ iOS:

- The type signature of `value` supports `string | undefined`. Previously the type signature was `string | null | undefined`.
- Developers needing to clear the checked segment item should assign a value of `''` instead of `null`.

<h4 id="version-7x-select">Select</h4>

- `ionChange` is no longer emitted when the `value` of `ion-select` is modified externally. `ionChange` is only emitted from user committed changes, such as confirming a selected option in the select's overlay.

<h4 id="version-7x-slides">Slides</h4>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export class ModalExampleComponent implements OnInit, ViewWillLeave, ViewDidEnte
this.onInit++;
}

setSelect(value: null | undefined) {
this.form.get('select').setValue(value);
}

ionViewWillEnter() {
if (this.onInit !== 1) {
throw new Error('ngOnInit was not called');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export class ModalExampleComponent implements OnInit, ViewWillLeave, ViewDidEnte
this.onInit++;
}

setSelect(value: null | undefined) {
this.form.get('select').setValue(value);
}

ionViewWillEnter() {
if (this.onInit !== 1) {
throw new Error('ngOnInit was not called');
Expand Down
8 changes: 7 additions & 1 deletion angular/test/base/e2e/src/form.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ describe('Form', () => {
// TODO: FW-1160 - Remove when v7 is released
cy.wait(300);

cy.get('ion-select').invoke('prop', 'value', 'nes');
cy.get('ion-select').click();
cy.get('ion-alert').should('exist').should('be.visible');
// NES option
cy.get('ion-alert .alert-radio-button:nth-of-type(2)').click();
// Click confirm button
cy.get('ion-alert .alert-button:not(.alert-button-role-cancel)').click();

testStatus('INVALID');

cy.get('ion-range').invoke('prop', 'value', 40);
Expand Down
11 changes: 9 additions & 2 deletions angular/test/base/e2e/src/inputs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,21 @@ describe('Inputs', () => {
cy.get('ion-input input').eq(0).blur();

cy.get('ion-datetime').invoke('prop', 'value', '1996-03-15');
cy.get('ion-select').invoke('prop', 'value', 'playstation');

cy.get('ion-select#game-console').click();
cy.get('ion-alert').should('exist').should('be.visible');
// Playstation option
cy.get('ion-alert .alert-radio-button:nth-of-type(4)').click();
// Click confirm button
cy.get('ion-alert .alert-button:not(.alert-button-role-cancel)').click();

cy.get('ion-range').invoke('prop', 'value', 20);

cy.get('#checkbox-note').should('have.text', 'true');
cy.get('#toggle-note').should('have.text', 'true');
cy.get('#input-note').should('have.text', 'hola');
cy.get('#datetime-note').should('have.text', '1996-03-15');
cy.get('#select-note').should('have.text', 'playstation');
cy.get('#select-note').should('have.text', 'ps');
cy.get('#range-note').should('have.text', '20');
});

Expand Down
13 changes: 9 additions & 4 deletions angular/test/base/e2e/src/modal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,23 @@ describe('when in a modal', () => {
});

it('should render ion-item item-has-value class when control value is set', () => {
cy.get('[formControlName="select"]').invoke('attr', 'value', 0);
cy.get('ion-select').click();
cy.get('ion-alert').should('exist').should('be.visible');
// Option 0 option
cy.get('ion-alert .alert-radio-button:nth-of-type(1)').click();
// Click confirm button
cy.get('ion-alert .alert-button:not(.alert-button-role-cancel)').click();

cy.get('#inputWithFloatingLabel').should('have.class', 'item-has-value');
});

it('should not render ion-item item-has-value class when control value is undefined', () => {
cy.get('[formControlName="select"]').invoke('attr', 'value', undefined);
cy.get('#set-to-undefined').click();
cy.get('#inputWithFloatingLabel').should('not.have.class', 'item-has-value');
});

it('should not render ion-item item-has-value class when control value is null', () => {
cy.get('[formControlName="select"]').invoke('attr', 'value', null);
cy.get('#set-to-null').click();
cy.get('#inputWithFloatingLabel').should('not.have.class', 'item-has-value');
});

});
2 changes: 1 addition & 1 deletion angular/test/base/src/app/inputs/inputs.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

<ion-item>
<ion-label>Select</ion-label>
<ion-select [(ngModel)]="select">
<ion-select [(ngModel)]="select" id="game-console">
<ion-select-option value="">No Game Console</ion-select-option>
<ion-select-option value="nes">NES</ion-select-option>
<ion-select-option value="n64" selected>Nintendo64</ion-select-option>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,8 @@ <h3>{{valueFromParams}}</h3>
<ion-select-option [value]="1">Option 1</ion-select-option>
</ion-select>
</ion-item>

<ion-button id="set-to-null" (click)="setSelect(null)">Set select value to "null"></ion-button>
<ion-button id="set-to-undefined" (click)="setSelect(undefined)">Set select value to "undefined"></ion-button>
</form>
</ion-content>
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export class ModalExampleComponent implements OnInit, ViewWillLeave, ViewDidEnte
this.onInit++;
}

setSelect(value: null | undefined) {
this.form.get('select').setValue(value);
}

ionViewWillEnter() {
if (this.onInit !== 1) {
throw new Error('ngOnInit was not called');
Expand Down
5 changes: 3 additions & 2 deletions core/src/components/datetime/test/color/datetime.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ test.describe('datetime: color', () => {
test('should not have visual regressions', async ({ page }) => {
await page.goto('/src/components/datetime/test/color');

const colorSelect = page.locator('ion-select');
const datetime = page.locator('ion-datetime');

await page.evaluate(() => document.body.classList.toggle('dark'));
Expand All @@ -19,7 +18,9 @@ test.describe('datetime: color', () => {
);

await page.evaluate(() => document.body.classList.toggle('dark'));
await colorSelect.evaluate((el: HTMLIonSelectElement) => (el.value = 'danger'));
await datetime.evaluateAll((els: HTMLIonDatetimeElement[]) => {
els.forEach((el) => (el.color = 'danger'));
});
await page.waitForChanges();

expect(await datetime.first().screenshot()).toMatchSnapshot(
Expand Down
22 changes: 8 additions & 14 deletions core/src/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import type { SelectCompareFn } from './select-interface';
export class Select implements ComponentInterface {
private inputId = `ion-sel-${selectIds++}`;
private overlay?: OverlaySelect;
private didInit = false;
private focusEl?: HTMLButtonElement;
private mutationO?: MutationObserver;

Expand Down Expand Up @@ -150,12 +149,11 @@ export class Select implements ComponentInterface {
@Watch('value')
valueChanged() {
this.emitStyle();
// TODO: FW-1160 - Remove the `didInit` property when ionChange behavior is changed in v7.
if (this.didInit) {
this.ionChange.emit({
value: this.value,
});
}
}

private setValue(value?: any | null) {
this.value = value;
this.ionChange.emit({ value });
}

async connectedCallback() {
Expand All @@ -174,10 +172,6 @@ export class Select implements ComponentInterface {
}
}

componentDidLoad() {
this.didInit = true;
}

/**
* Open the select overlay. The overlay is either an alert, action sheet, or popover,
* depending on the `interface` property on the `ion-select`.
Expand Down Expand Up @@ -279,7 +273,7 @@ export class Select implements ComponentInterface {
text: option.textContent,
cssClass: optClass,
handler: () => {
this.value = value;
this.setValue(value);
},
} as ActionSheetButton;
});
Expand Down Expand Up @@ -340,7 +334,7 @@ export class Select implements ComponentInterface {
checked: isOptionSelected(selectValue, value, this.compareWith),
disabled: option.disabled,
handler: (selected: any) => {
this.value = selected;
this.setValue(selected);
if (!this.multiple) {
this.close();
}
Expand Down Expand Up @@ -463,7 +457,7 @@ export class Select implements ComponentInterface {
{
text: this.okText,
handler: (selectedValues: any) => {
this.value = selectedValues;
this.setValue(selectedValues);
},
},
],
Expand Down
96 changes: 96 additions & 0 deletions core/src/components/select/test/basic/select.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,99 @@ test.describe('select: basic', () => {
});
});
});

test.describe('select: ionChange', () => {
test.beforeEach(({ skip }) => {
skip.rtl();
skip.mode('ios', 'ionChange has a consistent behavior across modes');
});

test('should fire ionChange when confirming a value from an alert', async ({ page }) => {
await page.setContent(`
<ion-select interface="alert">
<ion-select-option value="apple">Apple</ion-select-option>
<ion-select-option value="banana">Banana</ion-select-option>
</ion-select>
`);

const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent');
const ionChange = await page.spyOnEvent('ionChange');
const select = page.locator('ion-select');

await select.click();
await ionAlertDidPresent.next();

const alert = page.locator('ion-alert');
const radioButtons = alert.locator('.alert-radio-button');
const confirmButton = alert.locator('.alert-button:not(.alert-button-role-cancel)');

await radioButtons.nth(0).click();
await confirmButton.click();

await ionChange.next();
expect(ionChange).toHaveReceivedEventDetail({ value: 'apple' });
});

test('should fire ionChange when confirming a value from a popover', async ({ page }) => {
await page.setContent(`
<ion-select interface="popover">
<ion-select-option value="apple">Apple</ion-select-option>
<ion-select-option value="banana">Banana</ion-select-option>
</ion-select>
`);

const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
const ionChange = await page.spyOnEvent('ionChange');
const select = page.locator('ion-select');

await select.click();
await ionPopoverDidPresent.next();

const popover = page.locator('ion-popover');
const radioButtons = popover.locator('ion-radio');

await radioButtons.nth(0).click();

await ionChange.next();
expect(ionChange).toHaveReceivedEventDetail({ value: 'apple' });
});

test('should fire ionChange when confirming a value from an action sheet', async ({ page }) => {
await page.setContent(`
<ion-select interface="action-sheet">
<ion-select-option value="apple">Apple</ion-select-option>
<ion-select-option value="banana">Banana</ion-select-option>
</ion-select>
`);

const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');
const ionChange = await page.spyOnEvent('ionChange');
const select = page.locator('ion-select');

await select.click();
await ionActionSheetDidPresent.next();

const actionSheet = page.locator('ion-action-sheet');
const buttons = actionSheet.locator('.action-sheet-button');

await buttons.nth(0).click();

await ionChange.next();
expect(ionChange).toHaveReceivedEventDetail({ value: 'apple' });
});

test('should not fire when programmatically setting a valid value', async ({ page }) => {
await page.setContent(`
<ion-select>
<ion-select-option value="apple">Apple</ion-select-option>
<ion-select-option value="banana">Banana</ion-select-option>
</ion-select>
`);

const ionChange = await page.spyOnEvent('ionChange');
const select = page.locator('ion-select');

await select.evaluate((el: HTMLIonSelectElement) => (el.value = 'banana'));
await expect(ionChange).not.toHaveReceivedEvent();
});
});

0 comments on commit 34c4137

Please sign in to comment.