fix(range): handle unsupported values for range min and max (#30070)

Issue number: resolves #29667

---------

## What is the current behavior?
Currently, if min/max are set to undefined on `IonRange` (which is an
accepted value), it breaks the DOM.

## What is the new behavior?
After these changes, if min/max are set to undefined or any unsupported
value (such as infinity or a NaN), it will fall back to the default
values for min and max (currently, 1 and 100 respectively).

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

---------

Co-authored-by: ShaneK <shane@shanessite.net>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
This commit is contained in:
fudom
2025-03-07 23:19:54 +01:00
committed by GitHub
parent 4df0e0f4c0
commit 322319397c
5 changed files with 64 additions and 3 deletions

View File

@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core';
import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content';
import type { Attributes } from '@utils/helpers';
import { inheritAriaAttributes, clamp, debounceEvent, renderHiddenInput } from '@utils/helpers';
import { inheritAriaAttributes, clamp, debounceEvent, renderHiddenInput, isSafeNumber } from '@utils/helpers';
import { printIonWarning } from '@utils/logging';
import { isRTL } from '@utils/rtl';
import { createColorClasses, hostContext } from '@utils/theme';
@ -109,7 +109,11 @@ export class Range implements ComponentInterface {
*/
@Prop() min = 0;
@Watch('min')
protected minChanged() {
protected minChanged(newValue: number) {
if (!isSafeNumber(newValue)) {
this.min = 0;
}
if (!this.noUpdate) {
this.updateRatio();
}
@ -120,7 +124,11 @@ export class Range implements ComponentInterface {
*/
@Prop() max = 100;
@Watch('max')
protected maxChanged() {
protected maxChanged(newValue: number) {
if (!isSafeNumber(newValue)) {
this.max = 100;
}
if (!this.noUpdate) {
this.updateRatio();
}
@ -151,6 +159,12 @@ export class Range implements ComponentInterface {
* Specifies the value granularity.
*/
@Prop() step = 1;
@Watch('step')
protected stepChanged(newValue: number) {
if (!isSafeNumber(newValue)) {
this.step = 1;
}
}
/**
* If `true`, tick marks are displayed based on the step value.
@ -300,6 +314,11 @@ export class Range implements ComponentInterface {
}
this.inheritedAttributes = inheritAriaAttributes(this.el);
// If min, max, or step are not safe, set them to 0, 100, and 1, respectively.
// Each watch does this, but not before the initial load.
this.min = isSafeNumber(this.min) ? this.min : 0;
this.max = isSafeNumber(this.max) ? this.max : 100;
this.step = isSafeNumber(this.step) ? this.step : 1;
}
componentDidLoad() {

View File

@ -28,6 +28,25 @@ describe('Range', () => {
});
});
it('should handle undefined min and max values by falling back to defaults', async () => {
const page = await newSpecPage({
components: [Range],
html: `<ion-range id="my-custom-range">
<div slot="label">Range</div>
</ion-range>`,
});
const range = page.body.querySelector('ion-range')!;
// Here we have to cast this to any, but in its react wrapper it accepts undefined as a valid value
range.min = undefined as any;
range.max = undefined as any;
range.step = undefined as any;
await page.waitForChanges();
expect(range.min).toBe(0);
expect(range.max).toBe(100);
expect(range.step).toBe(1);
});
it('should return the clamped value for a range dual knob component', () => {
sharedRange.min = 0;
sharedRange.max = 100;

View File

@ -11,6 +11,12 @@ describe('floating point utils', () => {
const n = getDecimalPlaces(5);
expect(n).toBe(0);
});
it('should handle nullish values', () => {
expect(getDecimalPlaces(undefined as any)).toBe(0);
expect(getDecimalPlaces(null as any)).toBe(0);
expect(getDecimalPlaces(NaN as any)).toBe(0);
});
});
describe('roundToMaxDecimalPlaces', () => {
@ -18,5 +24,11 @@ describe('floating point utils', () => {
const n = roundToMaxDecimalPlaces(5.12345, 1.12, 2.123);
expect(n).toBe(5.123);
});
it('should handle nullish values', () => {
expect(roundToMaxDecimalPlaces(undefined as any)).toBe(0);
expect(roundToMaxDecimalPlaces(null as any)).toBe(0);
expect(roundToMaxDecimalPlaces(NaN as any)).toBe(0);
});
});
});

View File

@ -1,4 +1,7 @@
import { isSafeNumber } from '@utils/helpers';
export function getDecimalPlaces(n: number) {
if (!isSafeNumber(n)) return 0;
if (n % 1 === 0) return 0;
return n.toString().split('.')[1].length;
}
@ -36,6 +39,7 @@ export function getDecimalPlaces(n: number) {
* be used as a reference for the desired specificity.
*/
export function roundToMaxDecimalPlaces(n: number, ...references: number[]) {
if (!isSafeNumber(n)) return 0;
const maxPlaces = Math.max(...references.map((r) => getDecimalPlaces(r)));
return Number(n.toFixed(maxPlaces));
}

View File

@ -424,3 +424,10 @@ export const getNextSiblingOfType = <T extends Element>(element: Element): T | n
}
return null;
};
/**
* Checks input for usable number. Not NaN and not Infinite.
*/
export const isSafeNumber = (input: unknown): input is number => {
return typeof input === 'number' && !isNaN(input) && isFinite(input);
};