Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/components/fields/ArrayField.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ function DefaultFixedArrayFieldTemplate(props) {
}

function DefaultNormalArrayFieldTemplate(props) {
const canDelete =
!("minItems" in props.schema) || props.items.length > props.schema.minItems;
return (
<fieldset className={props.className}>

Expand All @@ -161,7 +163,14 @@ function DefaultNormalArrayFieldTemplate(props) {
<div
className="row array-item-list"
key={`array-item-list-${props.idSchema.$id}`}>
{props.items && props.items.map(p => DefaultArrayItem(p))}
{props.items &&
props.items.map(p => {
const { hasRemove, ...ps } = p;
if (canDelete) {
ps.hasRemove = hasRemove;
}
return DefaultArrayItem(ps);
})}
</div>

{props.canAdd &&
Expand Down Expand Up @@ -301,7 +310,8 @@ class ArrayField extends Component {
const { addable = true } = getUiOptions(uiSchema);

const arrayProps = {
canAdd: addable,
canAdd: addable &&
(!("maxItems" in schema) || formData.length < schema.maxItems),
items: formData.map((item, index) => {
const itemErrorSchema = errorSchema ? errorSchema[index] : undefined;
const itemIdPrefix = idSchema.$id + "_" + index;
Expand Down
9 changes: 7 additions & 2 deletions src/components/fields/ObjectField.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ObjectField extends Component {
onBlur,
} = this.props;
const { definitions, fields, formContext } = this.props.registry;
const { SchemaField, TitleField, DescriptionField } = fields;
const { ObjectPropertyField, TitleField, DescriptionField } = fields;
Copy link
Copy Markdown
Contributor Author

@eggyal eggyal Apr 25, 2017

Choose a reason for hiding this comment

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

With circular schema, we must avoid rendering everything: object properties should only be rendered when necessary. I define a new component, ObjectPropertyField that only renders the underlying SchemaField when necessary—i.e. if the schema requires the property to be present, if the formData contains an explicit value for it, or if the user explicitly requests the property to be shown.

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.

Consider putting this comment as JSDoc on the ObjectPropertyField class.

const schema = retrieveSchema(this.props.schema, definitions);
const title = schema.title === undefined ? name : schema.title;
let orderedProperties;
Expand Down Expand Up @@ -79,8 +79,12 @@ class ObjectField extends Component {
formContext={formContext}
/>}
{orderedProperties.map((name, index) => {
const isDefault =
!(name in formData) ||
typeof Object.getOwnPropertyDescriptor(formData, name).get ===
"function";
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.

Here I'm trying determine whether the value in formData was given explicitly, or if it's just a default value from the lazily-evaluated Proxy object (this is used in the ObjectPropertyField component to determine whether the underlying SchemaField should be rendered). It may be better to pass explicit formData separately from default data?

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.

Yes, I have to say this is super ugly, and passing explicit formData and defaults would be better in a perfect world. On the other hand, I'm not sure I want to separate them as part of this PR. I think extracting this into a variable called isDefault is good enough, although maybe you could add a FIXME comment explaining this.

return (
<SchemaField
<ObjectPropertyField
key={index}
name={name}
required={this.isRequired(name)}
Expand All @@ -94,6 +98,7 @@ class ObjectField extends Component {
registry={this.props.registry}
disabled={disabled}
readonly={readonly}
isDefault={isDefault}
/>
);
})}
Expand Down
74 changes: 74 additions & 0 deletions src/components/fields/ObjectPropertyField.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React, { Component, PropTypes } from "react";

import { getDefaultRegistry } from "../../utils";

class ObjectPropertyField extends Component {
static defaultProps = {
uiSchema: {},
errorSchema: {},
idSchema: {},
registry: getDefaultRegistry(),
disabled: false,
readonly: false,
autofocus: false,
isDefault: true,
};

constructor(props) {
super(props);
this.state = { present: props.required || !props.isDefault };
}

show = event => {
event.preventDefault();
this.setState({ present: true });
};

hide = event => {
event.preventDefault();
this.setState({ present: false });
};

render() {
const title = this.props.schema.title === undefined
? this.props.name
: this.props.schema.title;

if (this.state.present) {
const SchemaField = this.props.registry.fields.SchemaField;
const { isDefault, ...props } = this.props;
return (
<div>
<SchemaField {...props} />
{this.props.required ||
<button onClick={this.hide}>REMOVE {title}</button>}
</div>
);
} else {
return <button onClick={this.show}>ADD {title}</button>;
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.

No doubt these <button/> elements should be replaced with a better UI?

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'm pretty nervous about the UX in general. I know that @n1k0 took a swing at collapsibility before in #268 and wasn't able to find a UX that he found felt right for most use cases, and this feels like a similar situation to me. Can we add this only if we are in a circular-schema situation? (N.B. I haven't actually tried running the code so this may not be as intrusive as I imagine it being.)

}
}
}

if (process.env.NODE_ENV !== "production") {
ObjectPropertyField.propTypes = {
schema: PropTypes.object.isRequired,
uiSchema: PropTypes.object,
idSchema: PropTypes.object,
formData: PropTypes.any,
errorSchema: PropTypes.object,
registry: PropTypes.shape({
widgets: PropTypes.objectOf(
PropTypes.oneOfType([PropTypes.func, PropTypes.object])
).isRequired,
fields: PropTypes.objectOf(PropTypes.func).isRequired,
definitions: PropTypes.object.isRequired,
ArrayFieldTemplate: PropTypes.func,
FieldTemplate: PropTypes.func,
formContext: PropTypes.object.isRequired,
}),
isDefault: PropTypes.bool,
};
}

export default ObjectPropertyField;
2 changes: 2 additions & 0 deletions src/components/fields/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import BooleanField from "./BooleanField";
import DescriptionField from "./DescriptionField";
import NumberField from "./NumberField";
import ObjectField from "./ObjectField";
import ObjectPropertyField from "./ObjectPropertyField";
import SchemaField from "./SchemaField";
import StringField from "./StringField";
import TitleField from "./TitleField";
Expand All @@ -14,6 +15,7 @@ export default {
DescriptionField,
NumberField,
ObjectField,
ObjectPropertyField,
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'm not sure we really need to expose ObjectPropertyField?

SchemaField,
StringField,
TitleField,
Expand Down
136 changes: 109 additions & 27 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,40 @@ function computeDefaults(schema, parentDefaults, definitions = {}) {
switch (schema.type) {
// We need to recur for object schema inner default values.
case "object":
return Object.keys(schema.properties).reduce((acc, key) => {
// Compute the defaults for this node, with the parent defaults we might
// have from a previous run: defaults[key].
acc[key] = computeDefaults(
schema.properties[key],
(defaults || {})[key],
definitions
);
return acc;
}, {});
return new Proxy(
{},
{
get(obj, key) {
if (!(key in obj) && key in schema.properties) {
obj[key] = computeDefaults(
schema.properties[key],
(defaults || {})[key],
definitions
);
}

return obj[key];
},

has(obj, key) {
return key in schema.properties;
},

ownKeys(obj) {
return Object.keys(schema.properties);
},

getOwnPropertyDescriptor(obj, key) {
return this.has(obj, key)
? {
configurable: true,
enumerable: true,
get: this.get.bind(this, obj, key),
}
: void 0;
},
}
);

case "array":
if (schema.minItems) {
Expand All @@ -157,15 +181,47 @@ export function getDefaultFormState(_schema, formData, definitions = {}) {
}
const schema = retrieveSchema(_schema, definitions);
const defaults = computeDefaults(schema, _schema.default, definitions);
return getDefault(formData, defaults);
}

function getDefault(formData, defaults) {
if (typeof formData === "undefined") {
// No form data? Use schema defaults.
return defaults;
}
if (isObject(formData)) {
// Override schema defaults with form data.
return mergeObjects(defaults, formData);
let copyObj = {};
Object.keys(formData).forEach(key => {
copyObj[key] = getDefault(formData[key], defaults[key]);
});

return new Proxy(copyObj, {
get(obj, key) {
if (!(key in obj) && key in defaults) {
obj[key] = defaults[key];
}

return obj[key];
},

has(obj, key) {
return key in defaults;
},

ownKeys(obj) {
return Object.keys(defaults);
},

getOwnPropertyDescriptor(obj, key) {
return Object.getOwnPropertyDescriptor(
key in obj ? obj : defaults,
key
);
},
});
}
return formData || defaults;
return formData;
}

export function getUiOptions(uiSchema) {
Expand Down Expand Up @@ -320,6 +376,7 @@ function findSchemaDefinition($ref, definitions = {}) {
let current = definitions;
for (let part of parts) {
part = part.replace(/~1/g, "/").replace(/~0/g, "~");
current = retrieveSchema(current, definitions);
if (current.hasOwnProperty(part)) {
current = current[part];
} else {
Expand All @@ -335,16 +392,18 @@ function findSchemaDefinition($ref, definitions = {}) {
}

export function retrieveSchema(schema, definitions = {}) {
let current = schema;
while (current.hasOwnProperty("$ref")) {
// Retrieve the referenced schema definition.
const $refSchema = findSchemaDefinition(current.$ref, definitions);
// Drop the $ref property of the source schema.
const { $ref, ...localSchema } = current;
// Update referenced schema definition with local schema properties.
current = { ...$refSchema, ...localSchema };
}

// No $ref attribute found, returning the original schema.
if (!schema.hasOwnProperty("$ref")) {
return schema;
}
// Retrieve the referenced schema definition.
const $refSchema = findSchemaDefinition(schema.$ref, definitions);
// Drop the $ref property of the source schema.
const { $ref, ...localSchema } = schema;
// Update referenced schema definition with local schema properties.
return { ...$refSchema, ...localSchema };
return current;
}

function isArguments(object) {
Expand Down Expand Up @@ -447,12 +506,35 @@ export function toIdSchema(schema, id, definitions) {
if (schema.type !== "object") {
return idSchema;
}
for (const name in schema.properties || {}) {
const field = schema.properties[name];
const fieldId = idSchema.$id + "_" + name;
idSchema[name] = toIdSchema(field, fieldId, definitions);
}
return idSchema;
return new Proxy(idSchema, {
get(idSchema, name) {
if (!(name in idSchema) && name in schema.properties) {
const field = schema.properties[name];
const fieldId = idSchema.$id + "_" + name;
idSchema[name] = toIdSchema(field, fieldId, definitions);
}

return idSchema[name];
},

has(idSchema, name) {
return name === "$id" || name in schema.properties;
},

ownKeys(idSchema) {
return ["$id"].concat(Object.keys(schema.properties));
},

getOwnPropertyDescriptor(idSchema, name) {
return this.has(idSchema, name)
? {
configurable: true,
enumerable: true,
get: this.get.bind(this, idSchema, name),
}
: void 0;
},
});
}

export function parseDateString(dateString, includeTime = true) {
Expand Down
3 changes: 3 additions & 0 deletions test/ArrayField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,11 @@ describe("ArrayField", () => {
it("should render enough inputs with proper defaults to match minItems in schema when no formData is set", () => {
const complexSchema = {
type: "object",
required: ["foo"],
definitions: {
Thing: {
type: "object",
required: ["name"],
properties: {
name: {
type: "string",
Expand Down Expand Up @@ -870,6 +872,7 @@ describe("ArrayField", () => {
it("should pass field name to TitleField if there is no title", () => {
const schema = {
type: "object",
required: ["array"],
properties: {
array: {
type: "array",
Expand Down
1 change: 1 addition & 0 deletions test/BooleanField_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ describe("BooleanField", () => {
it("should pass field name to widget if there is no title", () => {
const schema = {
type: "object",
required: ["boolean"],
properties: {
boolean: {
type: "boolean",
Expand Down
Loading