From 308f396389d65a5451b4a19b34e0c68c353d1127 Mon Sep 17 00:00:00 2001 From: Sean Perkins <13732623+sean-perkins@users.noreply.github.com> Date: Thu, 15 Feb 2024 17:39:26 -0500 Subject: [PATCH] refactor(angular): apply range to numeric value accessor (#29029) Issue number: Internal --------- ## What is the current behavior? The ticket describes renaming `TextValueAccessorDirective` since `ion-range` is not a text-based control, however I think this was an incorrect assumption made during the original implementation. `ion-range` is a numeric based value accessor (either as a single number or an object accepting a numeric start/end value). ## What is the new behavior? - Migrates the usage of `ion-range` value accessor implementation to the `NumericValueAccessorDirective` - Adds tests for validating the value accessor is functioning as expected ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information --------- Co-authored-by: Liam DeBeasi --- .../numeric-value-accessor.ts | 16 ++++++++++------ .../text-value-accessor.ts | 7 ++----- .../test/base/e2e/src/lazy/inputs.spec.ts | 3 +++ .../src/app/lazy/inputs/inputs.component.html | 6 ++++++ .../base/src/app/lazy/inputs/inputs.component.ts | 3 +++ 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/angular/src/directives/control-value-accessors/numeric-value-accessor.ts b/packages/angular/src/directives/control-value-accessors/numeric-value-accessor.ts index e63a91f3c7..9e79f816fe 100644 --- a/packages/angular/src/directives/control-value-accessors/numeric-value-accessor.ts +++ b/packages/angular/src/directives/control-value-accessors/numeric-value-accessor.ts @@ -3,7 +3,7 @@ import { NG_VALUE_ACCESSOR } from '@angular/forms'; import { ValueAccessor } from '@ionic/angular/common'; @Directive({ - selector: 'ion-input[type=number]', + selector: 'ion-input[type=number],ion-range', providers: [ { provide: NG_VALUE_ACCESSOR, @@ -13,18 +13,22 @@ import { ValueAccessor } from '@ionic/angular/common'; ], }) export class NumericValueAccessorDirective extends ValueAccessor { - constructor(injector: Injector, el: ElementRef) { + constructor(injector: Injector, private el: ElementRef) { super(injector, el); } @HostListener('ionInput', ['$event.target']) - handleInputEvent(el: HTMLIonInputElement): void { + handleInputEvent(el: HTMLIonInputElement | HTMLIonRangeElement): void { this.handleValueChange(el, el.value); } registerOnChange(fn: (_: number | null) => void): void { - super.registerOnChange((value: string) => { - fn(value === '' ? null : parseFloat(value)); - }); + if (this.el.nativeElement.tagName === 'ION-INPUT') { + super.registerOnChange((value: string) => { + fn(value === '' ? null : parseFloat(value)); + }); + } else { + super.registerOnChange(fn); + } } } diff --git a/packages/angular/src/directives/control-value-accessors/text-value-accessor.ts b/packages/angular/src/directives/control-value-accessors/text-value-accessor.ts index 14395dfeb4..f4b52198cf 100644 --- a/packages/angular/src/directives/control-value-accessors/text-value-accessor.ts +++ b/packages/angular/src/directives/control-value-accessors/text-value-accessor.ts @@ -2,9 +2,8 @@ import { ElementRef, Injector, Directive, HostListener } from '@angular/core'; import { NG_VALUE_ACCESSOR } from '@angular/forms'; import { ValueAccessor } from '@ionic/angular/common'; -// TODO(FW-5495): rename class since range isn't a text component @Directive({ - selector: 'ion-input:not([type=number]),ion-textarea,ion-searchbar,ion-range', + selector: 'ion-input:not([type=number]),ion-textarea,ion-searchbar', providers: [ { provide: NG_VALUE_ACCESSOR, @@ -19,9 +18,7 @@ export class TextValueAccessorDirective extends ValueAccessor { } @HostListener('ionInput', ['$event.target']) - _handleInputEvent( - el: HTMLIonInputElement | HTMLIonTextareaElement | HTMLIonSearchbarElement | HTMLIonRangeElement - ): void { + _handleInputEvent(el: HTMLIonInputElement | HTMLIonTextareaElement | HTMLIonSearchbarElement): void { this.handleValueChange(el, el.value); } } diff --git a/packages/angular/test/base/e2e/src/lazy/inputs.spec.ts b/packages/angular/test/base/e2e/src/lazy/inputs.spec.ts index bbc677bd7a..5f9bb0525a 100644 --- a/packages/angular/test/base/e2e/src/lazy/inputs.spec.ts +++ b/packages/angular/test/base/e2e/src/lazy/inputs.spec.ts @@ -10,6 +10,7 @@ describe('Inputs', () => { cy.get('ion-input').should('have.prop', 'value').and('equal', 'some text'); cy.get('ion-datetime').should('have.prop', 'value').and('equal', '1994-03-15'); cy.get('ion-select').should('have.prop', 'value').and('equal', 'nes'); + cy.get('ion-range').should('have.prop', 'value').and('equal', 50); }); it('should have reset value', () => { @@ -27,6 +28,7 @@ describe('Inputs', () => { cy.get('ion-input').should('not.have.prop', 'value'); cy.get('ion-datetime').should('not.have.prop', 'value'); cy.get('ion-select').should('not.have.prop', 'value'); + cy.get('ion-range').should('not.have.prop', 'value'); }); it('should get some value', () => { @@ -39,6 +41,7 @@ describe('Inputs', () => { cy.get('ion-input').should('have.prop', 'value').and('equal', 'some text'); cy.get('ion-datetime').should('have.prop', 'value').and('equal', '1994-03-15'); cy.get('ion-select').should('have.prop', 'value').and('equal', 'nes'); + cy.get('ion-range').should('have.prop', 'value').and('equal', 50); }); it('change values should update angular', () => { diff --git a/packages/angular/test/base/src/app/lazy/inputs/inputs.component.html b/packages/angular/test/base/src/app/lazy/inputs/inputs.component.html index 63b6264359..07cf0727e3 100644 --- a/packages/angular/test/base/src/app/lazy/inputs/inputs.component.html +++ b/packages/angular/test/base/src/app/lazy/inputs/inputs.component.html @@ -99,6 +99,12 @@ {{radio}} + + + Range + + +

Set values diff --git a/packages/angular/test/base/src/app/lazy/inputs/inputs.component.ts b/packages/angular/test/base/src/app/lazy/inputs/inputs.component.ts index a2fa0996c7..59d3b1ca8e 100644 --- a/packages/angular/test/base/src/app/lazy/inputs/inputs.component.ts +++ b/packages/angular/test/base/src/app/lazy/inputs/inputs.component.ts @@ -13,6 +13,7 @@ export class InputsComponent { toggle = true; select? = 'nes'; changes = 0; + range? = 50; setValues() { console.log('set values'); @@ -22,6 +23,7 @@ export class InputsComponent { this.radio = 'nes'; this.toggle = true; this.select = 'nes'; + this.range = 50; } resetValues() { @@ -32,6 +34,7 @@ export class InputsComponent { this.radio = undefined; this.toggle = false; this.select = undefined; + this.range = undefined; } counter() {