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..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, @@ -16,7 +17,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 +58,17 @@ export const SliderWithTextInput = ({ slotEnd, displayName, }: SliderWithTextInputProps) => { + const [draftValue, setDraftValue] = useState(null); + const handleSliderChange = (newValue: number) => { - const clampedValue = Math.min(Math.max(newValue, min), max); + const clampedValue = clamp(newValue, min, max); + 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 +76,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, min, max); + 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 +175,7 @@ export const SliderWithTextInput = ({ className={`${textInputWidth} h-[40px] shrink-0`} {...attributes?.TextInput} onValueChange={handleTextInputChange} + onBlur={handleTextInputBlur} attributes={{ Input: { 'aria-label': `${id || 'slider'}_text_input`, 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); 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);