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

Fix #284: Introduce a field template API. #304

Merged
merged 10 commits into from
Aug 19, 2016
Merged

Fix #284: Introduce a field template API. #304

merged 10 commits into from
Aug 19, 2016

Conversation

n1k0
Copy link
Collaborator

@n1k0 n1k0 commented Aug 18, 2016

Refs #284, WiP. Early feedback appreciated :)

Field template

To take control over the inner organization of each field (each form row), you can define a field template for your form.

A field template is basically a React stateless component being passed field-related props so you can structure your form row as you like:

function CustomFieldTemplate(props) {
  const {id, classNames, label, help, required, description, errors, children} = props;
  return (
    <div className={classNames}>
      <label htmlFor={id}>{label}{required ? "*" : null}</label>
      {description}
      {children}
      {errors}
      {help}
    </div>
  );
}

render((
  <Form schema={schema}
        FieldTemplate={CustomFieldTemplate} />,
), document.getElementById("app"));

The following props are passed to a custom field template component:

  • id: The id of the field in the hierarchy. You can use it to render a label targetting the wrapped widget;
  • classNames: A string containing the base bootstrap CSS classes merged with any custom ones defined in your uiSchema;
  • label: The computed label for this field, as a string;
  • description: A component instance rendering the field description, if any defined (this will use any custom DescriptionField defined);
  • children: The field or widget component instance for this field row;
  • errors: A component instance listing any encountered errors for this field;
  • help: A component instance rendering any ui:help uiSchema directive defined;
  • hidden: A boolean value stating if the field should be hidden;
  • required: A boolean value stating if the field is required;
  • readonly: A boolean value stating if the field is read-only;
  • displayLabel: A boolean value stating if the label should be rendered or not. This is useful for nested fields in arrays where you don't want to clutter the UI.

Note: you can only define a single field template for a form. If you need many, it's probably time to look for custom fields instead.

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 18, 2016

I'd really appreciate feedback from @mplis-jetsetter @nuclearspike @Natim @magopian @amarnus @MoOx or basically anybody interested in this feature.

@MoOx
Copy link
Contributor

MoOx commented Aug 18, 2016

lgtm. This is really a nice addition.

It would be nice to have "style" in addition to "classNames", but this comment probably apply for the entire project :D (Nice when you write CSS in JS - which is the future, you already know that /trollOrNotThatSTheQuestion ^^)

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 18, 2016

tbh I precisely asked to gather this sort of feedback :) we can definitely add more props, I was wondering about passing the schema, uiSchema and friends, but I fear that would be opening the door to a lot of possible abuse ^^

@MoOx
Copy link
Contributor

MoOx commented Aug 19, 2016

I was wondering about passing the schema, uiSchema and friends, but I fear that would be opening the door to a lot of possible abuse

With great powers blah blah blah. If that can offer flexibility, maybe that's a good idea?

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 19, 2016

I don't know, I like the idea of having a template not dealing with any kind of business logic; I feel like people will see this as a way to implement logic in there while they should be doing this in custom field/widget comps.

Sure I could document this heavily with big warnings and so on but in my experience:

  • people don't read them
  • even if they read them, they'll keep doing the wrong thing
  • and after that they'll file issues
  • and that usually tends to burn me out

That may just be me being paranoid here though :p

@MoOx
Copy link
Contributor

MoOx commented Aug 19, 2016

That may just be me being paranoid here though :p

No, you are concerned by the project you are shipping. Totally legit.

Just bikeshedding: if people want to add logic here, it's because the "U/DX" won't be good enough so they will use that because it's seems more simple for them. This might mean the "normal" way to do is less accessible or more complicated :)

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 19, 2016

I can always land this limited at first and we can add more props in the future if anyone comes with good legit use cases for them.

@MoOx
Copy link
Contributor

MoOx commented Aug 19, 2016

Definitely!

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 19, 2016

Okay people, I'm landing this.

