From 08ef6bccf116cc0f7fb1e5fbee8dc9b499e41ff3 Mon Sep 17 00:00:00 2001 From: Lucian Buzzo Date: Thu, 13 Dec 2018 16:21:54 +0000 Subject: [PATCH 1/9] Infer type from enum if a type is not provided to SelectWidget (#1100) Connects to #1098 Change-type: minor Signed-off-by: Lucian --- src/components/fields/StringField.js | 6 +- src/components/widgets/SelectWidget.js | 17 +++++- src/utils.js | 2 +- test/BooleanField_test.js | 79 ++++++++++++++++++++++++++ test/NumberField_test.js | 21 +++++++ test/utils_test.js | 27 +++++++++ 6 files changed, 148 insertions(+), 4 deletions(-) diff --git a/src/components/fields/StringField.js b/src/components/fields/StringField.js index c2887e28e3..91fcf6911e 100644 --- a/src/components/fields/StringField.js +++ b/src/components/fields/StringField.js @@ -65,7 +65,11 @@ if (process.env.NODE_ENV !== "production") { onChange: PropTypes.func.isRequired, onBlur: PropTypes.func, onFocus: PropTypes.func, - formData: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), + formData: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.number, + PropTypes.bool, + ]), registry: PropTypes.shape({ widgets: PropTypes.objectOf( PropTypes.oneOfType([PropTypes.func, PropTypes.object]) diff --git a/src/components/widgets/SelectWidget.js b/src/components/widgets/SelectWidget.js index a3da1fe37b..a5ef160355 100644 --- a/src/components/widgets/SelectWidget.js +++ b/src/components/widgets/SelectWidget.js @@ -1,7 +1,7 @@ import React from "react"; import PropTypes from "prop-types"; -import { asNumber } from "../../utils"; +import { asNumber, guessType } from "../../utils"; const nums = new Set(["number", "integer"]); @@ -9,7 +9,9 @@ const nums = new Set(["number", "integer"]); * This is a silly limitation in the DOM where option change event values are * always retrieved as strings. */ -function processValue({ type, items }, value) { +function processValue(schema, value) { + // "enum" is a reserved word, so only "type" and "items" can be destructured + const { type, items } = schema; if (value === "") { return undefined; } else if (type === "array" && items && nums.has(items.type)) { @@ -19,6 +21,17 @@ function processValue({ type, items }, value) { } else if (type === "number") { return asNumber(value); } + + // If type is undefined, but an enum is present, try and infer the type from + // the enum values + if (schema.enum) { + if (schema.enum.every(x => guessType(x) === "number")) { + return asNumber(value); + } else if (schema.enum.every(x => guessType(x) === "boolean")) { + return value === "true"; + } + } + return value; } diff --git a/src/utils.js b/src/utils.js index 816db131dd..47856324d2 100644 --- a/src/utils.js +++ b/src/utils.js @@ -406,7 +406,7 @@ function findSchemaDefinition($ref, definitions = {}) { // In the case where we have to implicitly create a schema, it is useful to know what type to use // based on the data we are defining -const guessType = function guessType(value) { +export const guessType = function guessType(value) { if (Array.isArray(value)) { return "array"; } else if (typeof value === "string") { diff --git a/test/BooleanField_test.js b/test/BooleanField_test.js index 95bb58853b..eaa8a5a983 100644 --- a/test/BooleanField_test.js +++ b/test/BooleanField_test.js @@ -1,6 +1,7 @@ import React from "react"; import { expect } from "chai"; import { Simulate } from "react-addons-test-utils"; +import sinon from "sinon"; import { createFormComponent, createSandbox } from "./test_utils"; @@ -227,4 +228,82 @@ describe("BooleanField", () => { expect(node.querySelector("#label-")).to.not.be.null; }); }); + + describe("SelectWidget", () => { + it("should render a field that contains an enum of booleans", () => { + const { node } = createFormComponent({ + schema: { + enum: [true, false], + }, + }); + + expect(node.querySelectorAll(".field select")).to.have.length.of(1); + }); + + it("should infer the value from an enum on change", () => { + const spy = sinon.spy(); + const { node } = createFormComponent({ + schema: { + enum: [true, false], + }, + onChange: spy, + }); + + expect(node.querySelectorAll(".field select")).to.have.length.of(1); + const $select = node.querySelector(".field select"); + expect($select.value).eql(""); + + Simulate.change($select, { + target: { value: "true" }, + }); + expect($select.value).eql("true"); + expect(spy.lastCall.args[0].formData).eql(true); + }); + + it("should render a string field with a label", () => { + const { node } = createFormComponent({ + schema: { + enum: [true, false], + title: "foo", + }, + }); + + expect(node.querySelector(".field label").textContent).eql("foo"); + }); + + it("should assign a default value", () => { + const { comp } = createFormComponent({ + schema: { + enum: [true, false], + default: true, + }, + }); + + expect(comp.state.formData).eql(true); + }); + + it("should handle a change event", () => { + const { comp, node } = createFormComponent({ + schema: { + enum: [true, false], + }, + }); + + Simulate.change(node.querySelector("select"), { + target: { value: "false" }, + }); + + expect(comp.state.formData).eql(false); + }); + + it("should render the widget with the expected id", () => { + const { node } = createFormComponent({ + schema: { + enum: [true, false], + }, + }); + + expect(node.querySelector("select").id).eql("root"); + }); + }); }); diff --git a/test/NumberField_test.js b/test/NumberField_test.js index f1f5fd4824..b265e8e642 100644 --- a/test/NumberField_test.js +++ b/test/NumberField_test.js @@ -1,6 +1,7 @@ import React from "react"; import { expect } from "chai"; import { Simulate } from "react-addons-test-utils"; +import sinon from "sinon"; import { createFormComponent, createSandbox } from "./test_utils"; @@ -208,6 +209,26 @@ describe("NumberField", () => { expect(node.querySelectorAll(".field select")).to.have.length.of(1); }); + it("should infer the value from an enum on change", () => { + const spy = sinon.spy(); + const { node } = createFormComponent({ + schema: { + enum: [1, 2], + }, + onChange: spy, + }); + + expect(node.querySelectorAll(".field select")).to.have.length.of(1); + const $select = node.querySelector(".field select"); + expect($select.value).eql(""); + + Simulate.change(node.querySelector(".field select"), { + target: { value: "1" }, + }); + expect($select.value).eql("1"); + expect(spy.lastCall.args[0].formData).eql(1); + }); + it("should render a string field with a label", () => { const { node } = createFormComponent({ schema: { diff --git a/test/utils_test.js b/test/utils_test.js index deb24f5a37..0d97f0551c 100644 --- a/test/utils_test.js +++ b/test/utils_test.js @@ -16,6 +16,7 @@ import { shouldRender, toDateString, toIdSchema, + guessType, } from "../src/utils"; describe("utils", () => { @@ -1322,4 +1323,30 @@ describe("utils", () => { ); }); }); + + describe("guessType()", () => { + it("should guess the type of array values", () => { + expect(guessType([1, 2, 3])).eql("array"); + }); + + it("should guess the type of string values", () => { + expect(guessType("foobar")).eql("string"); + }); + + it("should guess the type of null values", () => { + expect(guessType(null)).eql("null"); + }); + + it("should treat undefined values as null values", () => { + expect(guessType()).eql("null"); + }); + + it("should guess the type of boolean values", () => { + expect(guessType(true)).eql("boolean"); + }); + + it("should guess the type of object values", () => { + expect(guessType({})).eql("object"); + }); + }); }); From d6c3c3760c99098bf0e13263d526b0312f18ee8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Tue, 22 Jan 2019 12:30:56 -0500 Subject: [PATCH 2/9] No more useless div on schema field not additional --- .gitignore | 1 + src/components/fields/SchemaField.js | 104 +++++++++++++++------------ 2 files changed, 60 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 03c1b53ecb..d2fdb44322 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ build dist lib yarn.lock +.vscode diff --git a/src/components/fields/SchemaField.js b/src/components/fields/SchemaField.js index b0965678e4..07b5607fb2 100644 --- a/src/components/fields/SchemaField.js +++ b/src/components/fields/SchemaField.js @@ -120,7 +120,6 @@ function ErrorList(props) { function DefaultTemplate(props) { const { id, - classNames, label, children, errors, @@ -129,56 +128,19 @@ function DefaultTemplate(props) { hidden, required, displayLabel, - onKeyChange, - onDropPropertyClick, } = props; if (hidden) { return
{children}
; } - const additional = props.schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); - const keyLabel = `${label} Key`; - return ( -
-
- {additional && ( -
-
-
-
- )} - -
- {displayLabel &&
-
- {additional && ( - - )} -
-
-
+ + {displayLabel && ); } if (process.env.NODE_ENV !== "production") { @@ -209,6 +171,58 @@ DefaultTemplate.defaultProps = { displayLabel: true, }; +function WrapIfAdditional(props) { + const { + id, + classNames, + disabled, + label, + onKeyChange, + onDropPropertyClick, + readonly, + required, + schema, + } = props.parentProps; + const keyLabel = `${label} Key`; // i18n ? + const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); + + if (!additional) { + return
{props.children}
; + } + + return ( +
+
+
+
+
+
+ {props.children} +
+
+
+
+ +
+
+
+
+ ); +} + function SchemaFieldRender(props) { const { uiSchema, From bff64ad82dd99d63c04926fcba15c0263ebf7b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Mon, 28 Jan 2019 11:16:42 -0500 Subject: [PATCH 3/9] Fix tests (remove useles html) - Revert some test added by https://github.com/mozilla-services/react-jsonschema-form/pull/1123/files - use new css class for form-additional insted layout css class for test selection --- test/ArrayFieldTemplate_test.js | 2 +- test/ArrayField_test.js | 3 +-- test/ObjectField_test.js | 26 +++++++++++++++++--------- test/SchemaField_test.js | 2 +- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/test/ArrayFieldTemplate_test.js b/test/ArrayFieldTemplate_test.js index 9abc9db7af..b6bad381e8 100644 --- a/test/ArrayFieldTemplate_test.js +++ b/test/ArrayFieldTemplate_test.js @@ -53,7 +53,7 @@ describe("ArrayFieldTemplate", () => { ArrayFieldTemplate, }); - expect(node.querySelectorAll(".field-array div")).to.have.length.of(6); + expect(node.querySelectorAll(".field-array div")).to.have.length.of(3); }); }); diff --git a/test/ArrayField_test.js b/test/ArrayField_test.js index da17bf630f..65489c9720 100644 --- a/test/ArrayField_test.js +++ b/test/ArrayField_test.js @@ -24,8 +24,7 @@ describe("ArrayField", () => { const { node } = createFormComponent({ schema: { type: "array" } }); expect( - node.querySelector(".field-array > div > div > .unsupported-field") - .textContent + node.querySelector(".field-array > .unsupported-field").textContent ).to.contain("Missing items definition"); }); }); diff --git a/test/ObjectField_test.js b/test/ObjectField_test.js index afd1fb04cb..a90dc6fa59 100644 --- a/test/ObjectField_test.js +++ b/test/ObjectField_test.js @@ -209,7 +209,7 @@ describe("ObjectField", () => { }, }); const labels = [].map.call( - node.querySelectorAll(".field > div > div > label"), + node.querySelectorAll(".field > label"), l => l.textContent ); @@ -224,7 +224,7 @@ describe("ObjectField", () => { }, }); const labels = [].map.call( - node.querySelectorAll(".field > div > div> label"), + node.querySelectorAll(".field > label"), l => l.textContent ); @@ -289,7 +289,7 @@ describe("ObjectField", () => { }, }); const labels = [].map.call( - node.querySelectorAll(".field > div > div > label"), + node.querySelectorAll(".field > label"), l => l.textContent ); @@ -322,7 +322,7 @@ describe("ObjectField", () => { }, }); const labels = [].map.call( - node.querySelectorAll(".field > div > div > label"), + node.querySelectorAll(".field > label"), l => l.textContent ); @@ -720,13 +720,18 @@ describe("ObjectField", () => { }); expect( - node.querySelector(".form-group > .row > .col-xs-2 .btn-danger") + node.querySelector( + ".form-group > .form-additional > .form-additional-actions .btn-danger" + ) ).eql(null); Simulate.click(node.querySelector(".object-property-expand button")); - expect(node.querySelector(".form-group > .row > .col-xs-2 > .btn-danger")) - .to.not.be.null; + expect( + node.querySelector( + ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger" + ) + ).to.not.be.null; }); it("delete button should delete key-value pair", () => { @@ -736,7 +741,9 @@ describe("ObjectField", () => { }); expect(node.querySelector("#root_first-key").value).to.eql("first"); Simulate.click( - node.querySelector(".form-group > .row > .col-xs-2 > .btn-danger") + node.querySelector( + ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger" + ) ); expect(node.querySelector("#root_first-key")).to.not.exist; }); @@ -746,7 +753,8 @@ describe("ObjectField", () => { schema, formData: { first: 1, second: 2, third: 3 }, }); - const selector = ".form-group > .row > .col-xs-2 > .btn-danger"; + const selector = + ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger"; expect(node.querySelectorAll(selector).length).to.eql(3); Simulate.click(node.querySelectorAll(selector)[1]); expect(node.querySelector("#root_second-key")).to.not.exist; diff --git a/test/SchemaField_test.js b/test/SchemaField_test.js index 504e722ce8..fa503d767e 100644 --- a/test/SchemaField_test.js +++ b/test/SchemaField_test.js @@ -313,7 +313,7 @@ describe("SchemaField", () => { submit(node); const matches = node.querySelectorAll( - "form > .form-group > div > div > div > .error-detail .text-danger" + "form > .form-group > div > .error-detail .text-danger" ); expect(matches).to.have.length.of(1); expect(matches[0].textContent).to.eql("container"); From 046d09c846fffcf038d5585a8d928963e000d904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Mon, 11 Feb 2019 12:05:33 -0500 Subject: [PATCH 4/9] Don't use WrapIfAdditonal Component to hav a clerer diff, but adding code duplicate --- src/components/fields/SchemaField.js | 105 +++++++++++++-------------- 1 file changed, 51 insertions(+), 54 deletions(-) diff --git a/src/components/fields/SchemaField.js b/src/components/fields/SchemaField.js index 07b5607fb2..bab007d787 100644 --- a/src/components/fields/SchemaField.js +++ b/src/components/fields/SchemaField.js @@ -120,6 +120,7 @@ function ErrorList(props) { function DefaultTemplate(props) { const { id, + classNames, label, children, errors, @@ -127,20 +128,68 @@ function DefaultTemplate(props) { description, hidden, required, + disabled, + readonly, displayLabel, + onKeyChange, + onDropPropertyClick, + schema, } = props; if (hidden) { return
{children}
; } + const keyLabel = `${label} Key`; // i18n ? + const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); + + if (additional) { + return ( +
+
+
+
+
+
+ {displayLabel && ( +
+
+
+
+ +
+
+
+
+ ); + } + return ( - +
{displayLabel &&
); } if (process.env.NODE_ENV !== "production") { @@ -171,58 +220,6 @@ DefaultTemplate.defaultProps = { displayLabel: true, }; -function WrapIfAdditional(props) { - const { - id, - classNames, - disabled, - label, - onKeyChange, - onDropPropertyClick, - readonly, - required, - schema, - } = props.parentProps; - const keyLabel = `${label} Key`; // i18n ? - const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); - - if (!additional) { - return
{props.children}
; - } - - return ( -
-
-
-
-
-
- {props.children} -
-
-
-
- -
-
-
-
- ); -} - function SchemaFieldRender(props) { const { uiSchema, From 79bfe8082fac0720b1836b398b73f73ca8777a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Thu, 28 Feb 2019 12:03:47 -0500 Subject: [PATCH 5/9] Use a WrapIfAdditional Component to have a cleanner code / diff --- src/components/fields/SchemaField.js | 105 ++++++++++++++------------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/src/components/fields/SchemaField.js b/src/components/fields/SchemaField.js index bab007d787..9d869e54c4 100644 --- a/src/components/fields/SchemaField.js +++ b/src/components/fields/SchemaField.js @@ -120,7 +120,6 @@ function ErrorList(props) { function DefaultTemplate(props) { const { id, - classNames, label, children, errors, @@ -128,68 +127,20 @@ function DefaultTemplate(props) { description, hidden, required, - disabled, - readonly, displayLabel, - onKeyChange, - onDropPropertyClick, - schema, } = props; if (hidden) { return
{children}
; } - const keyLabel = `${label} Key`; // i18n ? - const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); - - if (additional) { - return ( -
-
-
-
-
-
- {displayLabel && ( -
-
-
-
- -
-
-
-
- ); - } - return ( -
+ {displayLabel &&
+
); } if (process.env.NODE_ENV !== "production") { @@ -220,6 +171,58 @@ DefaultTemplate.defaultProps = { displayLabel: true, }; +function WrapIfAdditional(props) { + const { + id, + classNames, + disabled, + label, + onKeyChange, + onDropPropertyClick, + readonly, + required, + schema, + } = props.parentProps; + const keyLabel = `${label} Key`; // i18n ? + const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG); + + if (!additional) { + return
{props.children}
; + } + + return ( +
+
+
+
+
+
+
+ {props.children} +
+
+ +
+
+
+ ); +} + function SchemaFieldRender(props) { const { uiSchema, From 1c8233a9483322b3be9e610489aa8c0634aa8a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Thu, 28 Feb 2019 12:07:21 -0500 Subject: [PATCH 6/9] Fix tests --- test/ObjectField_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ObjectField_test.js b/test/ObjectField_test.js index a90dc6fa59..b9c2884065 100644 --- a/test/ObjectField_test.js +++ b/test/ObjectField_test.js @@ -729,7 +729,7 @@ describe("ObjectField", () => { expect( node.querySelector( - ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger" + ".form-group > .row > .form-additional + .col-xs-2 > .btn-danger" ) ).to.not.be.null; }); @@ -742,7 +742,7 @@ describe("ObjectField", () => { expect(node.querySelector("#root_first-key").value).to.eql("first"); Simulate.click( node.querySelector( - ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger" + ".form-group > .row > .form-additional + .col-xs-2 > .btn-danger" ) ); expect(node.querySelector("#root_first-key")).to.not.exist; @@ -754,7 +754,7 @@ describe("ObjectField", () => { formData: { first: 1, second: 2, third: 3 }, }); const selector = - ".form-group > .form-additional > .form-additional-actions > .btn-group > .btn-danger"; + ".form-group > .row > .form-additional + .col-xs-2 > .btn-danger"; expect(node.querySelectorAll(selector).length).to.eql(3); Simulate.click(node.querySelectorAll(selector)[1]); expect(node.querySelector("#root_second-key")).to.not.exist; From 56d1a8d0a1b2c06b7797a4a5f3794ff154d5ca3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Thu, 28 Feb 2019 12:12:23 -0500 Subject: [PATCH 7/9] Ignore vscode Ide folder --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 7f2ca177f1..b5009d0cbe 100644 --- a/.gitignore +++ b/.gitignore @@ -122,6 +122,9 @@ dist lib yarn.lock +# IDE +.vscode + # Code coverage coverage .nyc_output From 941d63419fc63694835675082721083c545123bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Pi=C3=A9plu?= Date: Tue, 5 Mar 2019 11:59:29 -0500 Subject: [PATCH 8/9] Remove useless parentProps --- src/components/fields/SchemaField.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/fields/SchemaField.js b/src/components/fields/SchemaField.js index 9d869e54c4..38df7a044b 100644 --- a/src/components/fields/SchemaField.js +++ b/src/components/fields/SchemaField.js @@ -134,7 +134,7 @@ function DefaultTemplate(props) { } return ( - + {displayLabel &&