Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 7, 2025
Merged
Prev Previous commit
Next Next commit
fix(range): handling the case where range would return NaN if max or …
…min were set to undefined by setting max and min to their default values if you try to set them directly to undefined
  • Loading branch information
ShaneK committed Mar 7, 2025
commit e01c7d728575fcbc03d7371acbcf1071e735dbbc
18 changes: 15 additions & 3 deletions core/src/components/range/range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -300,6 +308,10 @@ export class Range implements ComponentInterface {
}

this.inheritedAttributes = inheritAriaAttributes(this.el);
// If the min or max is not safe, set it to 0 or 100 respectively.
// Our 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;
}

componentDidLoad() {
Expand Down
17 changes: 17 additions & 0 deletions core/src/components/range/test/range.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ 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;
await page.waitForChanges();
expect(range.min).toBe(0);
expect(range.max).toBe(100);
});

it('should return the clamped value for a range dual knob component', () => {
sharedRange.min = 0;
sharedRange.max = 100;
Expand Down
Loading