Skip to content
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ build
dist
lib
yarn.lock
.vscode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this belongs here. It would be better to put this in your .git/info/exclude.

36 changes: 32 additions & 4 deletions src/components/fields/ObjectField.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
retrieveSchema,
getDefaultRegistry,
getUiOptions,
ADDITIONAL_PROPERTY_FLAG,
} from "../../utils";

function DefaultObjectFieldTemplate(props) {
Expand Down Expand Up @@ -79,9 +80,21 @@ class ObjectField extends Component {
);
}

onPropertyChange = name => {
onPropertyChange = (name, additionalProperties = false) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about naming this parameter isAdditionalProperty or addedByAdditionalProperties?

return (value, errorSchema) => {
const newFormData = { ...this.props.formData, [name]: value };
let newFormData;
if (!value && additionalProperties) {
newFormData = {
...this.props.formData,
[name]: "",
};
Comment thread
glasserc marked this conversation as resolved.
} else {
newFormData = {
...this.props.formData,
[name]: value,
};
}

this.props.onChange(
newFormData,
errorSchema &&
Expand All @@ -93,6 +106,18 @@ class ObjectField extends Component {
};
};

onDropIndexClick = index => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name index in the names of the prop, the function, and the argument are misleading. This should be a string property name, right?

return event => {
if (event) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we need to check whether event exists or not..

event.preventDefault();
}
const { onChange, formData } = this.props;
const copiedFormData = { ...formData };
delete copiedFormData[index];
onChange(copiedFormData);
};
};

getAvailableKey = (preferredKey, formData) => {
var index = 0;
var newKey = preferredKey;
Expand Down Expand Up @@ -176,7 +201,6 @@ class ObjectField extends Component {
const title = schema.title === undefined ? name : schema.title;
const description = uiSchema["ui:description"] || schema.description;
let orderedProperties;

try {
const properties = Object.keys(schema.properties);
orderedProperties = orderProperties(properties, uiSchema["ui:order"]);
Expand All @@ -200,6 +224,9 @@ class ObjectField extends Component {
TitleField,
DescriptionField,
properties: orderedProperties.map(name => {
const additionalPropertyFlag = schema.properties[name].hasOwnProperty(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to rename this variable too.

ADDITIONAL_PROPERTY_FLAG
);
return {
content: (
<SchemaField
Expand All @@ -213,12 +240,13 @@ class ObjectField extends Component {
idPrefix={idPrefix}
formData={formData[name]}
onKeyChange={this.onKeyChange(name)}
onChange={this.onPropertyChange(name)}
onChange={this.onPropertyChange(name, additionalPropertyFlag)}
onBlur={onBlur}
onFocus={onFocus}
registry={registry}
disabled={disabled}
readonly={readonly}
onDropIndexClick={this.onDropIndexClick}
/>
),
name,
Expand Down
69 changes: 53 additions & 16 deletions src/components/fields/SchemaField.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ADDITIONAL_PROPERTY_FLAG } from "../../utils";
import IconButton from "../IconButton";
import React from "react";
import PropTypes from "prop-types";

Expand Down Expand Up @@ -107,7 +108,6 @@ function ErrorList(props) {
</div>
);
}

function DefaultTemplate(props) {
const {
id,
Expand All @@ -121,35 +121,70 @@ function DefaultTemplate(props) {
required,
displayLabel,
onKeyChange,
onDropIndexClick,
} = props;
if (hidden) {
return children;
}
const btnStyle = {
flex: 1,
paddingLeft: 10,
paddingRight: 10,
fontWeight: "bold",
height: "34px",
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project doesn't use inline styles. You can rely on Bootstrap styles.

const additional = props.schema.hasOwnProperty(ADDITIONAL_PROPERTY_FLAG);
const keyLabel = `${label} Key`;

const contentStyle = additional
? {
clear: "both",
display: "flex",
marginBottom: 0,
}
: {};

return (
<div className={classNames}>
<div className={classNames} style={contentStyle}>
{additional && (
<div className="form-group">
<Label label={keyLabel} required={required} id={`${id}-key`} />
<LabelInput
label={label}
required={required}
id={`${id}-key`}
onChange={onKeyChange}
/>
<div
className="col-lg-6"
style={{
padding: 0,
}}>
<div className="form-group form-additional">
<Label label={keyLabel} required={required} id={`${id}-key`} />
<LabelInput
label={label}
required={required}
id={`${id}-key`}
onChange={onKeyChange}
/>
</div>
</div>
)}
{displayLabel && <Label label={label} required={required} id={id} />}
{displayLabel && description ? description : null}
{children}
{errors}
{help}

<div className={additional ? "col-lg-6 form-additional" : ""}>
{displayLabel && <Label label={label} required={required} id={id} />}
{displayLabel && description ? description : null}
{children}
{errors}
{help}
</div>

@pieplu pieplu Jan 18, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code add useless empty div (and a empty col-2) for all fields

Before:
image

After:
image

I think it's a regression. In our case, its break some styles

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pieplu I agree that lines 134 and 135 of this file can surely be merged into one (mistake on my side), which would probably leave just one empty div which is in line 150, but I at the first glance I don't know how to achieve same appearence using Bootstrap 3, and have a look at this: #1123 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I can try PR a fix with bootstrap style

{additional && (
<IconButton
type="danger"
icon="remove"
className="array-item-remove"
tabIndex="-1"
style={btnStyle}
disabled={props.disabled || props.readonly}
onClick={onDropIndexClick(props.label)}
/>
)}
</div>
);
}

if (process.env.NODE_ENV !== "production") {
DefaultTemplate.propTypes = {
id: PropTypes.string,
Expand Down Expand Up @@ -186,6 +221,7 @@ function SchemaFieldRender(props) {
idPrefix,
name,
onKeyChange,
onDropIndexClick,
required,
registry = getDefaultRegistry(),
} = props;
Expand Down Expand Up @@ -285,6 +321,7 @@ function SchemaFieldRender(props) {
label,
hidden,
onKeyChange,
onDropIndexClick,
required,
disabled,
readonly,
Expand Down
1 change: 0 additions & 1 deletion src/components/fields/StringField.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ function StringField(props) {
uiSchema
);
const Widget = getWidget(schema, widget, widgets);

return (
<Widget
options={{ ...options, enumOptions }}
Expand Down
2 changes: 1 addition & 1 deletion test/ArrayFieldTemplate_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("ArrayFieldTemplate", () => {
ArrayFieldTemplate,
});

expect(node.querySelectorAll(".field-array div")).to.have.length.of(3);
expect(node.querySelectorAll(".field-array div")).to.have.length.of(4);
});
});

Expand Down
3 changes: 2 additions & 1 deletion test/ArrayField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ describe("ArrayField", () => {
const { node } = createFormComponent({ schema: { type: "array" } });

expect(
node.querySelector(".field-array > .unsupported-field").textContent
node.querySelector(".field-array > div > .unsupported-field")
.textContent
).to.contain("Missing items definition");
});
});
Expand Down
63 changes: 59 additions & 4 deletions test/ObjectField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe("ObjectField", () => {
},
});
const labels = [].map.call(
node.querySelectorAll(".field > label"),
node.querySelectorAll(".field > div > label"),
l => l.textContent
);

Expand All @@ -212,7 +212,7 @@ describe("ObjectField", () => {
},
});
const labels = [].map.call(
node.querySelectorAll(".field > label"),
node.querySelectorAll(".field > div > label"),
l => l.textContent
);

Expand Down Expand Up @@ -277,7 +277,7 @@ describe("ObjectField", () => {
},
});
const labels = [].map.call(
node.querySelectorAll(".field > label"),
node.querySelectorAll(".field > div > label"),
l => l.textContent
);

Expand Down Expand Up @@ -310,7 +310,7 @@ describe("ObjectField", () => {
},
});
const labels = [].map.call(
node.querySelectorAll(".field > label"),
node.querySelectorAll(".field > div > label"),
l => l.textContent
);

Expand Down Expand Up @@ -678,5 +678,60 @@ describe("ObjectField", () => {

expect(node.querySelector(".object-property-expand button")).to.be.null;
});

it("should not have delete button if expand button has not been clicked", () => {
const { node } = createFormComponent({ schema });

expect(node.querySelector(".form-group > .btn-danger")).eql(null);
});

it("should have delete button if expand button has been clicked", () => {
const { node } = createFormComponent({
schema,
});

expect(node.querySelector(".form-group > .btn-danger")).eql(null);

Simulate.click(node.querySelector(".object-property-expand button"));

expect(node.querySelector(".form-group > .btn-danger")).to.not.be.null;
});

it("delete button should delete key-value pair", () => {
const { node } = createFormComponent({
schema,
formData: { first: 1 },
});
expect(node.querySelector("#root_first-key").value).to.eql("first");
Simulate.click(node.querySelector(".form-group > .btn-danger"));
expect(node.querySelector("#root_first-key")).to.not.exist;
});

it("delete button should delete correct pair", () => {
const { node } = createFormComponent({
schema,
formData: { first: 1, second: 2, third: 3 },
});
expect(node.querySelectorAll(".form-group > .btn-danger").length).to.eql(
3
);
Simulate.click(node.querySelectorAll(".form-group > .btn-danger")[1]);
expect(node.querySelector("#root_second-key")).to.not.exist;
expect(node.querySelectorAll(".form-group > .btn-danger").length).to.eql(
2
);
});

it("deleting content of value input should not delete pair", () => {
const { comp, node } = createFormComponent({
schema,
formData: { first: 1 },
});

Simulate.change(node.querySelector("#root_first"), {
target: { value: "" },
});
expect(comp.state.formData["first"]).eql("");
});
});
});
2 changes: 1 addition & 1 deletion test/SchemaField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ describe("SchemaField", () => {
submit(node);

const matches = node.querySelectorAll(
"form > .form-group > div > .error-detail .text-danger"
"form > .form-group > div > div > .error-detail .text-danger"
);
expect(matches).to.have.length.of(1);
expect(matches[0].textContent).to.eql("container");
Expand Down