From 7789bb59ee5c76074ff4872dc6a50ae2d83df8f5 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Mon, 24 Mar 2025 14:06:08 -0700 Subject: [PATCH] fix(range): emit ionInput when value changes (#30293) Issue number: resolves #29619 --------- ## What is the current behavior? The `ionInput` emits for range even when the value hasn't changed. This does not match our documentation. It should only emit when the value changes (and continuously while the user is dragging the knob). ## What is the new behavior? - Moved the emitter to the value watch function, to determine if the value has changed. - Added a test - ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information How to test: 1. Navigate to the [range basic HTML file](https://github.com/ionic-team/ionic-framework/blob/8ed08fcba59c9fde5c6d08d721fd1d00642de4f9/core/src/components/range/test/basic/index.html#L77) 2. Add the following script ``` const ionicRanges = document.querySelectorAll('ion-range'); ionicRanges.forEach(range => { range.addEventListener('ionInput', function(ev) { console.log('ionInput', ev.currentTarget.value); }); }); ``` 3. Navigate to the [range test page](http://localhost:3333/src/components/range/test/basic) 4. Open the console 5. Move the single knob range (let go when you're done) 6. Verify that the value is shown in the console 7. Tap as close to the middle of the knob. The goal is to tap it without the value moving. 8. Verify that the value does not show in the console --------- Co-authored-by: Brandy Smith --- core/src/components/range/range.tsx | 24 +++++++++++-- .../components/range/test/range-events.e2e.ts | 36 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index b36617ad3e..478b2a57e2 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -213,12 +213,32 @@ export class Range implements ComponentInterface { */ @Prop({ mutable: true }) value: RangeValue = 0; @Watch('value') - protected valueChanged() { + protected valueChanged(newValue: RangeValue, oldValue: RangeValue) { + const valuesChanged = this.compareValues(newValue, oldValue); + if (valuesChanged) { + this.ionInput.emit({ value: this.value }); + } + if (!this.noUpdate) { this.updateRatio(); } } + /** + * Compares two RangeValue inputs to determine if they are different. + * + * @param newVal - The new value. + * @param oldVal - The old value. + * @returns `true` if the values are different, `false` otherwise. + */ + private compareValues = (newVal: RangeValue, oldVal: RangeValue) => { + if (typeof newVal === 'object' && typeof oldVal === 'object') { + return newVal.lower !== oldVal.lower || newVal.upper !== oldVal.upper; + } + + return newVal !== oldVal; + }; + private clampBounds = (value: any): number => { return clamp(this.min, value, this.max); }; @@ -591,8 +611,6 @@ export class Range implements ComponentInterface { upper: Math.max(valA, valB), }; - this.ionInput.emit({ value: this.value }); - this.noUpdate = false; } diff --git a/core/src/components/range/test/range-events.e2e.ts b/core/src/components/range/test/range-events.e2e.ts index bfbdfac48e..f76d147627 100644 --- a/core/src/components/range/test/range-events.e2e.ts +++ b/core/src/components/range/test/range-events.e2e.ts @@ -217,6 +217,42 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => expect(ionInputSpy).toHaveReceivedEvent(); }); + test('should not emit when the value does not change', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29619', + }); + + /** + * Requires padding to prevent the knob from being clipped. + * If it's clipped, then the value might be one off. + * For example, if the knob is clipped on the right, then the value + * will be 99 instead of 100. + */ + await page.setContent( + ` +
+ +
+ `, + config + ); + + const rangeHandle = page.locator('ion-range .range-knob-handle'); + const ionInputSpy = await page.spyOnEvent('ionInput'); + + const rangeHandleBoundingBox = await rangeHandle.boundingBox(); + const x = rangeHandleBoundingBox!.width / 2; + const y = rangeHandleBoundingBox!.height / 2; + + // Click in the middle of the knob to prevent the knob from moving. + await rangeHandle.click({ + position: { x, y }, + }); + + expect(ionInputSpy).not.toHaveReceivedEvent(); + }); + test('should emit when the knob is moved with the keyboard', async ({ page }) => { await page.setContent(``, config);