Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 2 additions & 2 deletions examples/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const ExampleForm = ({
<br />
<input type="checkbox" name="disableSelectionHandling" defaultChecked={disableSelectionHandling} /> Disable selection handling
<br />
{/* <input type="text" name="value" placeholder="Value" defaultValue={value} />Value<br /> */}
<input type="text" name="value" placeholder="Value" defaultValue={value} />Value<br />
<input
type="text"
name="decimalSeparator"
Expand Down Expand Up @@ -163,7 +163,7 @@ function FormContainer() {
setDisableSelectionHandling(
document.getElementsByName("disableSelectionHandling")[0].checked
);
// setValue(document.getElementsByName('value')[0].value);
setValue(document.getElementsByName('value')[0].value);
setDecimalSeparator(document.getElementsByName("decimalSeparator")[0].value);
setThousandSeparator(
document.getElementsByName("thousandSeparator")[0].value
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ericblade/react-currency-input",
"version": "1.4.4",
"version": "1.4.5",
"description": "React component for inputting currency amounts",
"main": "dist/cjs/index.js",
"module": "dist/esm/index.js",
Expand Down Expand Up @@ -71,4 +71,4 @@
"publishConfig": {
"access": "public"
}
}
}
10 changes: 10 additions & 0 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ export default defineConfig({
testMatch: '**/mask.spec.ts',
dependencies: ['base tests'],
},
{
name: 'controlled-value tests',
testMatch: '**/controlled-value.spec.ts',

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new 'controlled-value tests' project doesn't specify any dependencies, unlike other test projects like 'mask tests' which depends on 'base tests'. If there are any setup requirements or if these tests should run after base tests complete, consider adding a dependencies array to ensure proper test execution order.

Suggested change
testMatch: '**/controlled-value.spec.ts',
testMatch: '**/controlled-value.spec.ts',
dependencies: ['base tests'],

Copilot uses AI. Check for mistakes.
dependencies: ['base tests'],
},
{
name: 'input-type tests',
testMatch: '**/input-type.spec.ts',
dependencies: ['base tests'],
},
Comment on lines +46 to +55

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test file 'tests/input-type.spec.ts' is not configured as a project in playwright.config.ts, which means these tests won't run. Add a new project configuration for 'input-type tests' with testMatch pattern '**/input-type.spec.ts' to ensure these tests are executed.

Copilot uses AI. Check for mistakes.
{
name: 'chromium',
use: { ...devices['Desktop Chrome'] },
Expand Down
91 changes: 70 additions & 21 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type CurrencyInputState = {
disableSelectionHandling: boolean,
maskedValue: string,
value: number | string, // TODO: should be string? should also have a separate float field for 'pennies'
previousProps?: Readonly<CurrencyInputProps>, // Track previous props to detect changes
};

type SelectionSnapshot = {
Expand Down Expand Up @@ -124,22 +125,23 @@ class CurrencyInput extends React.Component<CurrencyInputProps, CurrencyInputSta
* General function used to cleanup and define the final props used for rendering
* @returns CurrencyInputState
*/
static prepareProps({
onChangeEvent,
value: propValue,
decimalSeparator,
thousandSeparator,
precision,
inputType,
allowNegative,
allowEmpty,
prefix,
suffix,
selectAllOnFocus,
autoFocus,
disableSelectionHandling: propDisableSelectionHandling,
...customProps
}: Readonly<CurrencyInputProps>): CurrencyInputState {
static prepareProps(props: Readonly<CurrencyInputProps>): CurrencyInputState {
const {
onChangeEvent,
value: propValue,
decimalSeparator,
thousandSeparator,
precision,
inputType,
allowNegative,
allowEmpty,
prefix,
suffix,
selectAllOnFocus,
autoFocus,
disableSelectionHandling: propDisableSelectionHandling,
...customProps
} = props;
let initialValue = propValue;
if (initialValue === null) {
initialValue = allowEmpty ? null : '';
Expand All @@ -166,7 +168,7 @@ class CurrencyInput extends React.Component<CurrencyInputProps, CurrencyInputSta
);

const disableSelectionHandling = propDisableSelectionHandling || inputType === 'number';
return { maskedValue, value, customProps, disableSelectionHandling };
return { maskedValue, value, customProps, disableSelectionHandling, previousProps: props };
}

/**
Expand All @@ -182,11 +184,58 @@ class CurrencyInput extends React.Component<CurrencyInputProps, CurrencyInputSta
* @see https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops
*/
static getDerivedStateFromProps(nextProps: Readonly<CurrencyInputProps>, prevState: Readonly<CurrencyInputState>) {
const props = { ...nextProps };
if (nextProps.value !== prevState.value) {
props.value = prevState.value;
const previousProps = prevState.previousProps || nextProps; // First call uses the initial props snapshot

// Check if the VALUE prop itself changed (parent is controlling the input)
const valueChanged = nextProps.value !== previousProps.value;

// Check if separators or display formatting changed (these require reformatting the current value)
const formatChanged =
nextProps.decimalSeparator !== previousProps.decimalSeparator ||
nextProps.thousandSeparator !== previousProps.thousandSeparator ||
nextProps.precision !== previousProps.precision ||
nextProps.prefix !== previousProps.prefix ||
nextProps.suffix !== previousProps.suffix;

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatChanged check doesn't include inputType changes, but inputType can affect how the component behaves (as seen in line 169 where inputType === 'number' affects disableSelectionHandling). If inputType changes from 'text' to 'number' or vice versa, the component should update disableSelectionHandling in the state, but this isn't handled in the format change detection. Consider adding inputType to the formatChanged conditions or handling it separately.

Suggested change
nextProps.suffix !== previousProps.suffix;
nextProps.suffix !== previousProps.suffix ||
nextProps.inputType !== previousProps.inputType;

Copilot uses AI. Check for mistakes.

if (valueChanged) {
// Parent changed the value prop - use the new value
const newState = CurrencyInput.prepareProps(nextProps);
return { ...newState, previousProps: nextProps };
}
return CurrencyInput.prepareProps(props);

if (formatChanged) {
// Display formatting changed - reformat the current value with new formatting
const propsWithCurrentValue = { ...nextProps, value: prevState.value };
const newState = CurrencyInput.prepareProps(propsWithCurrentValue);
return { ...newState, previousProps: nextProps };
}

Copilot AI Dec 14, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sequential if-statements mean that if multiple prop changes occur simultaneously, only the first matching condition is handled. For example, if both format props (like prefix) and allowNegative change at the same time, the formatChanged block executes and returns early, bypassing the special allowNegativeChanged logic that converts negative values to positive. Consider checking allowNegativeChanged before formatChanged, or include allowNegative in the formatChanged condition so that the special negative-to-positive conversion logic is always applied when allowNegative becomes false.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


// Check if allowNegative changed
const allowNegativeChanged = nextProps.allowNegative !== previousProps.allowNegative;

if (allowNegativeChanged) {
if (!nextProps.allowNegative) {
// allowNegative was disabled
// If current value is negative, make it positive
const parsedValue = typeof prevState.value === 'number'
? prevState.value
: prevState.value === null
? 0
: CurrencyInput.stringValueToFloat(String(prevState.value), nextProps.thousandSeparator, nextProps.decimalSeparator);

if (parsedValue < 0) {
const propsWithPositiveValue = { ...nextProps, value: Math.abs(parsedValue) };
const newState = CurrencyInput.prepareProps(propsWithPositiveValue);
return { ...newState, previousProps: nextProps };
}
}

Copilot AI Dec 13, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The allowNegativeChanged check only handles the case when allowNegative is disabled. If allowNegative is enabled (changed from false to true), the function doesn't update previousProps and falls through to line 229 which preserves the state. However, when other props like inputType change together with allowNegative being enabled, this could lead to stale previousProps. Consider adding an else clause or ensuring previousProps is always updated when allowNegativeChanged is true.

Suggested change
}
}
// Even if value is not negative, still update previousProps to avoid staleness
return { ...prevState, previousProps: nextProps };
} else if (allowNegativeChanged) {
// allowNegative was enabled; update previousProps to avoid staleness
return { ...prevState, previousProps: nextProps };

Copilot uses AI. Check for mistakes.
// allowNegative toggled on or no sign change needed: still advance previousProps
return { ...prevState, previousProps: nextProps };
}

// Other props changed but value and display formatting didn't
// Just update the previousProps reference and preserve current state
return { ...prevState, previousProps: nextProps };
}

/**
Expand Down
Loading