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

Switch to input type="text" and @wojtekmaj/predict-input-value package to prevent invalid inputs #272

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"dependencies": {
"@types/react-calendar": "^3.0.0",
"@wojtekmaj/date-utils": "^1.0.3",
"@wojtekmaj/predict-input-value": "^1.0.0",
"get-user-locale": "^1.2.0",
"make-event-props": "^1.1.0",
"merge-class-names": "^1.1.1",
Expand Down
50 changes: 35 additions & 15 deletions src/DateInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,28 @@ function findInput(element, property) {
return nextElement;
}

function isInputValid(input) {
if (!input.validity.valid) {
return false;
}

const { value } = input;

if (!value) {
return false;
}

const rawValue = Number(value);
const min = Number(input.getAttribute('min'));
const max = Number(input.getAttribute('max'));

if (rawValue < min || rawValue > max) {
return false;
}

return true;
}

function focus(element) {
if (element) {
element.focus();
Expand Down Expand Up @@ -192,9 +214,9 @@ export default class DateInput extends PureComponent {
)
) {
if (nextValue) {
nextState.year = getYear(nextValue);
nextState.month = getMonthHuman(nextValue);
nextState.day = getDate(nextValue);
nextState.year = getYear(nextValue).toString();
nextState.month = getMonthHuman(nextValue).toString();
nextState.day = getDate(nextValue).toString();
} else {
nextState.year = null;
nextState.month = null;
Expand Down Expand Up @@ -352,7 +374,7 @@ export default class DateInput extends PureComponent {
return;
}

const { value } = input;
const { maxLength, value } = input;
const max = input.getAttribute('max');

/**
Expand All @@ -361,7 +383,7 @@ export default class DateInput extends PureComponent {
* However, given 2, smallers possible number would be 20, and thus keeping the focus in
* this field doesn't make sense.
*/
if ((value * 10 > max) || (value.length >= max.length)) {
if ((value * 10 > max) || (value.length >= maxLength)) {
const property = 'nextElementSibling';
const nextInput = findInput(input, property);
focus(nextInput);
Expand All @@ -375,7 +397,7 @@ export default class DateInput extends PureComponent {
const { name, value } = event.target;

this.setState(
{ [name]: value ? parseInt(value, 10) : null },
{ [name]: value },
this.onChangeExternal,
);
}
Expand All @@ -397,9 +419,9 @@ export default class DateInput extends PureComponent {
}

const [yearString, monthString, dayString] = value.split('-');
const year = parseInt(yearString, 10);
const monthIndex = parseInt(monthString, 10) - 1 || 0;
const day = parseInt(dayString, 10) || 1;
const year = Number(yearString);
const monthIndex = Number(monthString) - 1 || 0;
const day = Number(dayString) || 1;

const proposedValue = new Date();
proposedValue.setFullYear(year, monthIndex, day);
Expand Down Expand Up @@ -431,12 +453,10 @@ export default class DateInput extends PureComponent {

if (formElements.every((formElement) => !formElement.value)) {
onChange(null, false);
} else if (
formElements.every((formElement) => formElement.value && formElement.validity.valid)
) {
const year = parseInt(values.year, 10);
const monthIndex = parseInt(values.month, 10) - 1 || 0;
const day = parseInt(values.day || 1, 10);
} else if (formElements.every(isInputValid)) {
const year = Number(values.year);
const monthIndex = Number(values.month) - 1 || 0;
const day = Number(values.day || 1);

const proposedValue = new Date();
proposedValue.setFullYear(year, monthIndex, day);
Expand Down
36 changes: 18 additions & 18 deletions src/DateInput.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(9);
expect(customInputs.at(1).prop('value')).toBe(30);
expect(customInputs.at(2).prop('value')).toBe(2017);
expect(customInputs.at(0).prop('value')).toBe('9');
expect(customInputs.at(1).prop('value')).toBe('30');
expect(customInputs.at(2).prop('value')).toBe('2017');
});

it('shows a given date in all inputs correctly given array of Date objects (12-hour format)', () => {
Expand All @@ -134,9 +134,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(9);
expect(customInputs.at(1).prop('value')).toBe(30);
expect(customInputs.at(2).prop('value')).toBe(2017);
expect(customInputs.at(0).prop('value')).toBe('9');
expect(customInputs.at(1).prop('value')).toBe('30');
expect(customInputs.at(2).prop('value')).toBe('2017');
});

it('shows a given date in all inputs correctly given ISO string (12-hour format)', () => {
Expand All @@ -153,9 +153,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(9);
expect(customInputs.at(1).prop('value')).toBe(30);
expect(customInputs.at(2).prop('value')).toBe(2017);
expect(customInputs.at(0).prop('value')).toBe('9');
expect(customInputs.at(1).prop('value')).toBe('30');
expect(customInputs.at(2).prop('value')).toBe('2017');
});

itIfFullICU('shows a given date in all inputs correctly given Date (24-hour format)', () => {
Expand All @@ -173,9 +173,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(2017);
expect(customInputs.at(1).prop('value')).toBe(9);
expect(customInputs.at(2).prop('value')).toBe(30);
expect(customInputs.at(0).prop('value')).toBe('2017');
expect(customInputs.at(1).prop('value')).toBe('9');
expect(customInputs.at(2).prop('value')).toBe('30');
});

itIfFullICU('shows a given date in all inputs correctly given array of Date objects (24-hour format)', () => {
Expand All @@ -193,9 +193,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(2017);
expect(customInputs.at(1).prop('value')).toBe(9);
expect(customInputs.at(2).prop('value')).toBe(30);
expect(customInputs.at(0).prop('value')).toBe('2017');
expect(customInputs.at(1).prop('value')).toBe('9');
expect(customInputs.at(2).prop('value')).toBe('30');
});

itIfFullICU('shows a given date in all inputs correctly given ISO string (24-hour format)', () => {
Expand All @@ -213,9 +213,9 @@ describe('DateInput', () => {
const customInputs = component.find('input[data-input]');

expect(nativeInput.prop('value')).toBe('2017-09-30');
expect(customInputs.at(0).prop('value')).toBe(2017);
expect(customInputs.at(1).prop('value')).toBe(9);
expect(customInputs.at(2).prop('value')).toBe(30);
expect(customInputs.at(0).prop('value')).toBe('2017');
expect(customInputs.at(1).prop('value')).toBe('9');
expect(customInputs.at(2).prop('value')).toBe('30');
});

it('shows empty value in all inputs correctly given null', () => {
Expand Down
10 changes: 6 additions & 4 deletions src/DateInput/DayInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function DayInput({
})();

function isSameMonth(date) {
return date && year === getYear(date) && month === getMonthHuman(date);
return date && Number(year) === getYear(date) && Number(month) === getMonthHuman(date);
}

const maxDay = safeMin(currentMonthMaxDays, isSameMonth(maxDate) && getDate(maxDate));
Expand All @@ -44,20 +44,22 @@ export default function DayInput({
);
}

const isValue = PropTypes.oneOfType([PropTypes.string, PropTypes.number]);

DayInput.propTypes = {
ariaLabel: PropTypes.string,
className: PropTypes.string.isRequired,
disabled: PropTypes.bool,
itemRef: PropTypes.func,
maxDate: isMaxDate,
minDate: isMinDate,
month: PropTypes.number,
month: isValue,
onChange: PropTypes.func,
onKeyDown: PropTypes.func,
onKeyUp: PropTypes.func,
placeholder: PropTypes.string,
required: PropTypes.bool,
showLeadingZeros: PropTypes.bool,
value: PropTypes.number,
year: PropTypes.number,
value: isValue,
year: isValue,
};
2 changes: 1 addition & 1 deletion src/DateInput/DayInput.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('DayInput', () => {
});

it('displays given value properly', () => {
const value = 11;
const value = '11';

const component = mount(
<DayInput
Expand Down
82 changes: 62 additions & 20 deletions src/DateInput/Input.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import mergeClassNames from 'merge-class-names';
import updateInputWidth, { getFontShorthand } from 'update-input-width';
import predictInputValue from '@wojtekmaj/predict-input-value';

/* eslint-disable jsx-a11y/no-autofocus */

Expand Down Expand Up @@ -45,27 +46,51 @@ function updateInputWidthOnFontLoad(element) {
document.fonts.addEventListener('loadingdone', onLoadingDone);
}

function getSelectionString() {
if (typeof window === 'undefined') {
return null;
}
function addLeadingZero(value, max) {
return `0${value}`.slice(-(`${max}`.length));
}

function makeOnKeyDown({
max, min, onChange, showLeadingZeros,
}) {
return function onKeyDown(event) {
switch (event.key) {
case 'ArrowUp':
case 'ArrowDown': {
event.preventDefault();

const { target: input } = event;
const { value } = input;

const numericValue = Number(value);
const rawNextValue = numericValue + (event.key === 'ArrowUp' ? 1 : -1);

const limitedRawNextValue = Math.min(max, Math.max(min, rawNextValue));

const hasLeadingZero = showLeadingZeros && limitedRawNextValue < 10;
const nextValue = (hasLeadingZero
? addLeadingZero(limitedRawNextValue, max)
: limitedRawNextValue
);

return window.getSelection().toString();
input.value = nextValue;
onChange(event);

break;
}
default:
}
};
}

function makeOnKeyPress(maxLength) {
/**
* Prevents keystrokes that would not produce a number or when value after keystroke would
* exceed maxLength.
*/
function makeOnKeyPress(max) {
return function onKeyPress(event) {
const { key, target: input } = event;
const { value } = input;
const { key } = event;

const isNumberKey = !isNaN(parseInt(key, 10));
const selection = getSelectionString();
const nextValue = predictInputValue(event);

if (isNumberKey && (selection || value.length < maxLength)) {
if (isNumberKey && (nextValue <= max)) {
return;
}

Expand All @@ -92,8 +117,16 @@ export default function Input({
step,
value,
}) {
const hasLeadingZero = showLeadingZeros && value !== null && value < 10;
const maxLength = max.toString().length;
const hasLeadingZero = (
showLeadingZeros
&& (value !== null && value !== undefined)
&& value.toString().length < 2
);

const onKeyDownInternal = makeOnKeyDown({
max, min, onChange, showLeadingZeros,
});
const onKeyPressInternal = makeOnKeyPress(max);

return [
(hasLeadingZero && <span key="leadingZero" className={`${className}__leadingZero`}>0</span>),
Expand All @@ -111,12 +144,19 @@ export default function Input({
disabled={disabled}
inputMode="numeric"
max={max}
maxLength={`${max}`.length}
min={min}
name={name}
onChange={onChange}
onFocus={onFocus}
onKeyDown={onKeyDown}
onKeyPress={makeOnKeyPress(maxLength)}
onKeyDown={(event) => {
onKeyDownInternal(event);

if (onKeyDown) {
onKeyDown(event);
}
}}
onKeyPress={onKeyPressInternal}
onKeyUp={(event) => {
updateInputWidth(event.target);

Expand All @@ -137,12 +177,14 @@ export default function Input({
}}
required={required}
step={step}
type="number"
type="text"
value={value !== null ? value : ''}
/>,
];
}

const isValue = PropTypes.oneOfType([PropTypes.string, PropTypes.number]);

Input.propTypes = {
ariaLabel: PropTypes.string,
autoFocus: PropTypes.bool,
Expand All @@ -160,5 +202,5 @@ Input.propTypes = {
required: PropTypes.bool,
showLeadingZeros: PropTypes.bool,
step: PropTypes.number,
value: PropTypes.number,
value: isValue,
};
8 changes: 5 additions & 3 deletions src/DateInput/MonthInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function MonthInput({
...otherProps
}) {
function isSameYear(date) {
return date && year === getYear(date);
return date && Number(year) === getYear(date);
}

const maxMonth = safeMin(12, isSameYear(maxDate) && getMonthHuman(maxDate));
Expand All @@ -30,6 +30,8 @@ export default function MonthInput({
);
}

const isValue = PropTypes.oneOfType([PropTypes.string, PropTypes.number]);

MonthInput.propTypes = {
ariaLabel: PropTypes.string,
className: PropTypes.string.isRequired,
Expand All @@ -43,6 +45,6 @@ MonthInput.propTypes = {
placeholder: PropTypes.string,
required: PropTypes.bool,
showLeadingZeros: PropTypes.bool,
value: PropTypes.number,
year: PropTypes.number,
value: isValue,
year: isValue,
};
Loading