Skip to content

Commit

Permalink
refactor: address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bgutsol committed Aug 5, 2022
1 parent b63dc81 commit 51ad4af
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
23 changes: 14 additions & 9 deletions packages/number/src/NumberEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { styles } from './NumberEditor.styles';
import { isNumberInputValueValid, parseNumber } from './parseNumber';
import { getRangeFromField, valueToString, countDecimals } from './utils';


export interface NumberEditorProps {
/**
* is the field disabled initially
Expand All @@ -32,6 +31,11 @@ type InnerNumberEditorProps = Pick<
field: NumberEditorProps['field'];
};

enum StepChangeType {
Increment = 'increment',
Decrement = 'decrement',
}

const NUMBER_STEP = 1;

function InnerNumberEditor({
Expand All @@ -51,7 +55,7 @@ function InnerNumberEditor({
if (stringSdkValue !== inputValue) {
setInputValue(stringSdkValue);
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- we want to trigger it only when sdkValue had been changed
// eslint-disable-next-line react-hooks/exhaustive-deps -- we want to trigger it only when sdkValue has changed
}, [sdkValue]);

const updateExternalValue = (value: number | undefined) => {
Expand All @@ -60,9 +64,10 @@ function InnerNumberEditor({
}
};

const changeValueByStep = (type: 'increment' | 'decrement') => {
const currentValue = isNaN(+inputValue) ? 0 : +inputValue;
let nextValue = type === 'increment' ? currentValue + NUMBER_STEP : currentValue - NUMBER_STEP;
const changeValueByStep = (type: StepChangeType) => {
const currentValue = Number.isNaN(+inputValue) ? 0 : +inputValue;
let nextValue =
type === StepChangeType.Increment ? currentValue + NUMBER_STEP : currentValue - NUMBER_STEP;
// Floating point numbers cannot represent all decimals precisely in binary.
// This can lead to unexpected results, such as 0.1 + 0.2 = 0.30000000000000004.
// See more details: https://floating-point-gui.de/
Expand All @@ -82,8 +87,8 @@ function InnerNumberEditor({
const keyToFnMap: {
[key: string]: () => void;
} = {
ArrowUp: () => changeValueByStep('increment'),
ArrowDown: () => changeValueByStep('decrement'),
ArrowUp: () => changeValueByStep(StepChangeType.Increment),
ArrowDown: () => changeValueByStep(StepChangeType.Decrement),
};

const fn = keyToFnMap[event.key];
Expand Down Expand Up @@ -150,14 +155,14 @@ function InnerNumberEditor({
<button
tabIndex={-1}
className={styles.control}
onClick={() => changeValueByStep('increment')}
onClick={() => changeValueByStep(StepChangeType.Increment)}
onPointerDown={handleControlPointerDown}>
<ArrowUpTrimmedIcon size="medium" />
</button>
<button
tabIndex={-1}
className={styles.control}
onClick={() => changeValueByStep('decrement')}
onClick={() => changeValueByStep(StepChangeType.Decrement)}
onPointerDown={handleControlPointerDown}>
<ArrowDownTrimmedIcon size="medium" />
</button>
Expand Down
4 changes: 2 additions & 2 deletions packages/number/src/parseNumber.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('parseNumber', () => {
});

describe('isNumberInputValueValid', () => {
it('should correctrly validate input when type is Integer', () => {
it('should correctly validate input when type is Integer', () => {
expect(isNumberInputValueValid('3', 'Integer')).toBe(true);
expect(isNumberInputValueValid('33', 'Integer')).toBe(true);
expect(isNumberInputValueValid('+', 'Integer')).toBe(true);
Expand All @@ -33,7 +33,7 @@ describe('isNumberInputValueValid', () => {
expect(isNumberInputValueValid('foo', 'Integer')).toBe(false);
});

it('should correctrly validate input when type is Number', () => {
it('should correctly validate input when type is Number', () => {
expect(isNumberInputValueValid('3', 'Decimal')).toBe(true);
expect(isNumberInputValueValid('33', 'Decimal')).toBe(true);
expect(isNumberInputValueValid('+', 'Decimal')).toBe(true);
Expand Down
2 changes: 1 addition & 1 deletion packages/number/src/parseNumber.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export function parseNumber(value: string, type: string) {
if (!value || isNaN(+value)) {
if (!value || Number.isNaN(+value)) {
return;
}

Expand Down

0 comments on commit 51ad4af

Please sign in to comment.