@n1k0 n1k0 merged commit a83d016 into master Aug 19, 2016
@n1k0 n1k0 deleted the 284-field-template branch August 19, 2016 10:10
n1k0 added a commit that referenced this pull request Aug 19, 2016
- Introduce a field template API. (#304)
@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 19, 2016

Released in v0.39.0.

@n1k0
Copy link
Collaborator Author

n1k0 commented Aug 19, 2016

I can always land this limited at first and we can add more props in the future if anyone comes with good legit use cases for them.

It didn't take long to identify a first good use case :p #196 (comment)

@nuclearspike
Copy link

nuclearspike commented Aug 29, 2016

Finally got a chance to take this for a spin, and definitely appreciate this. I use it to do automatic title prettification so I can leave out a lot of titles now. "first_name" => "First Name" or in some strange situations where I want no titles/labels (uses placeholders only), no indication that it's required but I still want the required behavior (popup, no posting until it's filled in).

Additionally (maybe best as a new ticket) it would be nice to be able to override the error field to handle that however I want. One thing I want to do is to just do a find and replace on the word "property" in the error messages. No user should ever see that (I realize that's the json validator library and not this component's code).

@matias-sandell
Copy link

matias-sandell commented Sep 13, 2016

Excellent work! Now I can remove the labeling from the field template and have Material-UI render the title/label.

Unfortunately, the field template only gets me half way since objects like this;

"watchType": {
    "type": "object",
    "properties": {
        "id": {
            "title": "Watch Type",
            "type": "number",
            "enum": [1, 2, 3],
            "enumNames": ["Apple", "Big Ben", "Cuckoo"]
        }
    }
},

...will be wrapped in a <fieldset> with the <legend> set to watchType, like this;
screen shot 2016-09-13 at 18 51 20
...so what I would need is either

  1. A way to turn of the automatic labelling with something like autolabels: false which only uses the title from the json-schema if it's available without falling back to the property name
  2. Access to a fieldset template, like @nuclearspike suggested in #284
  3. Some very hard coded display: none CSS rules.

Or have I missed the perfect solution behind door number four?

@prevuelta
Copy link

Quick question, I'm guessing the ArrayFieldTemplate (or FieldTemplate) would need to be a full component to allow collapsing? eg. collapsed state & toggle method.. At the moment I can't pass in a component class or object, it has to be a stateless functional component.. or am I missing something?

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 20, 2017

At the moment I can't pass in a component class or object, it has to be a stateless functional component

Why can't you pass a stateful component? What's the error?

@prevuelta
Copy link

prevuelta commented Mar 20, 2017

If I pass a class I get TypeError: Cannot set property 'props' of undefined on this line

// Check if a custom render function was passed in
var renderFunction = ArrayFieldTemplate || DefaultNormalArrayFieldTemplate;
return renderFunction(arrayProps);

It's expecting a render function but gets the class constructor which has no this in that context

    function ArrayFieldTemplate(props) {
        ____Class0.call(this,props);
    }

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 21, 2017

That's indeed an unfortunate limitation #519 is gonna fix.

Edit: released in v0.44.0.

@prevuelta
Copy link

Cheers :)

@EvertLagerberg
Copy link

EvertLagerberg commented Jul 6, 2017

I am trying to figuring out how to make a part of my schema collapsed from the start as to improve UX on a large form. According to #598, this merge was merged for my use case. However, I cannot figure out how I am supposed to use the field template API to make a part of the form collapsible. Can anyone point me in the right direction?

@janpauldahlke
Copy link

janpauldahlke commented Aug 23, 2017

Will use this to bump. Or can someone be so kind to point me to an example, where only specific parts of the formschema are collapsible?

@loganvolkers loganvolkers mentioned this pull request Aug 21, 2018
2 tasks
@MatinF
Copy link

MatinF commented Aug 21, 2019

Hi again,

I've tried to achieve the removal of the "*" for required fields using the Field Template, but I'm finding it to be rather difficult to achieve without breaking something else.

As an example, I'm currently also using an ArrayFieldTemplate similar to the expanded example in the docs. But when I use a FieldTemplate in conjunction, it seems like my ArrayFieldTemplate gets overruled.

What I would like is really simple: Not having the "*" next to any required fields. It seems like a very clean addition to the source code and something many would like. I realize that it's possible to achieve via custom fields, but for more complex forms, this quickly gets very difficult to manage.

Any thoughts on potentially making this possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants