From cd9ad38a3f13c2e21c74fba1ca08ba963639035f Mon Sep 17 00:00:00 2001 From: Sean Teramae Date: Wed, 5 Aug 2026 14:17:51 -0700 Subject: [PATCH 1/3] fix(studio): SliderWithTextInput doesn't accept out of range partial inputs Signed-off-by: Sean Teramae --- .../InferenceParameters.test.tsx | 17 +++++ .../SliderWithTextInput.test.tsx | 73 ++++++++++++++++++- .../components/SliderWithTextInput/index.tsx | 31 ++++++-- 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/web/packages/common/src/components/ModelSelectV2/InferenceParameters.test.tsx b/web/packages/common/src/components/ModelSelectV2/InferenceParameters.test.tsx index 7469d8bd13..49e545dc5e 100644 --- a/web/packages/common/src/components/ModelSelectV2/InferenceParameters.test.tsx +++ b/web/packages/common/src/components/ModelSelectV2/InferenceParameters.test.tsx @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import { InferenceParameters } from '@nemo/common/src/components/ModelSelectV2/InferenceParameters'; +import type { InferenceParams } from '@nemo/sdk/generated/platform/schema'; import { fireEvent, render, screen } from '@testing-library/react'; +import { useState } from 'react'; const renderComponent = (props: Partial> = {}) => { const onChange = vi.fn(); @@ -63,6 +65,21 @@ describe('InferenceParameters', () => { ); }); + it('lets the user clear the temperature field and type a new value', () => { + const StatefulHarness = () => { + const [value, setValue] = useState>({ temperature: 1 }); + return ; + }; + render(); + const temperature = screen.getAllByRole('spinbutton')[0]; + + fireEvent.change(temperature, { target: { value: '' } }); + expect(temperature).toHaveValue(null); + + fireEvent.change(temperature, { target: { value: '0.7' } }); + expect(temperature).toHaveValue(0.7); + }); + it('disables all inputs when disabled', () => { renderComponent({ disabled: true }); const inputs = screen.getAllByRole('spinbutton'); diff --git a/web/packages/common/src/components/SliderWithTextInput/SliderWithTextInput.test.tsx b/web/packages/common/src/components/SliderWithTextInput/SliderWithTextInput.test.tsx index 0bbb0efbe7..52af7e0be8 100644 --- a/web/packages/common/src/components/SliderWithTextInput/SliderWithTextInput.test.tsx +++ b/web/packages/common/src/components/SliderWithTextInput/SliderWithTextInput.test.tsx @@ -249,18 +249,43 @@ describe('SliderWithTextInput', () => { /> ); - const slider = screen.getByRole('spinbutton'); + const slider = screen.getByRole('slider'); // Test value above max - fireEvent.change(slider, { target: { value: '150' } }); + fireEvent.input(slider, { target: { value: '150' } }); expect(mockOnChange).toHaveBeenCalledWith(90); // Test value below min - fireEvent.change(slider, { target: { value: '5' } }); + fireEvent.input(slider, { target: { value: '5' } }); expect(mockOnChange).toHaveBeenCalledWith(10); }); - it('should clamp values to min/max bounds when text input changes', () => { + it('should not clamp text input values while the user is still typing', () => { + const mockOnChange = vi.fn(); + const field = createMockField(50, mockOnChange); + render( + + ); + + const textInput = screen.getByRole('spinbutton'); + + // '5' is below min but is a valid prefix of '55', so it must survive. + fireEvent.change(textInput, { target: { value: '5' } }); + expect(mockOnChange).not.toHaveBeenCalled(); + expect(textInput).toHaveValue(5); + + fireEvent.change(textInput, { target: { value: '55' } }); + expect(mockOnChange).toHaveBeenCalledWith(55); + }); + + it('should clamp out-of-range text input values on blur', () => { const mockOnChange = vi.fn(); const field = createMockField(50, mockOnChange); render( @@ -278,12 +303,52 @@ describe('SliderWithTextInput', () => { // Test value above max fireEvent.change(textInput, { target: { value: '150' } }); + fireEvent.blur(textInput); expect(mockOnChange).toHaveBeenCalledWith(90); // Test value below min fireEvent.change(textInput, { target: { value: '5' } }); + fireEvent.blur(textInput); expect(mockOnChange).toHaveBeenCalledWith(10); }); + + it('should allow typing a value whose prefix is below min', () => { + const mockOnChange = vi.fn(); + const field = createMockField(1, mockOnChange); + const { rerender } = render( + + ); + + const textInput = screen.getByRole('spinbutton'); + + // Typing '0' must not be rewritten to the 0.1 minimum. + fireEvent.change(textInput, { target: { value: '0' } }); + expect(mockOnChange).not.toHaveBeenCalled(); + expect(textInput).toHaveValue(0); + + fireEvent.change(textInput, { target: { value: '0.7' } }); + expect(mockOnChange).toHaveBeenCalledWith(0.7); + + rerender( + + ); + fireEvent.blur(textInput); + expect(textInput).toHaveValue(0.7); + }); }); describe('Reset Functionality', () => { diff --git a/web/packages/common/src/components/SliderWithTextInput/index.tsx b/web/packages/common/src/components/SliderWithTextInput/index.tsx index 352857ccec..4102c96368 100644 --- a/web/packages/common/src/components/SliderWithTextInput/index.tsx +++ b/web/packages/common/src/components/SliderWithTextInput/index.tsx @@ -16,7 +16,7 @@ import { Tooltip, } from '@nvidia/foundations-react-core'; import { Info, RotateCcw } from 'lucide-react'; -import { ComponentProps, ReactNode } from 'react'; +import { ComponentProps, ReactNode, useState } from 'react'; import { FieldValues } from 'react-hook-form'; type SliderProps = ComponentProps; @@ -57,12 +57,19 @@ export const SliderWithTextInput = ({ slotEnd, displayName, }: SliderWithTextInputProps) => { + const [draftValue, setDraftValue] = useState(null); + + const clamp = (value: number) => Math.min(Math.max(value, min), max); + const handleSliderChange = (newValue: number) => { - const clampedValue = Math.min(Math.max(newValue, min), max); + const clampedValue = clamp(newValue); + setDraftValue(null); field.onChange(clampedValue); attributes?.Slider?.onValueChange?.(clampedValue); }; const handleTextInputChange = (newValue: string, event: React.ChangeEvent) => { + setDraftValue(newValue); + if (event.target.validity?.badInput) return; if (newValue === '') { field.onChange(undefined); attributes?.TextInput?.onValueChange?.('', event); @@ -70,18 +77,31 @@ export const SliderWithTextInput = ({ } const numberValue = parseFloat(newValue); if (Number.isNaN(numberValue)) return; - const clampedValue = Math.min(Math.max(numberValue, min), max); + if (numberValue < min || numberValue > max) return; + field.onChange(numberValue); + attributes?.TextInput?.onValueChange?.(numberValue.toString(), event); + }; + const handleTextInputBlur = (event: React.FocusEvent) => { + const rawValue = draftValue; + setDraftValue(null); + attributes?.TextInput?.onBlur?.(event); + if (rawValue === null || rawValue === '') return; + const numberValue = parseFloat(rawValue); + if (Number.isNaN(numberValue)) return; + const clampedValue = clamp(numberValue); + if (clampedValue === field.value) return; field.onChange(clampedValue); - attributes?.TextInput?.onValueChange?.(clampedValue.toString(), event); + attributes?.Slider?.onValueChange?.(clampedValue); }; const handleReset = () => { + setDraftValue(null); field.onChange(defaultValue); attributes?.Slider?.onValueChange?.(defaultValue); }; const fallback = defaultValue ?? min; const isFieldValueNumber = typeof field.value === 'number' && !Number.isNaN(field.value); const safeFieldValue = isFieldValueNumber ? field.value : fallback; - const textInputValue = isFieldValueNumber ? field.value.toString() : ''; + const textInputValue = draftValue ?? (isFieldValueNumber ? field.value.toString() : ''); const stepMarkerClassNames = 'pb-5 [&_.nv-slider-step:first-of-type]:items-start [&_.nv-slider-step:last-of-type]:items-end'; @@ -156,6 +176,7 @@ export const SliderWithTextInput = ({ className={`${textInputWidth} h-[40px] shrink-0`} {...attributes?.TextInput} onValueChange={handleTextInputChange} + onBlur={handleTextInputBlur} attributes={{ Input: { 'aria-label': `${id || 'slider'}_text_input`, From eedd8b4ba753cf9d28f0fe856cf579dcb5d3e355 Mon Sep 17 00:00:00 2001 From: Sean Teramae Date: Thu, 6 Aug 2026 13:21:54 -0700 Subject: [PATCH 2/3] fix test Signed-off-by: Sean Teramae --- .../components/AdvancedParameters.test.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/packages/studio/src/routes/SafeSynthesizerNewRoute/components/AdvancedParameters.test.tsx b/web/packages/studio/src/routes/SafeSynthesizerNewRoute/components/AdvancedParameters.test.tsx index 6483648eee..b25d428064 100644 --- a/web/packages/studio/src/routes/SafeSynthesizerNewRoute/components/AdvancedParameters.test.tsx +++ b/web/packages/studio/src/routes/SafeSynthesizerNewRoute/components/AdvancedParameters.test.tsx @@ -223,6 +223,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '3'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeLessThanOrEqual(2); @@ -302,6 +303,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '-0.5'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeGreaterThanOrEqual(0); @@ -323,6 +325,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '1.5'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeLessThanOrEqual(1); @@ -428,6 +431,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '0.5'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeGreaterThanOrEqual(1); @@ -452,6 +456,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '3'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeLessThanOrEqual(2); @@ -742,6 +747,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '0'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeGreaterThanOrEqual(1); @@ -775,6 +781,7 @@ describe('AdvancedParameters', () => { await user.clear(numberInput); await user.type(numberInput, '10'); + await user.tab(); await waitFor(() => { expect(Number(numberInput.value)).toBeLessThanOrEqual(6); From 6d93ac5c69002c5c63c7703cf8806446bab28100 Mon Sep 17 00:00:00 2001 From: Sean Teramae Date: Mon, 10 Aug 2026 10:51:55 -0700 Subject: [PATCH 3/3] clamp to util Signed-off-by: Sean Teramae --- .../common/src/components/SliderWithTextInput/index.tsx | 7 +++---- web/packages/common/src/utils/math.ts | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 web/packages/common/src/utils/math.ts diff --git a/web/packages/common/src/components/SliderWithTextInput/index.tsx b/web/packages/common/src/components/SliderWithTextInput/index.tsx index 4102c96368..4d5bac8e80 100644 --- a/web/packages/common/src/components/SliderWithTextInput/index.tsx +++ b/web/packages/common/src/components/SliderWithTextInput/index.tsx @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { toScientificNotation } from '@nemo/common/src/utils/formatters'; +import { clamp } from '@nemo/common/src/utils/math'; import { Block, Button, @@ -59,10 +60,8 @@ export const SliderWithTextInput = ({ }: SliderWithTextInputProps) => { const [draftValue, setDraftValue] = useState(null); - const clamp = (value: number) => Math.min(Math.max(value, min), max); - const handleSliderChange = (newValue: number) => { - const clampedValue = clamp(newValue); + const clampedValue = clamp(newValue, min, max); setDraftValue(null); field.onChange(clampedValue); attributes?.Slider?.onValueChange?.(clampedValue); @@ -88,7 +87,7 @@ export const SliderWithTextInput = ({ if (rawValue === null || rawValue === '') return; const numberValue = parseFloat(rawValue); if (Number.isNaN(numberValue)) return; - const clampedValue = clamp(numberValue); + const clampedValue = clamp(numberValue, min, max); if (clampedValue === field.value) return; field.onChange(clampedValue); attributes?.Slider?.onValueChange?.(clampedValue); diff --git a/web/packages/common/src/utils/math.ts b/web/packages/common/src/utils/math.ts new file mode 100644 index 0000000000..4a848cd193 --- /dev/null +++ b/web/packages/common/src/utils/math.ts @@ -0,0 +1,5 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export const clamp = (value: number, min: number, max: number) => + Math.min(Math.max(value, min), max);