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

Add delete button for additionalProperties key-value pair #1123

Merged
merged 15 commits into from
Jan 10, 2019
40 changes: 36 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,24 @@ class ObjectField extends Component {
);
}

onPropertyChange = name => {
onPropertyChange = (name, addedByAdditionalProperties = false) => {
return (value, errorSchema) => {
const newFormData = { ...this.props.formData, [name]: value };
let newFormData;
//section below sets zero value of input field to empty string
//instead of undefined, so that value input in additionalProperties
//doesn't disappear when emptied
if (!value && addedByAdditionalProperties) {
newFormData = {
...this.props.formData,
[name]: "",
};
glasserc marked this conversation as resolved.
Show resolved Hide resolved
} else {
newFormData = {
...this.props.formData,
[name]: value,
};
}

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

onDropIndexClick = key => {
return event => {
event.preventDefault();
const { onChange, formData } = this.props;
const copiedFormData = { ...formData };
delete copiedFormData[key];
onChange(copiedFormData);
};
};

getAvailableKey = (preferredKey, formData) => {
var index = 0;
var newKey = preferredKey;
Expand Down Expand Up @@ -176,7 +202,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 +225,9 @@ class ObjectField extends Component {
TitleField,
DescriptionField,
properties: orderedProperties.map(name => {
const addedByAdditionalProperties = schema.properties[
name
].hasOwnProperty(ADDITIONAL_PROPERTY_FLAG);
return {
content: (
<SchemaField
Expand All @@ -213,12 +241,16 @@ class ObjectField extends Component {
idPrefix={idPrefix}
formData={formData[name]}
onKeyChange={this.onKeyChange(name)}
onChange={this.onPropertyChange(name)}
onChange={this.onPropertyChange(
name,
addedByAdditionalProperties
)}
onBlur={onBlur}
onFocus={onFocus}
registry={registry}
disabled={disabled}
readonly={readonly}
onDropIndexClick={this.onDropIndexClick}
/>
),
name,
Expand Down
58 changes: 41 additions & 17 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,57 @@ function DefaultTemplate(props) {
required,
displayLabel,
onKeyChange,
onDropIndexClick,
} = props;
if (hidden) {
return children;
}

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

return (
<div className={classNames}>
{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={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={onDropIndexClick(props.label)}
/>
)}
</div>
)}
{displayLabel && <Label label={label} required={required} id={id} />}
{displayLabel && description ? description : null}
{children}
{errors}
{help}
</div>
Copy link
Contributor

@pieplu pieplu Jan 18, 2019

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
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
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

</div>
);
}

if (process.env.NODE_ENV !== "production") {
DefaultTemplate.propTypes = {
id: PropTypes.string,
Expand Down Expand Up @@ -186,6 +208,7 @@ function SchemaFieldRender(props) {
idPrefix,
name,
onKeyChange,
onDropIndexClick,
required,
registry = getDefaultRegistry(),
} = props;
Expand Down Expand Up @@ -285,6 +308,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(6);
});
});

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 > div > .unsupported-field")
.textContent
).to.contain("Missing items definition");
});
});
Expand Down
65 changes: 61 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 > 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 > 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 > 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 > div > label"),
l => l.textContent
);

Expand Down Expand Up @@ -678,5 +678,62 @@ 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 > .row > .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;
});

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 > .row > .col-xs-2 > .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 },
});
const selector = ".form-group > .row > .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;
expect(node.querySelectorAll(selector).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 > div > .error-detail .text-danger"
);
expect(matches).to.have.length.of(1);
expect(matches[0].textContent).to.eql("container");
Expand Down