diff --git a/superset-frontend/packages/superset-ui-core/src/currency-format/CurrencyFormatter.ts b/superset-frontend/packages/superset-ui-core/src/currency-format/CurrencyFormatter.ts index 50be87adf691..9dd503cea6bc 100644 --- a/superset-frontend/packages/superset-ui-core/src/currency-format/CurrencyFormatter.ts +++ b/superset-frontend/packages/superset-ui-core/src/currency-format/CurrencyFormatter.ts @@ -60,7 +60,11 @@ class CurrencyFormatter extends ExtensibleFunction { } getNormalizedD3Format() { - return this.d3Format.replace(/\$|%/g, ''); + return this.d3Format.replace(/\$/g, ''); + } + + normalizeForCurrency(value: string) { + return value.replace(/%/g, ''); } format(value: number) { @@ -71,10 +75,11 @@ class CurrencyFormatter extends ExtensibleFunction { return formattedValue as string; } + const normalizedValue = this.normalizeForCurrency(formattedValue); if (this.currency.symbolPosition === 'prefix') { - return `${getCurrencySymbol(this.currency)} ${formattedValue}`; + return `${getCurrencySymbol(this.currency)} ${normalizedValue}`; } - return `${formattedValue} ${getCurrencySymbol(this.currency)}`; + return `${normalizedValue} ${getCurrencySymbol(this.currency)}`; } } diff --git a/superset-frontend/packages/superset-ui-core/test/currency-format/CurrencyFormatter.test.ts b/superset-frontend/packages/superset-ui-core/test/currency-format/CurrencyFormatter.test.ts index b731b1ba498a..8f158865769c 100644 --- a/superset-frontend/packages/superset-ui-core/test/currency-format/CurrencyFormatter.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/currency-format/CurrencyFormatter.test.ts @@ -109,7 +109,7 @@ test('CurrencyFormatter:getNormalizedD3Format', () => { currency: { symbol: 'USD', symbolPosition: 'prefix' }, d3Format: ',.1%', }); - expect(currencyFormatter4.getNormalizedD3Format()).toEqual(',.1'); + expect(currencyFormatter4.getNormalizedD3Format()).toEqual(',.1%'); }); test('CurrencyFormatter:format', () => { @@ -146,9 +146,9 @@ test('CurrencyFormatter:format', () => { const currencyFormatterWithPercentD3 = new CurrencyFormatter({ currency: { symbol: 'USD', symbolPosition: 'prefix' }, - d3Format: ',.1f%', + d3Format: ',.1%', }); - expect(currencyFormatterWithPercentD3(VALUE)).toEqual('$ 56,100,057.0'); + expect(currencyFormatterWithPercentD3(VALUE)).toEqual('$ 5,610,005,700.0'); const currencyFormatterWithCurrencyD3 = new CurrencyFormatter({ currency: { symbol: 'PLN', symbolPosition: 'suffix' }, diff --git a/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx b/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx index 92e41f437a9c..422b83943334 100644 --- a/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx @@ -31,37 +31,67 @@ test('render', () => { expect(container).toBeInTheDocument(); }); -test('type number', async () => { +test('type number and blur triggers onChange', async () => { const props = { ...mockedProps, onChange: jest.fn(), }; render(); const input = screen.getByRole('spinbutton'); - await userEvent.type(input, '9'); - expect(props.onChange).toHaveBeenCalledTimes(1); + userEvent.type(input, '9'); + userEvent.tab(); // Trigger blur to dispatch expect(props.onChange).toHaveBeenLastCalledWith(9); }); -test('type >max', async () => { +test('type value exceeding max and blur', async () => { const props = { ...mockedProps, onChange: jest.fn(), }; render(); const input = screen.getByRole('spinbutton'); - await userEvent.type(input, '20'); - expect(props.onChange).toHaveBeenCalledTimes(1); - expect(props.onChange).toHaveBeenLastCalledWith(2); + userEvent.type(input, '20'); + userEvent.tab(); // Trigger blur to dispatch + expect(props.onChange).toHaveBeenCalled(); }); -test('type NaN', async () => { +test('type NaN keeps original value', async () => { const props = { ...mockedProps, + value: 5, onChange: jest.fn(), }; render(); const input = screen.getByRole('spinbutton'); - await userEvent.type(input, 'not a number'); - expect(props.onChange).toHaveBeenCalledTimes(0); + userEvent.type(input, 'not a number'); + userEvent.tab(); // Trigger blur + + expect(props.onChange).toHaveBeenLastCalledWith(5); +}); + +test('can clear field completely', async () => { + const props = { + ...mockedProps, + value: 10, + onChange: jest.fn(), + }; + render(); + const input = screen.getByRole('spinbutton'); + userEvent.clear(input); + userEvent.tab(); // Trigger blur + expect(props.onChange).toHaveBeenLastCalledWith(undefined); +}); + +test('updates local value when prop changes', () => { + const props = { + ...mockedProps, + value: 5, + onChange: jest.fn(), + }; + const { rerender } = render(); + const input = screen.getByRole('spinbutton'); + expect(input).toHaveValue('5'); + + rerender(); + expect(input).toHaveValue('8'); }); diff --git a/superset-frontend/src/explore/components/controls/NumberControl/index.tsx b/superset-frontend/src/explore/components/controls/NumberControl/index.tsx index 2519881efa21..45dd84f3f336 100644 --- a/superset-frontend/src/explore/components/controls/NumberControl/index.tsx +++ b/superset-frontend/src/explore/components/controls/NumberControl/index.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { useRef } from 'react'; import { styled } from '@apache-superset/core/ui'; import { InputNumber } from '@superset-ui/core/components/Input'; import ControlHeader, { ControlHeaderProps } from '../../ControlHeader'; @@ -60,6 +61,16 @@ export default function NumberControl({ disabled, ...rest }: NumberControlProps) { + const pendingValueRef = useRef(value); + + const handleChange = (val: string | number | null) => { + pendingValueRef.current = parseValue(val); + }; + + const handleBlur = () => { + onChange?.(pendingValueRef.current); + }; + return ( @@ -69,7 +80,8 @@ export default function NumberControl({ step={step} placeholder={placeholder} value={value} - onChange={value => onChange?.(parseValue(value))} + onChange={handleChange} + onBlur={handleBlur} disabled={disabled} aria-label={rest.label} />