Skip to content

refactor: replace bounds logic with clamp and add tests #19118

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 1 commit into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/react/src/components/Layer/LayerLevel.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/**
* Copyright IBM Corp. 2023
* Copyright IBM Corp. 2023, 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

export const levels = ['one', 'two', 'three'] as const;

export const MIN_LEVEL = 0;
export const MAX_LEVEL = levels.length - 1;

export const LayerLevels = [0, 1, 2] as const;

export const MIN_LEVEL = LayerLevels[0];
export const MAX_LEVEL = LayerLevels[LayerLevels.length - 1];

export type LayerLevel = (typeof LayerLevels)[number];
8 changes: 3 additions & 5 deletions packages/react/src/components/Layer/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright IBM Corp. 2016, 2023
* Copyright IBM Corp. 2016, 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
Expand All @@ -15,6 +15,7 @@ import {
PolymorphicComponentPropWithRef,
PolymorphicRef,
} from '../../internal/PolymorphicProps';
import { clamp } from '../../internal/clamp';

/**
* A custom hook that will return information about the current layer. A common
Expand Down Expand Up @@ -64,10 +65,7 @@ const Layer = React.forwardRef<
const prefix = usePrefix();
const className = cx(`${prefix}--layer-${levels[level]}`, customClassName);
// The level should be between MIN_LEVEL and MAX_LEVEL
const value = Math.max(
MIN_LEVEL,
Math.min(level + 1, MAX_LEVEL)
) as LayerLevel;
const value = clamp(level + 1, MIN_LEVEL, MAX_LEVEL);

const BaseComponent = as || 'div';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ const NumberInput = React.forwardRef<HTMLInputElement, NumberInputProps>(
getDecimalPlaces(step)
);
const floatValue = parseFloat(rawValue.toFixed(precision));
const newValue = clamp(floatValue, min, max);
const newValue = clamp(floatValue, min ?? -Infinity, max ?? Infinity);

const state = {
value:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright IBM Corp. 2020
* Copyright IBM Corp. 2020, 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
Expand All @@ -18,6 +18,7 @@ import { usePrefix } from '../../internal/usePrefix';
import { TranslateWithId } from '../../types/common';
import { breakpoints } from '@carbon/layout';
import { useMatchMedia } from '../../internal/useMatchMedia';
import { clamp } from '../../internal/clamp';

const translationIds = {
'carbon.pagination-nav.next': 'Next',
Expand Down Expand Up @@ -350,7 +351,7 @@ const PaginationNav = React.forwardRef<HTMLElement, PaginationNavProps>(
numberOfPages = itemsShown === 4 ? itemsShown : 5;
break;
case 'sm':
numberOfPages = Math.max(4, Math.min(itemsShown, 7));
numberOfPages = clamp(itemsShown, 4, 7);
break;

default:
Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/components/Slider/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
UpperHandleFocus,
} from './SliderHandles';
import { TranslateWithId } from '../../types/common';
import { clamp } from '../../internal/clamp';

const ThumbWrapper = ({
hasTooltip = false,
Expand Down Expand Up @@ -1069,7 +1070,7 @@ class Slider extends PureComponent<SliderProps> {
range,
});
/** `leftPercentRaw` clamped between 0 and 1. */
const leftPercent = Math.min(1, Math.max(0, leftPercentRaw));
const leftPercent = clamp(leftPercentRaw, 0, 1);

if (useRawValue) {
return {
Expand Down
64 changes: 64 additions & 0 deletions packages/react/src/internal/__tests__/clamp-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Copyright IBM Corp. 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

import { clamp } from '../clamp';

describe('clamp', () => {
it('should return the value unmodified when it is within the bounds', () => {
expect(clamp(5, 1, 10)).toBe(5);
expect(clamp(1, 1, 10)).toBe(1);
expect(clamp(10, 1, 10)).toBe(10);
});

it('should clamp the value to the lower bound when below min', () => {
expect(clamp(-5, 0, 10)).toBe(0);
expect(clamp(5, 10, 20)).toBe(10);
});

it('should clamp the value to the upper bound when above max', () => {
expect(clamp(15, 0, 10)).toBe(10);
expect(clamp(5, 0, 2)).toBe(2);
});

it('should return `NaN` if any argument is not a number', () => {
expect(clamp('a', 0, 10)).toBeNaN();
expect(clamp(5, 'a', 10)).toBeNaN();
expect(clamp(5, 0, 'a')).toBeNaN();
});

it('should handle cases where the lower bound is greater than the upper bound', () => {
expect(clamp(5, 10, 1)).toBe(1);
expect(clamp(15, 10, 1)).toBe(1);
expect(clamp(-5, 10, 1)).toBe(1);
});

it('should return the bound when lower and upper bounds are equal', () => {
expect(clamp(-5, 1, 1)).toBe(1);
expect(clamp(0, 1, 1)).toBe(1);
expect(clamp(5, 1, 1)).toBe(1);
});

it('should handle explicit `Infinity` and `-Infinity` bounds', () => {
expect(clamp(Infinity, 0, 10)).toBe(10);
expect(clamp(-Infinity, 0, 10)).toBe(0);
expect(clamp(5, 0, Infinity)).toBe(5);
expect(clamp(5, -Infinity, 10)).toBe(5);
expect(clamp(5, -Infinity, Infinity)).toBe(5);
});

it('should return `NaN` when bounds are `NaN`', () => {
expect(clamp(5, NaN, 10)).toBeNaN();
expect(clamp(5, 0, NaN)).toBeNaN();
expect(clamp(5, NaN, NaN)).toBeNaN();
});

it('should work with decimal values', () => {
expect(clamp(5.5, 1.2, 10.8)).toBe(5.5);
expect(clamp(0.5, 1.2, 10.8)).toBe(1.2);
expect(clamp(11.5, 1.2, 10.8)).toBe(10.8);
});
});
14 changes: 4 additions & 10 deletions packages/react/src/internal/clamp.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
/**
* Copyright IBM Corp. 2016, 2025
* Copyright IBM Corp. 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* Synonymous to ECMA2017+ `Math.clamp`.
*
* @param {number} val
* @param {number} min
* @param {number} max
*
* @returns `val` if `max>=val>=min`; `min` if `val<min`; `max` if `val>max`.
* Clamps a number between a minimum and maximum value (inclusive).
*/
export const clamp = (value: number, min?: number, max?: number) =>
Math.min(max ?? Infinity, Math.max(min ?? -Infinity, value));
export const clamp = <T extends number>(num: number, min: T, max: T): T =>
Math.min(max, Math.max(min, num)) as T;
Loading