From 34c41378682a202d4fab11aad171df7f55f4c243 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Mon, 10 Oct 2022 08:36:51 -0500 Subject: [PATCH] feat(select): ionChange will only emit from user committed changes (#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. --- BREAKING.md | 5 + .../modal-example/modal-example.component.ts | 4 + .../modal-example/modal-example.component.ts | 4 + angular/test/base/e2e/src/form.spec.ts | 8 +- angular/test/base/e2e/src/inputs.spec.ts | 11 ++- angular/test/base/e2e/src/modal.spec.ts | 13 ++- .../base/src/app/inputs/inputs.component.html | 2 +- .../modal-example.component.html | 3 + .../modal-example/modal-example.component.ts | 4 + .../datetime/test/color/datetime.e2e.ts | 5 +- core/src/components/select/select.tsx | 22 ++--- .../select/test/basic/select.e2e.ts | 96 +++++++++++++++++++ 12 files changed, 153 insertions(+), 24 deletions(-) diff --git a/BREAKING.md b/BREAKING.md index 841001c4af..fd909652af 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -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) @@ -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`. + +

Select

+ +- `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.

Slides

diff --git a/angular/test/apps/ng12/src/app/modal-example/modal-example.component.ts b/angular/test/apps/ng12/src/app/modal-example/modal-example.component.ts index a9640244c8..9a1c483251 100644 --- a/angular/test/apps/ng12/src/app/modal-example/modal-example.component.ts +++ b/angular/test/apps/ng12/src/app/modal-example/modal-example.component.ts @@ -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'); diff --git a/angular/test/apps/ng13/src/app/modal-example/modal-example.component.ts b/angular/test/apps/ng13/src/app/modal-example/modal-example.component.ts index a9640244c8..9a1c483251 100644 --- a/angular/test/apps/ng13/src/app/modal-example/modal-example.component.ts +++ b/angular/test/apps/ng13/src/app/modal-example/modal-example.component.ts @@ -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'); diff --git a/angular/test/base/e2e/src/form.spec.ts b/angular/test/base/e2e/src/form.spec.ts index 37cad16281..6fe74df9f0 100644 --- a/angular/test/base/e2e/src/form.spec.ts +++ b/angular/test/base/e2e/src/form.spec.ts @@ -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); diff --git a/angular/test/base/e2e/src/inputs.spec.ts b/angular/test/base/e2e/src/inputs.spec.ts index 9991ad234e..740a4675dd 100644 --- a/angular/test/base/e2e/src/inputs.spec.ts +++ b/angular/test/base/e2e/src/inputs.spec.ts @@ -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'); }); diff --git a/angular/test/base/e2e/src/modal.spec.ts b/angular/test/base/e2e/src/modal.spec.ts index 22d80bc04a..049b83020f 100644 --- a/angular/test/base/e2e/src/modal.spec.ts +++ b/angular/test/base/e2e/src/modal.spec.ts @@ -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'); }); - }); diff --git a/angular/test/base/src/app/inputs/inputs.component.html b/angular/test/base/src/app/inputs/inputs.component.html index fa65a16dcb..60f13a7b95 100644 --- a/angular/test/base/src/app/inputs/inputs.component.html +++ b/angular/test/base/src/app/inputs/inputs.component.html @@ -23,7 +23,7 @@ Select - + No Game Console NES Nintendo64 diff --git a/angular/test/base/src/app/modal-example/modal-example.component.html b/angular/test/base/src/app/modal-example/modal-example.component.html index 513f7ad889..4f2bd52a24 100644 --- a/angular/test/base/src/app/modal-example/modal-example.component.html +++ b/angular/test/base/src/app/modal-example/modal-example.component.html @@ -31,5 +31,8 @@ Option 1 + + Set select value to "null"> + Set select value to "undefined"> diff --git a/angular/test/base/src/app/modal-example/modal-example.component.ts b/angular/test/base/src/app/modal-example/modal-example.component.ts index 7c7a6ac63a..65e9fa3aae 100644 --- a/angular/test/base/src/app/modal-example/modal-example.component.ts +++ b/angular/test/base/src/app/modal-example/modal-example.component.ts @@ -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'); diff --git a/core/src/components/datetime/test/color/datetime.e2e.ts b/core/src/components/datetime/test/color/datetime.e2e.ts index c7c6c668b6..4a62beba53 100644 --- a/core/src/components/datetime/test/color/datetime.e2e.ts +++ b/core/src/components/datetime/test/color/datetime.e2e.ts @@ -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')); @@ -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( diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index f727994a9b..18b3ca3ddb 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -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; @@ -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() { @@ -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`. @@ -279,7 +273,7 @@ export class Select implements ComponentInterface { text: option.textContent, cssClass: optClass, handler: () => { - this.value = value; + this.setValue(value); }, } as ActionSheetButton; }); @@ -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(); } @@ -463,7 +457,7 @@ export class Select implements ComponentInterface { { text: this.okText, handler: (selectedValues: any) => { - this.value = selectedValues; + this.setValue(selectedValues); }, }, ], diff --git a/core/src/components/select/test/basic/select.e2e.ts b/core/src/components/select/test/basic/select.e2e.ts index e2cba013a7..81df203e53 100644 --- a/core/src/components/select/test/basic/select.e2e.ts +++ b/core/src/components/select/test/basic/select.e2e.ts @@ -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(` + + Apple + Banana + + `); + + 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(` + + Apple + Banana + + `); + + 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(` + + Apple + Banana + + `); + + 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(` + + Apple + Banana + + `); + + 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(); + }); +});