Skip to content

[TextField] Add integer number type #9051

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 25, 2023
Merged

[TextField] Add integer number type #9051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/heavy-bats-jam.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Added `integer` option for the `type` prop of TextField
20 changes: 18 additions & 2 deletions polaris-react/src/components/TextField/TextField.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ export function Default() {
}

export function Number() {
const [value, setValue] = useState('1');
const [value1, setValue1] = useState('1');
const [value, setValue] = useState('1.0');
const [value1, setValue1] = useState('1.0');

const handleChange = useCallback((newValue) => setValue(newValue), []);
const handleChange1 = useCallback((newValue) => setValue1(newValue), []);
Expand All @@ -59,6 +59,22 @@ export function Number() {
);
}

export function Integer() {
const [value, setValue] = useState('1');

const handleChange = useCallback((newValue) => setValue(newValue), []);

return (
<TextField
label="Integer"
type="integer"
value={value}
onChange={handleChange}
autoComplete="off"
/>
);
}

export function Email() {
const [value, setValue] = useState('bernadette.lapresse@jadedpixel.com');

Expand Down
23 changes: 17 additions & 6 deletions polaris-react/src/components/TextField/TextField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Type =
| 'text'
| 'email'
| 'number'
| 'integer'
| 'password'
| 'search'
| 'tel'
Expand Down Expand Up @@ -293,6 +294,7 @@ export function TextField({
);

const inputType = type === 'currency' ? 'text' : type;
const isNumericType = type === 'number' || type === 'integer';

const prefixMarkup = prefix ? (
<div className={styles.Prefix} id={`${id}-Prefix`} ref={prefixRef}>
Expand Down Expand Up @@ -373,7 +375,8 @@ export function TextField({

// Making sure the new value has the same length of decimal places as the
// step / value has.
const decimalPlaces = Math.max(dpl(numericValue), dpl(stepAmount));
const decimalPlaces =
type === 'integer' ? 0 : Math.max(dpl(numericValue), dpl(stepAmount));

const newValue = Math.min(
Number(normalizedMax),
Expand All @@ -393,6 +396,7 @@ export function TextField({
onChange,
onSpinnerChange,
normalizedStep,
type,
value,
],
);
Expand Down Expand Up @@ -426,7 +430,7 @@ export function TextField({
);

const spinnerMarkup =
type === 'number' && step !== 0 && !disabled && !readOnly ? (
isNumericType && step !== 0 && !disabled && !readOnly ? (
<Spinner
onClick={handleClickChild}
onChange={handleNumberChange}
Expand Down Expand Up @@ -507,7 +511,7 @@ export function TextField({
useEventListener('wheel', handleOnWheel, inputRef);

function handleOnWheel(event: WheelEvent) {
if (document.activeElement === event.target && type === 'number') {
if (document.activeElement === event.target && isNumericType) {
event.stopPropagation();
}
}
Expand Down Expand Up @@ -656,16 +660,23 @@ export function TextField({

function handleKeyPress(event: React.KeyboardEvent) {
const {key, which} = event;
const numbersSpec = /[\d.eE+-]$/;
if (type !== 'number' || which === Key.Enter || numbersSpec.test(key)) {
const numbersSpec = /[\d.,eE+-]$/;
Copy link
Contributor Author

@camielvs camielvs Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggestion:
Add the comma , to the number spec so that users in regions where the comma is used as the decimal separator (Europe, Africa, South America) can use the correct key. Currently, all users, regardless of location, must use the period . to indicate a decimal place.
Handling and display of the decimal separator is done by the html input element based on the browser's language. The code will always receive the decimal point as a period . regardless of which separator the user entered, so we do not need to make any additional allowances to accommodate this.
The result is that a user in North America will be able to use the decimal separator . whilst a user in Europe can use either . or ,.

Additionally, there is one edge case in Safari to be aware of. Safari's number type input boxes do not appear to limit the input to one decimal character (i.e. 123.4.5.6.7.8 is a valid input). Currently, Shopify does not validate to prevent the user from typing in extra decimal points. Combined with this regex change, users in Europe who use Safari may try to enter thousand separators into their numbers, which are unhandled by Shopify.
My recommendation is to add a validation on the TextField value to prevent more than one . or , being entered into the input field, but I'm interested in ideas/thoughts on this.

const integerSpec = /[\deE+-]$/;
Copy link
Contributor Author

@camielvs camielvs Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this prevents a user from copy + pasting a decimal number into the integer field. We may need to come up with a way to prevent users from doing this, e.g. filter the value before displaying.

Copy link

@csaba-shopify csaba-shopify Apr 24, 2023

Choose a reason for hiding this comment

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

This would still allow non sensical input like 45-234234, correct? Is there some way we could validate against that? 🤔

Also, it looks like the keypress event is deprecated. Shouldn't we do the validation in keydown instead?

Copy link
Contributor Author

@camielvs camielvs Apr 24, 2023

Choose a reason for hiding this comment

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

Good spotting! Indeed we can do non-sensical input like you've described, although this isn't any different to the existing number type.
I agree another layer of input validations would be useful, and we should be using keydown instead of keypress, but I don't have much context on this component and its architecture, so it's probably something someone from @Shopify/polaris-team needs to advise on.


if (
!isNumericType ||
which === Key.Enter ||
(type === 'number' && numbersSpec.test(key)) ||
(type === 'integer' && integerSpec.test(key))
) {
return;
}

event.preventDefault();
}

function handleKeyDown(event: React.KeyboardEvent) {
if (type !== 'number') {
if (!isNumericType) {
return;
}

Expand Down
Loading