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 widget capability to Array and Object fields #225

Closed
wants to merge 1 commit into from

Conversation

kdmcclel
Copy link

@kdmcclel kdmcclel commented Jun 2, 2016

I had an array of roles for a user which I wanted to present as switches. I wanted to be able to do this inline without having to declare all my fields manually. Adding the capability for a widget to be used on ArrayField and ObjectField gives more flexibility.

@kdmcclel
Copy link
Author

kdmcclel commented Jun 2, 2016

Didn't see the checkbox widget on multiselect. I'll fix these errors and squash.

@kdmcclel kdmcclel force-pushed the master branch 5 times, most recently from b0e0725 to 2d83018 Compare June 6, 2016 18:31
@n1k0
Copy link
Collaborator

n1k0 commented Jun 21, 2016

This is an interesting extension of widgets scope. Would you mind adding some tests to help me figuring out the implication of such a change? Thanks.

@n1k0
Copy link
Collaborator

n1k0 commented Jul 11, 2016

@kdmcclel do you plan on keeping working on this or should I take over? Thanks

@n1k0
Copy link
Collaborator

n1k0 commented Jul 12, 2016

Okay, I'm gonna challenge this need; what would this bring over custom fields?

  • Custom field can render any free form HTML for any JSON schema types; including array and object fields;
  • Custom fields allow rendering a full form row, meaning you can render labels, requirements, errors list, help texts, etc. the way you like;
  • Also custom array and object widgets would mean ideally accessing the schema and uiSchema, possibly idSchema and errorSchema as well, which would imho broaden the concept of widget far too much.

So a few questions here:

  1. Do we actually want this?
  2. If so, why?
  3. What are the actual problems we're trying to solve?
  4. If custom fields are not enough, what's missing?

@amarnus
Copy link
Contributor

amarnus commented Jul 12, 2016

It is true that a custom field can be used to extend default behaviour. There are some field-level features though. Eg. Ordered properties for objects. Custom fields based off the ObjectField will have to replicate this for no reason.

Also, your question can be extended to: Why do we need widgets when we can create custom fields? Even for primitive field types. Especially because the signature for both, is roughly the same: f(Value, Schema, ...) -> Virtual DOM

@n1k0
Copy link
Collaborator

n1k0 commented Jul 12, 2016

There are some field-level features though. Eg. Ordered properties for objects. Custom fields based off the ObjectField will have to replicate this for no reason.

As far as I can tell, having custom object widgets wouldn't solve this, would it? What API would you provide in order for the custom widget component to know about the order of object fields?

Also, your question can be extended to: Why do we need widgets when we can create custom fields? Even for primitive field types. Especially because the signature for both, is roughly the same: f(Value, Schema, ...) -> Virtual DOM

There's actually a documented distinction between fields and widgets:

The API allows to specify your own custom widgets and fields components:

  • A widget represents a HTML tag for the user to enter data, eg. input, select, etc.
  • A field usually wraps one or more widgets and most often handles internal field state; think of a field as a form row, including the labels.

That's especially why I've always be reluctant at transforming widgets into more than single input components.

@kdmcclel
Copy link
Author

Sorry I've been meaning to write those tests just been very busy.

I understand the documented distinction but they seem to behave very similarly.

I initially used custom fields for my roles example (dynamic options list switches). The problem was with a custom field I was forced to specify all fields in the form. I wanted to let one form page handle all objects based on schema.

Perhaps a better modification would have been to add ui:field tag to ui json?

@amarnus
Copy link
Contributor

amarnus commented Jul 12, 2016

You are right. Ordered properties was not a very good example.

handles internal field state

If we take this alone as the definition of a field, then I think object/array widgets still make sense as the logic of handling the internal data is the same irrespective of whether you want to render an object sub-schema in a regular fieldset or within a set of tabs.

Also custom array and object widgets would mean ideally accessing the schema and uiSchema, possibly idSchema and errorSchema as well, which would imho broaden the concept of widget far too much.

If this is the main concern and we can make an assumption that a widget will work only on its immediate children, we can provide a renderChild function as a prop to the Widget that instantiates and renders SchemaField - the widget is responsible for calling this function at the right place with the right child.

@n1k0
Copy link
Collaborator

n1k0 commented Jul 12, 2016

@kdmcclel

Perhaps a better modification would have been to add ui:field tag to ui json?

I don't understand; there's already a ui:field uiSchema directive. What do you mean?

@kdmcclel
Copy link
Author

kdmcclel commented Jul 12, 2016

@n1k0

Ah sorry its been a while since I looked at this.

So you define your custom field in the uiSchema and then link it to a react class in the fields prop. The problem here is that now the form isn't dynamic. I would have to add every other field into this prop.

My current setup handles everything via api schema. There is only one edit page which uses routing data to pass the appropriate portion of the full schema into the form. Due to this I want to avoid defining all the fields at form startup. Using the widget instead was an easy solution as the setup is the same but the widgets prop doesn't override the dynamic behavior for form creation.

The other solution may be a customField prop which only links a ui:field moniker to a react class but doesn't define the entire fieldset of a form.

@kdmcclel kdmcclel closed this Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants