Bug Fix empty html useless added by PR #1123 / v1.2.0 (#1158)

* Infer type from enum if a type is not provided to SelectWidget (#1100)

Connects to #1098

Change-type: minor
Signed-off-by: Lucian <lucian.buzzo@gmail.com>

* No more useless div on schema field not additional

* 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

* Don't use WrapIfAdditonal Component to hav a clerer diff, but adding code duplicate

* Use a WrapIfAdditional Component to have a cleanner code / diff

* Fix tests

* Ignore vscode Ide folder

* Remove useless parentProps

* Fix old test selector for form additional

Author: pieplu

.gitignore

@@ -122,6 +122,9 @@ dist
 lib
 yarn.lock
 
+# IDE
+.vscode
+
 # Code coverage
 coverage
 .nyc_output

src/components/fields/SchemaField.js

@@ -122,7 +122,6 @@ function ErrorList(props) {
 function DefaultTemplate(props) {
   const {
     id,
-    classNames,
     label,
     children,
     errors,
@@ -131,56 +130,19 @@ function DefaultTemplate(props) {
     hidden,
     required,
     displayLabel,
-    onKeyChange,
-    onDropPropertyClick,
   } = props;
   if (hidden) {
     return <div className="hidden">{children}</div>;
   }
 
-  const additional = props.schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG);
-  const keyLabel = `${label} Key`;
-
   return (
-    <div className={classNames}>
-      <div className={additional ? "row" : ""}>
-        {additional && (
-          <div className="col-xs-5 form-additional">
-            <div className="form-group">
-              <Label label={keyLabel} required={required} id={`${id}-key`} />
-              <LabelInput
-                label={label}
-                required={required}
-                id={`${id}-key`}
-                onChange={onKeyChange}
-              />
-            </div>
-          </div>
-        )}
-
-        <div
-          className={additional ? "form-additional form-group col-xs-5" : ""}>
-          {displayLabel && <Label label={label} required={required} id={id} />}
-          {displayLabel && description ? description : null}
-          {children}
-          {errors}
-          {help}
-        </div>
-        <div className="col-xs-2">
-          {additional && (
-            <IconButton
-              type="danger"
-              icon="remove"
-              className="array-item-remove btn-block"
-              tabIndex="-1"
-              style={{ border: "0" }}
-              disabled={props.disabled || props.readonly}
-              onClick={onDropPropertyClick(props.label)}
-            />
-          )}
-        </div>
-      </div>
-    </div>
+    <WrapIfAdditional {...props}>
+      {displayLabel && <Label label={label} required={required} id={id} />}
+      {displayLabel && description ? description : null}
+      {children}
+      {errors}
+      {help}
+    </WrapIfAdditional>
   );
 }
 if (process.env.NODE_ENV !== "production") {
@@ -211,6 +173,58 @@ DefaultTemplate.defaultProps = {
   displayLabel: true,
 };
 
+function WrapIfAdditional(props) {
+  const {
+    id,
+    classNames,
+    disabled,
+    label,
+    onKeyChange,
+    onDropPropertyClick,
+    readonly,
+    required,
+    schema,
+  } = props;
+  const keyLabel = `${label} Key`; // i18n ?
+  const additional = schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG);
+
+  if (!additional) {
+    return <div className={classNames}>{props.children}</div>;
+  }
+
+  return (
+    <div className={classNames}>
+      <div className="row">
+        <div className="col-xs-5 form-additional">
+          <div className="form-group">
+            <Label label={keyLabel} required={required} id={`${id}-key`} />
+            <LabelInput
+              label={label}
+              required={required}
+              id={`${id}-key`}
+              onChange={onKeyChange}
+            />
+          </div>
+        </div>
+        <div className="form-additional form-group col-xs-5">
+          {props.children}
+        </div>
+        <div className="col-xs-2">
+          <IconButton
+            type="danger"
+            icon="remove"
+            className="array-item-remove btn-block"
+            tabIndex="-1"
+            style={{ border: "0" }}
+            disabled={disabled || readonly}
+            onClick={onDropPropertyClick(label)}
+          />
+        </div>
+      </div>
+    </div>
+  );
+}
+
 function SchemaFieldRender(props) {
   const {
     uiSchema,

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);
       });
     });
 

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");
     });
   });

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 + .col-xs-2 .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 > .row > .form-additional + .col-xs-2 > .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 > .row > .form-additional + .col-xs-2 > .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 > .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;

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");