Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(NumberInput): add new component #1826

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
34feb62
feat(NumberInput): added new component
DaryaLari Aug 28, 2024
6fef3f7
chore(NumberInput): added stories
DaryaLari Aug 28, 2024
f624e7b
test(NumberInput): added tests
DaryaLari Aug 28, 2024
7bbff75
fix(NumberInput): endContent & controls
DaryaLari Sep 13, 2024
d71416d
chore(NumberInput): refactor stories
DaryaLari Sep 13, 2024
526ae25
fix(NumberInput): typo
DaryaLari Sep 13, 2024
d964eff
chore(NumberInput): functional fixes
DaryaLari Sep 14, 2024
284955e
docs(NumberInput): fix readme
DaryaLari Sep 14, 2024
c980c14
test(NumberInput): visual tests added
DaryaLari Sep 16, 2024
cd266a8
fix(NumberInput): fix displayed zeros with empty value
DaryaLari Sep 16, 2024
220f91e
fixup! test(NumberInput): visual tests added
DaryaLari Sep 16, 2024
94fd8f9
fix(NumberInput): story item value update
DaryaLari Sep 16, 2024
4d34067
fix(NumberInput): fix after rebase
DaryaLari Sep 18, 2024
f368d8b
fix(NumberInput): arrows width
DaryaLari Sep 19, 2024
f9d33ef
fix(NumberInput): inserting invalid string to the middle of number
DaryaLari Sep 19, 2024
3502291
chore(NumberInput): change value type to number and add clamping valu…
DaryaLari Sep 25, 2024
07db0ef
test: update snapshots
DaryaLari Sep 25, 2024
bb4d866
chore(NumberInput): move component to lab
DaryaLari Sep 26, 2024
96c899c
fix(NumberInput): review fixes
DaryaLari Sep 27, 2024
cc4d7fb
docs(NumberInput): fix types in readme
DaryaLari Sep 30, 2024
63f2e8e
docs(NumberInput): add description of behaviour props to readme
DaryaLari Sep 30, 2024
68a529b
fix(NumberInput): more review fixes
DaryaLari Sep 30, 2024
c030749
fix(NumberInput): duplicated arrows border style
DaryaLari Oct 1, 2024
69c0a1d
fix(NumberInput): add e.preventDefault, rename showControls, remove f…
DaryaLari Oct 2, 2024
152bdfe
fix(NumberInput): fix clapting to step divisibles
DaryaLari Oct 2, 2024
1ff5952
fix(NumberInput): remove local step state
DaryaLari Oct 2, 2024
165316a
chore(NumberInput): handle home/end btns press
DaryaLari Oct 2, 2024
f4f5297
test(NumberInput): add tests on HOME/END press
DaryaLari Oct 3, 2024
bda3863
fix(NumberInput): remove exporting default props from TextInput
DaryaLari Oct 3, 2024
c4daf69
fix(NumberInput): hide description&default columns from storybook con…
DaryaLari Oct 3, 2024
ddfd727
feat(NumberInput): support uncontrolled usage type
DaryaLari Oct 4, 2024
b53f6bd
chore(NumberInput): change input type in default story to uncontrolled
DaryaLari Oct 4, 2024
7a98458
chore(NumberInput): change all other stiries' inputs to uncontrolled
DaryaLari Oct 4, 2024
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
44 changes: 44 additions & 0 deletions src/components/lab/NumberInput/NumberInput.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
@use '../../variables';

$block: '.#{variables.$ns}number-input';

#{$block} {
&_size {
&_s {
--_--textinput-end-padding: 1px;
}

&_m {
--_--textinput-end-padding: 1px;
}

&_l {
--_--textinput-end-padding: 3px;
}

&_xl {
--_--textinput-end-padding: 3px;
}
}

