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

Improve handling of decimal points and trailing zeroes in numbers #1183

Merged
merged 1 commit into from Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 83 additions & 8 deletions src/components/fields/NumberField.js
Expand Up @@ -3,14 +3,89 @@ import React from "react";
import * as types from "../../types";
import { asNumber } from "../../utils";

function NumberField(props) {
const { StringField } = props.registry.fields;
return (
<StringField
{...props}
onChange={value => props.onChange(asNumber(value))}
/>
);
// Matches a string that ends in a . character, optionally followed by a sequence of
// digits followed by any number of 0 characters up until the end of the line.
// Ensuring that there is at least one prefixed character is important so that
// you don't incorrectly match against "0".
const trailingCharMatcherWithPrefix = /\.([0-9]*0)*$/;

// This is used for trimming the trailing 0 and . characters without affecting
// the rest of the string. Its possible to use one RegEx with groups for this
// functionality, but it is fairly complex compared to simply defining two
// different matchers.
const trailingCharMatcher = /[0.]0*$/;

/**
* The NumberField class has some special handling for dealing with trailing
* decimal points and/or zeroes. This logic is designed to allow trailing values
* to be visible in the input element, but not be represented in the
* corresponding form data.
*
* The algorithm is as follows:
*
* 1. When the input value changes the value is cached in the component state
*
* 2. The value is then normalized, removing trailing decimal points and zeros,
* then passed to the "onChange" callback
*
* 3. When the component is rendered, the formData value is checked against the
* value cached in the state. If it matches the cached value, the cached
* value is passed to the input instead of the formData value
*/
class NumberField extends React.Component {
constructor(props) {
super(props);

this.state = {
lastValue: props.value,
};
}

handleChange = value => {
// Cache the original value in component state
this.setState({ lastValue: value });

// Normalize decimals that don't start with a zero character in advance so
// that the rest of the normalization logic is simpler
if (`${value}`.charAt(0) === ".") {
value = `0${value}`;
}

// Check that the value is a string (this can happen if the widget used is a
// <select>, due to an enum declaration etc) then, if the value ends in a
// trailing decimal point or multiple zeroes, strip the trailing values
let processed =
typeof value === "string" && value.match(trailingCharMatcherWithPrefix)
? asNumber(value.replace(trailingCharMatcher, ""))
: asNumber(value);

this.props.onChange(processed);
};

render() {
const { StringField } = this.props.registry.fields;
const { formData, ...props } = this.props;
const { lastValue } = this.state;

let value = formData;

if (typeof lastValue === "string" && value) {
// Construct a regular expression that checks for a string that consists
// of the formData value suffixed with zero or one '.' characters and zero
// or more '0' characters
const re = new RegExp(`${value}`.replace(".", "\\.") + "\\.?0*$");

// If the cached "lastValue" is a match, use that instead of the formData
// value to prevent the input value from changing in the UI
if (lastValue.match(re)) {
value = lastValue;
}
}

return (
<StringField {...props} formData={value} onChange={this.handleChange} />
);
}
}

if (process.env.NODE_ENV !== "production") {
Expand Down
28 changes: 26 additions & 2 deletions src/components/widgets/BaseInput.js
Expand Up @@ -23,7 +23,32 @@ function BaseInput(props) {
...inputProps
} = props;

inputProps.type = options.inputType || inputProps.type || "text";
// If options.inputType is set use that as the input type
if (options.inputType) {
inputProps.type = options.inputType;
} else if (!inputProps.type) {
// If the schema is of type number or integer, set the input type to number
if (schema.type === "number") {
inputProps.type = "number";
// Setting step to 'any' fixes a bug in Safari where decimals are not
// allowed in number inputs
inputProps.step = "any";
} else if (schema.type === "integer") {
inputProps.type = "number";
// Since this is integer, you always want to step up or down in multiples
// of 1
inputProps.step = "1";
} else {
inputProps.type = "text";
}
}

// If multipleOf is defined, use this as the step value. This mainly improves
// the experience for keyboard users (who can use the up/down KB arrows).
if (schema.multipleOf) {
inputProps.step = schema.multipleOf;
LucianBuzzo marked this conversation as resolved.
Show resolved Hide resolved
}

const _onChange = ({ target: { value } }) => {
return props.onChange(value === "" ? options.emptyValue : value);
};
Expand All @@ -44,7 +69,6 @@ function BaseInput(props) {
}

BaseInput.defaultProps = {
type: "text",
epicfaace marked this conversation as resolved.
Show resolved Hide resolved
required: false,
disabled: false,
readonly: false,
Expand Down
8 changes: 4 additions & 4 deletions test/ArrayField_test.js
Expand Up @@ -1102,7 +1102,7 @@ describe("ArrayField", () => {
"fieldset .field-string input[type=text]"
);
const numInput = node.querySelector(
"fieldset .field-number input[type=text]"
"fieldset .field-number input[type=number]"
);
expect(strInput.id).eql("root_0");
expect(numInput.id).eql("root_1");
Expand All @@ -1114,7 +1114,7 @@ describe("ArrayField", () => {
"fieldset .field-string input[type=text]"
);
const numInput = node.querySelector(
"fieldset .field-number input[type=text]"
"fieldset .field-number input[type=number]"
);
expect(strInput.required).eql(true);
expect(numInput.required).eql(true);
Expand All @@ -1129,7 +1129,7 @@ describe("ArrayField", () => {
"fieldset .field-string input[type=text]"
);
const numInput = node.querySelector(
"fieldset .field-number input[type=text]"
"fieldset .field-number input[type=number]"
);
expect(strInput.value).eql("foo");
expect(numInput.value).eql("42");
Expand All @@ -1141,7 +1141,7 @@ describe("ArrayField", () => {
"fieldset .field-string input[type=text]"
);
const numInput = node.querySelector(
"fieldset .field-number input[type=text]"
"fieldset .field-number input[type=number]"
);

Simulate.change(strInput, { target: { value: "bar" } });
Expand Down
4 changes: 2 additions & 2 deletions test/Form_test.js
Expand Up @@ -1693,7 +1693,7 @@ describe("Form", () => {
liveValidate: true,
});

Simulate.change(node.querySelector("input[type=text]"), {
Simulate.change(node.querySelector("input[type=number]"), {
target: { value: "not a number" },
});

Expand All @@ -1711,7 +1711,7 @@ describe("Form", () => {
formData: { branch: 2 },
});

Simulate.change(node.querySelector("input[type=text]"), {
Simulate.change(node.querySelector("input[type=number]"), {
target: { value: "not a number" },
});

Expand Down
129 changes: 121 additions & 8 deletions test/NumberField_test.js
Expand Up @@ -3,7 +3,7 @@ import { expect } from "chai";
import { Simulate } from "react-addons-test-utils";
import sinon from "sinon";

import { createFormComponent, createSandbox } from "./test_utils";
import { createFormComponent, createSandbox, setProps } from "./test_utils";

describe("NumberField", () => {
let sandbox;
Expand All @@ -17,15 +17,15 @@ describe("NumberField", () => {
});

describe("TextWidget", () => {
it("should render a string field", () => {
it("should render a number input", () => {
const { node } = createFormComponent({
schema: {
type: "number",
},
});

expect(
node.querySelectorAll(".field input[type=text]")
node.querySelectorAll(".field input[type=number]")
).to.have.length.of(1);
});

Expand Down Expand Up @@ -129,18 +129,107 @@ describe("NumberField", () => {
expect(node.querySelector(".field input").value).eql("2");
});

it("should not cast the input as a number if it ends with a dot", () => {
describe("when inputting a number that ends with a dot and/or zero it should normalize it, without changing the input value", () => {
const { comp, node } = createFormComponent({
schema: {
type: "number",
},
});

Simulate.change(node.querySelector("input"), {
target: { value: "2." },
const $input = node.querySelector("input");

const tests = [
{
input: "2.",
output: 2,
},
{
input: "2.0",
output: 2,
},
{
input: "2.3",
output: 2.3,
},
{
input: "2.30",
output: 2.3,
},
{
input: "2.300",
output: 2.3,
},
{
input: "2.3001",
output: 2.3001,
},
{
input: "2.03",
output: 2.03,
},
{
input: "2.003",
output: 2.003,
},
{
input: "2.00300",
output: 2.003,
},
{
input: "200300",
output: 200300,
},
];

tests.forEach(test => {
it(`should work with an input value of ${test.input}`, () => {
Simulate.change($input, {
target: { value: test.input },
});

expect(comp.state.formData).eql(test.output);
expect($input.value).eql(test.input);
});
});
});

it("should normalize values beginning with a decimal point", () => {
const { comp, node } = createFormComponent({
schema: {
type: "number",
},
});

expect(comp.state.formData).eql("2.");
const $input = node.querySelector("input");

Simulate.change($input, {
target: { value: ".00" },
});

expect(comp.state.formData).eql(0);
expect($input.value).eql("0");
});

it("should update input values correctly when formData prop changes", () => {
const schema = {
type: "number",
};

const { comp, node } = createFormComponent({
schema,
formData: 2.03,
});

const $input = node.querySelector("input");

expect($input.value).eql("2.03");

setProps(comp, {
schema,
formData: 203,
});

expect($input.value).eql("203");
});

it("should render the widget with the expected id", () => {
Expand All @@ -150,7 +239,7 @@ describe("NumberField", () => {
},
});

expect(node.querySelector("input[type=text]").id).eql("root");
expect(node.querySelector("input[type=number]").id).eql("root");
});

it("should render with trailing zeroes", () => {
Expand Down Expand Up @@ -181,6 +270,19 @@ describe("NumberField", () => {
expect(node.querySelector(".field input").value).eql("2.000");
});

it("should allow a zero to be input", () => {
const { node } = createFormComponent({
schema: {
type: "number",
},
});

Simulate.change(node.querySelector("input"), {
target: { value: "0" },
});
expect(node.querySelector(".field input").value).eql("0");
});

it("should render customized StringField", () => {
const CustomStringField = () => <div id="custom" />;

Expand All @@ -195,6 +297,17 @@ describe("NumberField", () => {

expect(node.querySelector("#custom")).to.exist;
});

it("should use step to represent the multipleOf keyword", () => {
const { node } = createFormComponent({
schema: {
type: "number",
multipleOf: 5,
},
});

expect(node.querySelector("input").step).to.eql("5");
});
});

describe("SelectWidget", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/uiSchema_test.js
Expand Up @@ -1933,7 +1933,7 @@ describe("uiSchema", () => {

it("should disable a number text widget", () => {
shouldBeDisabled(
"input[type=text]",
"input[type=number]",
{
type: "number",
},
Expand Down Expand Up @@ -2226,7 +2226,7 @@ describe("uiSchema", () => {

it("should mark as readonly a number text widget", () => {
shouldBeReadonly(
"input[type=text]",
"input[type=number]",
{
type: "number",
},
Expand Down