Skip to content

Commit

Permalink
Improve handling of decimal points and trailing zeroes in numbers (#1183
Browse files Browse the repository at this point in the history
)

Connects to #674 #958

Change-type: patch
Signed-off-by: Lucian <lucian.buzzo@gmail.com>
  • Loading branch information
LucianBuzzo authored and epicfaace committed Feb 26, 2019
1 parent 07decc5 commit aebfab9
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 26 deletions.
91 changes: 83 additions & 8 deletions src/components/fields/NumberField.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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;
}

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

BaseInput.defaultProps = {
type: "text",
required: false,
disabled: false,
readonly: false,
Expand Down
8 changes: 4 additions & 4 deletions test/ArrayField_test.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit aebfab9

Please sign in to comment.