&_view_normal {
--_-g-number-input-numeric-arrows-border-color: var(--g-color-line-generic);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--_-g-number-input-numeric-arrows-border-color: var(--g-color-line-generic);
--_--arrows-border-color: var(--g-color-line-generic);

It's a private var so no need to specify full name

--_--border-color-hover: var(--g-color-line-generic-hover);
--_--border-color-active: var(--g-color-line-generic-active);
Comment on lines +26 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

What these variables for? They duplicate TextInput styles


&#{$block}_state_error {
--_-g-number-input-numeric-arrows-border-color: var(--g-color-line-danger);
}
}

&_view_clear {
--_-g-number-input-numeric-arrows-border-color: transparent;
}

&__numeric-arrows {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&__numeric-arrows {
&__arrows {

Let's be compact here

border-style: none;
border-inline-start-style: solid;

margin-inline: var(--_--textinput-end-padding) calc(0px - var(--_--textinput-end-padding));
}
}
326 changes: 326 additions & 0 deletions src/components/lab/NumberInput/NumberInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,326 @@
'use client';

import React from 'react';

import {KeyCode} from '../../../constants';
import {useControlledState, useForkRef} from '../../../hooks';
import {useFormResetHandler} from '../../../hooks/private';
import {TextInput} from '../../controls/TextInput';
import type {BaseInputControlProps} from '../../controls/types';
import {getInputControlState} from '../../controls/utils';
import {block} from '../../utils/cn';

import {NumericArrows} from './NumericArrows/NumericArrows';
import {
areStringRepresentationOfNumbersEqual,
clampToNearestStepValue,
getInputPattern,
getInternalState,
getParsedValue,
getPossibleNumberSubstring,
updateCursorPosition,
} from './utils';

import './NumberInput.scss';

const b = block('number-input');

export interface NumberInputProps
extends Omit<
BaseInputControlProps<HTMLInputElement>,
'error' | 'value' | 'defaultValue' | 'onUpdate'
> {
/** The control's html attributes */
controlProps?: Omit<React.InputHTMLAttributes<HTMLInputElement>, 'min' | 'max' | 'onChange'>;
/** Help text rendered to the left of the input node */
label?: string;
/** Indicates that the user cannot change control's value */
readOnly?: boolean;
/** User`s node rendered before label and input node */
startContent?: React.ReactNode;
/** User`s node rendered after input node and clear button */
endContent?: React.ReactNode;
/** An optional element displayed under the lower right corner of the control and sharing the place with the error container */
note?: React.ReactNode;
/**Describes the validation state */
validationState?: 'invalid' | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

This prop already in the BaseInputControlProps, no need to specify it again


/** Hides increment/decrement buttons at the end of control
*/
hiddenControls?: boolean;
/** min allowed value. It is used for clamping entered value to allowed range
* @default Number.MAX_SAFE_INTEGER
*/
min?: number;
/** max allowed value. It is used for clamping entered value to allowed range
* @default Number.MIN_SAFE_INTEGER
*/
max?: number;
/** Delta for incrementing/decrementing entered value with arrow keyboard buttons or component controls
* @default 1
*/
step?: number;
/** Step multiplier when shift button is pressed
* @default 10
*/
shiftMultiplier?: number;
Comment on lines +63 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use Shift and not Page Up / Page Down ?

Copy link
Contributor

@korvin89 korvin89 Oct 1, 2024

Choose a reason for hiding this comment

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

We've discussed it with @ValeraS @amje and decided to keep current behaviour for now

/** Enables changing value by scrolling mousewheel on with cursor on the input
* @default false
*/
allowMouseWheel?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

We've decided to remove the wheel behaviour completely. It turned out that currently it brings a lot of bugs and inconsistency between mouse and touchpad devices. Maybe we reimplement it in the future.

/** Enables ability to enter decimal numbers
* @default false
*/
allowDecimal?: boolean;
/** The control's value */
value?: number | null;
/** The control's default value. Use when the component is not controlled */
defaultValue?: number | null;
/** Fires when the input’s value is changed by the user. Provides new value as an callback's argument */
onUpdate?: (value: number | null) => void;
}

function getStringValue(value: number | null) {
return value === null ? '' : String(value);
}

export const NumberInput = React.forwardRef<HTMLSpanElement, NumberInputProps>(function NumberInput(
{endContent, defaultValue: externalDefaultValue, ...props},
ref,
) {
const {
value: externalValue,
onChange: handleChange,
onUpdate: externalOnUpdate,
min: externalMin,
max: externalMax,
shiftMultiplier: externalShiftMultiplier = 10,
step: externalStep = 1,
size = 'm',
view = 'normal',
disabled,
hiddenControls,
validationState,
onFocus,
onBlur,
onKeyDown,
allowMouseWheel = false,
allowDecimal = false,
className,
} = props;

const {
min,
max,
step: baseStep,
value: internalValue,
defaultValue,
shiftMultiplier,
} = getInternalState({
min: externalMin,
max: externalMax,
step: externalStep,
shiftMultiplier: externalShiftMultiplier,
allowDecimal,
value: externalValue,
defaultValue: externalDefaultValue,
});

const [value, setValue] = useControlledState(
internalValue,
defaultValue ?? null,
externalOnUpdate,
);

const [inputValue, setInputValue] = React.useState(getStringValue(value));

React.useEffect(() => {
const stringPropsValue = getStringValue(value);
setInputValue((currentInputValue) => {
if (!areStringRepresentationOfNumbersEqual(currentInputValue, stringPropsValue)) {
return stringPropsValue;
}
return currentInputValue;
});
}, [value]);

const clamp = true;

const [active, setActive] = React.useState(false);
const safeValue = value ?? 0;

const state = getInputControlState(validationState);

const canIncrementNumber = safeValue < (max ?? Number.MAX_SAFE_INTEGER);

const canDecrementNumber = safeValue > (min ?? Number.MIN_SAFE_INTEGER);

const innerControlRef = React.useRef<HTMLInputElement>(null);
const fieldRef = useFormResetHandler({
initialValue: value,
onReset: setValue,
});
const handleRef = useForkRef(props.controlRef, innerControlRef, fieldRef);

const handleIncrement = (
e:
| React.MouseEvent<HTMLButtonElement>
| React.WheelEvent<HTMLInputElement>
| React.KeyboardEvent<HTMLInputElement>,
) => {
const step = e.shiftKey ? shiftMultiplier * baseStep : baseStep;
if (canIncrementNumber) {
const newValue = clampToNearestStepValue({
value: safeValue + step,
step: baseStep,
min,
max,
direction: 'up',
});
setValue?.(newValue);
setInputValue(newValue.toString());
}
};

const handleDecrement = (
Copy link
Contributor

Choose a reason for hiding this comment

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

handleIncrement and handleDecrement look very alike. Let's combine them into one function with the signature handleValueDelta(delta: number), and use like handleValueDelta(1), handleValueDelta(-10) (if shift is pressed, and derection is negative).

e:
| React.MouseEvent<HTMLButtonElement>
| React.WheelEvent<HTMLInputElement>
| React.KeyboardEvent<HTMLInputElement>,
) => {
const step = e.shiftKey ? shiftMultiplier * baseStep : baseStep;
if (canDecrementNumber) {
const newValue = clampToNearestStepValue({
value: safeValue - step,
step: baseStep,
min,
max,
direction: 'down',
});
setValue?.(newValue);
setInputValue(newValue.toString());
}
};

const handleWheel: React.WheelEventHandler<HTMLInputElement> = (e) => {
const delta = e.shiftKey ? e.deltaX : e.deltaY;
Copy link
Contributor

Choose a reason for hiding this comment

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

When shift is pressed, scroll to the right decreases the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it depends on system scroll settings?
On my computer scroll to the right increases value as expected

e.preventDefault();
if (delta > 0) {
handleIncrement(e);
} else if (delta < 0) {
handleDecrement(e);
}
};

const handleKeyDown: React.KeyboardEventHandler<HTMLInputElement> = (e) => {
Copy link
Contributor

@ValeraS ValeraS Oct 1, 2024

Choose a reason for hiding this comment

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

If min/max are set, we can set the value to min/max on End/Home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in native input type=number on End/Home just the caret moves to the end/beginning of the content...
Wouldn't this difference in behaviour with native control be confusing?

if (e.key === KeyCode.ARROW_DOWN) {
e.preventDefault();
handleDecrement(e);
} else if (e.key === KeyCode.ARROW_UP) {
e.preventDefault();
handleIncrement(e);
} else if (e.key === KeyCode.HOME) {
e.preventDefault();
if (min !== undefined) {
setValue?.(min);
setInputValue(min.toString());
}
} else if (e.key === KeyCode.END) {
e.preventDefault();
if (max !== undefined) {
const newValue = clampToNearestStepValue({
value: max,
step: baseStep,
min,
max,
});
setValue?.(newValue);
setInputValue(newValue.toString());
}
}
onKeyDown?.(e);
};

const handleFocus: React.FocusEventHandler<HTMLInputElement> = (e) => {
setActive(true);
onFocus?.(e);
};

const handleBlur: React.FocusEventHandler<HTMLInputElement> = (e) => {
setActive(false);
if (clamp && value) {
const clampedValue = clampToNearestStepValue({
value,
step: baseStep,
min,
max,
});
if (value !== clampedValue) {
setValue?.(clampedValue);
setInputValue(clampedValue.toString());
}
}
onBlur?.(e);
};

const handleUpdate = (v: string) => {
setInputValue(v);
const preparedStringValue = getPossibleNumberSubstring(v, allowDecimal);
updateCursorPosition(innerControlRef, v, preparedStringValue);
const {valid, value: parsedNumberValue} = getParsedValue(preparedStringValue);
if (valid && parsedNumberValue !== value) {
setValue?.(parsedNumberValue);
}
};

const handleInput: React.FormEventHandler<HTMLInputElement> = (e) => {
const preparedStringValue = getPossibleNumberSubstring(e.currentTarget.value, allowDecimal);
updateCursorPosition(innerControlRef, e.currentTarget.value, preparedStringValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

When you increase the value (press the up arrow), the cursor moves to the beginning and then to the end.

};

return (
<TextInput
{...props}
className={b({size, view, state}, className)}
controlProps={{
onInput: handleInput,
...props.controlProps,
onWheel: allowMouseWheel && active ? handleWheel : undefined,
role: 'spinbutton',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it's correct. It's an input for numeric text, not spinbutton.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Native input type number has spinbutton role

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's keep it.

inputMode: 'numeric',
Copy link
Contributor

Choose a reason for hiding this comment

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

when allowDecimal = true, we should pass decimal here to have correct keyborad on mobile devices

pattern: props.controlProps?.pattern ?? getInputPattern(allowDecimal, false),
'aria-valuemin': props.min,
'aria-valuemax': props.max,
'aria-valuenow': value === null ? undefined : value,
}}
controlRef={handleRef}
value={inputValue}
onChange={handleChange}
onUpdate={handleUpdate}
onKeyDown={handleKeyDown}
onFocus={handleFocus}
onBlur={handleBlur}
ref={ref}
unstable_endContent={
<React.Fragment>
{endContent}
{hiddenControls ? null : (
<NumericArrows
className={b('numeric-arrows')}
size={size}
disabled={disabled}
onUpClick={(e) => {
innerControlRef.current?.focus();
handleIncrement(e);
}}
onDownClick={(e) => {
innerControlRef.current?.focus();
handleDecrement(e);
}}
/>
)}
</React.Fragment>
}
/>
);
});
Loading
Loading