Skip to content

Custom Arrays#395

Closed
dehli wants to merge 11 commits intorjsf-team:masterfrom
dehli:master
Closed

Custom Arrays#395
dehli wants to merge 11 commits intorjsf-team:masterfrom
dehli:master

Conversation

@dehli
Copy link
Copy Markdown
Contributor

@dehli dehli commented Nov 16, 2016

Reasons for making this change

Currently, with rendering arrays, you're pretty much stuck with the way that the plus, trash, and reorder icons are rendered. With this change, you can pass:

render and renderItem to arrays through ui:options. These functions are used to construct the root array and each individual array item. The props passed to the functions contain attributes to reorder elements, add new elements, etc.

Let me know your thoughts and feedback! If you'd like any changes, let me know! I made sure that by default, the existing HTML is what's generated.

Checklist

  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

Edit: With this change, I've been able to implement drag and drop for items using react-dnd very easily. If interested in seeing this written out, let me know! 😁

@crumblix
Copy link
Copy Markdown
Collaborator

crumblix commented Nov 25, 2016

Look great! Thanks for taking the time to implement this and submit it! For what it's worth it's a thumbs up from me :)

I've tested it out in the playground and it works flawlessly for me. I love the refactor of ArrayField itself. I'd love to go one little step further and rename the existing ArrayField class to something like ArrayFieldBase and create a new ArrayField class with just the two default render methods.

It would make it really easy to create a Custom Array Control with different defaults ... just derive from ArrayFieldBase and set the default render methods and your good to go.

Just another way of creating your end result, but this time through a custom widget. It means you can override the default ArrayField easily for your entire schema.

So even if other people don't want to extend the public interface (via ui:options) the refactor itself to me means the overall patch is great, and you can still achieve the result you want via a Custom Widget.

Here is my quick and dirty proof of concept (this ArrayFieldBase.js file is basically the old ArrayField.js file, and the changes to it should be pretty obvious ... set the renders ... change the export and change ArrayField to ArrayFieldBase everywhere):

import ArrayFieldBase from './ArrayFieldBase';

class ArrayField extends ArrayFieldBase {

  constructor(props) {
    super(props);
    this.defaultRender = props => {
      return (
        <fieldset className={props.className}>
          {props.children}
          {props.canAdd ? <AddButton
                              onClick={props.onAddClick}
                              disabled={props.disabled || props.readonly}/> : null}
        </fieldset>
      );
    };
    this.defaultRenderItem =  props => {
      const btnStyle = {flex: 1, paddingLeft: 6, paddingRight: 6, fontWeight: "bold"};
      return (
        <div key={props.index} className={props.className}>
          <div className={props.hasToolbar ? "col-xs-9" : "col-xs-12"}>
            {props.children}
          </div>
          {props.hasToolbar ?
            <div className="col-xs-3 array-item-toolbox">
              <div className="btn-group" style={{display: "flex", justifyContent: "space-around"}}>
                {props.hasMoveUp || props.hasMoveDown ?
                  <IconBtn icon="arrow-up" className="array-item-move-up"
                          tabIndex="-1"
                          style={btnStyle}
                          disabled={props.disabled || props.readonly || !props.hasMoveUp}
                          onClick={props.onReorderClick(props.index, props.index - 1)}/>
                  : null}
                {props.hasMoveUp || props.hasMoveDown ?
                  <IconBtn icon="arrow-down" className="array-item-move-down"
                          tabIndex="-1"
                          style={btnStyle}
                          disabled={props.disabled || props.readonly || !props.hasMoveDown}
                          onClick={props.onReorderClick(props.index, props.index + 1)}/>
                  : null}
                {props.hasRemove ?
                  <IconBtn type="danger" icon="remove" className="array-item-remove"
                          tabIndex="-1"
                          style={btnStyle}
                          disabled={props.disabled || props.readonly}
                          onClick={props.onDropIndexClick(props.index)}/>
                  : null}
              </div>
            </div>
          : null}
        </div>
      );
    };
  }
}

export default ArrayField;

@olzraiti
Copy link
Copy Markdown
Contributor

Good work! Your pull request is a good refactor which allows customizing ArrayFields, which is probably needed by many users.

I agree that using the ui:options to pass the rendering functions is a bit awkward. It'd be best if users could describe their schemas and uiSchemas in pure JSON (JSON doesn't allow functions).

Also, if a user wants to implement a global rendering function for each ArrayField, according to your pull request he/she would have to do something like this:

jsx

import ArrayField from "react-jsonschema-form/lib/components/fields/ArrayField";

const MyArrayField = props => (
    <ArrayField 
        {...props}
        uiSchema={{
            ...props.uiSchema,
            "ui:options": {
                render: someCustomRenderFunction,
                renderItem: someCustomItemRenderFunction
                ...props.uiSchema["ui:options"]
            }
        }}
)

<Form
    fields={
        ArrayField: MyArrayField
    } />

A template function would solve this. A template function can be build later on on top of the rendering methods that you have provided, or you could implement it in this pull request.

If you are willing to polish your pull request further, you could make the ArrayField use a template function. This way there would be a unified way of providing templates functions. The render and renderItem functions would be given to Form so they'd be global across all ArrayFields:

jsx

const arrayFieldTemplate = (props) => {
    return (
        <div className={props.className}>		
            {props.formData.forEach(item => (
                <div className={props.className} key={props.index}>
                    {props.hasMoveDown &&
                        <button onClick={props.onReorderClick(props.index, props.index + 1)}>Down</button>}
                    {props.hasMoveUp &&
                        <button onClick={props.onReorderClick(props.index, props.index - 1)}>Up</button>}
                    <button onClick={props.onDropIndexClick(props.index)}>Delete</button>
</div>
))}
          <button onClick={props.onAddClick}>Plus</button>
        </div>
  );
}

<Form ArrayFieldTemplate={arrayFieldTemplate} />

There is already a template field for SchemaField so passing the template fields could also be done this way:

<Form templates={schema: SchemaTemplateField, array: ArrayTemplateField} />

This is not so important though - the point is that the template would be passed to Form and it'd be global. You could put it in the global registry object and pull it from there in ArrayField's render method.

Like I said earlier - a template field could be implemented later on and your work doesn't conflict with what I have described in #354 (though the code base would look a bit weird with both the ui:options -style and a template function built on top of that), so if you don't have time for this/I didn't explain my thoughts clearly enough/you disagree, that's OK!

@dehli
Copy link
Copy Markdown
Contributor Author

dehli commented Nov 30, 2016

Thanks @crumblix and @olzraiti for the feedback! I'm going to work on some cleanup of this today and hopefully will have the PR updated.

Edit: Sorry I haven't gotten to this! Something else has ended up taking priority at work. I'll get around to this eventually though :)

@dehli dehli closed this Jan 4, 2017
@dehli dehli mentioned this pull request Jan 4, 2017
8 tasks
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