Skip to content

Commit 7789bb5

Browse files
fix(range): emit ionInput when value changes (#30293)
Issue number: resolves #29619 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> 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? <!-- Please describe the behavior or changes that are being added by this PR. --> - 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 <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> 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 <brandyscarney@users.noreply.github.com>
1 parent 0f23526 commit 7789bb5

File tree

2 files changed

+57
-3
lines changed

2 files changed

+57
-3
lines changed

‎core/src/components/range/range.tsx

+21-3
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,32 @@ export class Range implements ComponentInterface {
213213
*/
214214
@Prop({ mutable: true }) value: RangeValue = 0;
215215
@Watch('value')
216-
protected valueChanged() {
216+
protected valueChanged(newValue: RangeValue, oldValue: RangeValue) {
217+
const valuesChanged = this.compareValues(newValue, oldValue);
218+
if (valuesChanged) {
219+
this.ionInput.emit({ value: this.value });
220+
}
221+
217222
if (!this.noUpdate) {
218223
this.updateRatio();
219224
}
220225
}
221226

227+
/**
228+
* Compares two RangeValue inputs to determine if they are different.
229+
*
230+
* @param newVal - The new value.
231+
* @param oldVal - The old value.
232+
* @returns `true` if the values are different, `false` otherwise.
233+
*/
234+
private compareValues = (newVal: RangeValue, oldVal: RangeValue) => {
235+
if (typeof newVal === 'object' && typeof oldVal === 'object') {
236+
return newVal.lower !== oldVal.lower || newVal.upper !== oldVal.upper;
237+
}
238+
239+
return newVal !== oldVal;
240+
};
241+
222242
private clampBounds = (value: any): number => {
223243
return clamp(this.min, value, this.max);
224244
};
@@ -591,8 +611,6 @@ export class Range implements ComponentInterface {
591611
upper: Math.max(valA, valB),
592612
};
593613

594-
this.ionInput.emit({ value: this.value });
595-
596614
this.noUpdate = false;
597615
}
598616

‎core/src/components/range/test/range-events.e2e.ts

+36
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,42 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
217217
expect(ionInputSpy).toHaveReceivedEvent();
218218
});
219219

220+
test('should not emit when the value does not change', async ({ page }, testInfo) => {
221+
testInfo.annotations.push({
222+
type: 'issue',
223+
description: 'https://github.com/ionic-team/ionic-framework/issues/29619',
224+
});
225+
226+
/**
227+
* Requires padding to prevent the knob from being clipped.
228+
* If it's clipped, then the value might be one off.
229+
* For example, if the knob is clipped on the right, then the value
230+
* will be 99 instead of 100.
231+
*/
232+
await page.setContent(
233+
`
234+
<div style="padding: 0 20px">
235+
<ion-range aria-label="range"></ion-range>
236+
</div>
237+
`,
238+
config
239+
);
240+
241+
const rangeHandle = page.locator('ion-range .range-knob-handle');
242+
const ionInputSpy = await page.spyOnEvent('ionInput');
243+
244+
const rangeHandleBoundingBox = await rangeHandle.boundingBox();
245+
const x = rangeHandleBoundingBox!.width / 2;
246+
const y = rangeHandleBoundingBox!.height / 2;
247+
248+
// Click in the middle of the knob to prevent the knob from moving.
249+
await rangeHandle.click({
250+
position: { x, y },
251+
});
252+
253+
expect(ionInputSpy).not.toHaveReceivedEvent();
254+
});
255+
220256
test('should emit when the knob is moved with the keyboard', async ({ page }) => {
221257
await page.setContent(`<ion-range aria-label="range" value="50"></ion-range>`, config);
222258

0 commit comments

Comments
 (0)