Kozea / formol

An opinionated react form framework.
MIT License
188 stars 17 forks source link

Material UI #53

Open gerwinbrunner opened 5 years ago

gerwinbrunner commented 5 years ago

I really like your approach. Beeing a big fan of Autoform this looks very promising.

Do you plan to add an abstraction for base components? So that we could use different, already styled components form UI frameworks... I.E. Material UI?

Could I be of any help there?

paradoxxxzero commented 5 years ago

There is already the types abstraction that lets you add new types and replace the default fields using the types Formol attribute, so you can already implement your Material UI fields.

These fields could even be integrated (with a PR) in Formol with an aliased type: mdtext, mdselect, ...

I don't think making default fields composable would be great since there's very few code in the basic fields and for more complicated fields you will need to write specific compatibility code anyway.

Gnilherk commented 5 years ago

Cool thanks.

I tried to do the same also with material ui, but I had problems positioning the label.

in material UI the basic TextField also includes the label, but in formol the InputField does not receive it in its props (it is handled in the Field.jsx seperately).

I tried to override Field.jsx in my project, but failed. How would I pass the label to the individual components? Or how could I create my own custom version of the Field.jsx file?

gerwinbrunner commented 5 years ago

@paradoxxxzero @Gnilherk I created a PR to be able to override Field.jsx by passing fieldComponent to the Field.

Now you just need to create your custom Field component:

import React from "react";
import { FieldBase } from "formol";
export class FieldMaterial extends FieldBase {
  render() {
    return (
      <div>Add your code here</div>
   );
  }
}

and use that in your Field call:

    <Formol
      types={{ text: InputField, select: SelectField }}
      item={inputData}
      onSubmit={onSubmit}
    >
      <Field name="scenarioId" title="scenarioId" fieldComponent= {FieldMaterial}/>
    </Formol>

Here is the PR: https://github.com/Kozea/formol/pull/57

paradoxxxzero commented 5 years ago

Thanks for the PR but wouldn't it be more simple to create a MaterialField in this case and use it like this: 

    <Formol
      types={{ text: InputField, select: SelectField }}
      item={inputData}
      onSubmit={onSubmit}
    >
      <MaterialField name="scenarioId" title="scenarioId" />
    </Formol>

As far as I can see if Formol exports its adapters (in particular the fieldPropsAdapter) it should work.

What I don't like is that it forces you to duplicate this logic:

https://github.com/Kozea/formol/blob/master/src/Field.jsx#L10-L54

It may be better to add a new indirection between Field and TypeField that simply override the label / field rendering, something like a FieldRenderer component that can be overriden locally on a field with a props or globally on the form.

What do you think?

gerwinbrunner commented 5 years ago

@paradoxxxzero that was actually the approach that I initially took. The issue is that most material UI components deal with the label internally and since the Field.jsx does the label wrapping for all components.

So it does not work for Material UI.

So the PR allows me to just override the render of the Field component, so all the individual fields can get rendered correctly.

What I don't like is that it forces you to duplicate this logic Actually, this can be worked around by just extending the FieldBase class. So no code duplication, only overriding of the render method.

Here is how the render of FieldMaterial.jsx looks like:

import React from "react";
import { FieldBase } from "formol";

export class FieldMaterial extends FieldBase {
  render() {
    const {
      b,
      name,
      value,
      type,
      title,
      modified,
      className,
      validator,
      readOnly,
      disabled,
      unit,
      extras,
      formatter,
      normalizer,
      unformatter,
      children,
      classNameModifiers,
      TypeField,
      i18n,
      error,
      validityErrors,
      handleChange,
      handleEntered,
      register,
      unregister,
      ...props
    } = this.props;
    console.log("  Formol ...props", props);
    console.log("  Formol this.props", this.props);
    return (
      <div>
        <TypeField
          /* mapped props */
          disabled={disabled}
          title={children || title}
          value={value}
          /* passed thru */
          elementRef={this.element}
          i18n={i18n}
          onFocus={this.handleFocus}
          onBlur={this.handleBlur}
          onChange={this.handleChange}
          {...props}
          /* props without mapping */
          name={name}
          type={type}
          readOnly={readOnly}
          /* *** */

          /* new pass thru */
          unit={unit}
          error={error}
        />
        {extras}
      </div>
    );
  }
}

And here is the actual text field for Material UI, that uses the types override of the Formol component.

import React from "react";
import { _ } from "lodash";
import TextField from "@material-ui/core/TextField";

export default class InputField extends React.PureComponent {
  static defaultProps = { type: "text" };

  render() {
    const { elementRef, className, onChange, placeholder, error, helperText, title, ...props } = this.props;
    // console.log("TextField ...props", placeholder, props);
    return (
      <TextField
        inputRef={elementRef}
        // disabled={!!disabled}
        error={!!error}
        //   fullWidth={fullWidth}
        // helperText={(error && showInlineError && errorMessage) || helperText}
        helperText={error || helperText}
        //   InputLabelProps={{
        //     shrink: label && !_.isEmpty(value),
        //     ...labelProps,
        //     ...InputLabelProps,
        //   }}
        label={title}
        //   margin={margin}
        //   onChange={(event) => disabled || onChange(event.target.value)}
        onChange={(e) => onChange(e.target.value)}
        //   required={required}
        //   select
        //   SelectProps={{
        //     displayEmpty: hasPlaceholder,
        //     inputProps: { name, id, ...inputProps },
        //     multiple: fieldType === Array || undefined,
        //     native,
        //     ...filterDOMProps(props),
        //   }}
        //   value={native && !value ? "" : value}
        //   variant={variant}
        {...props}
      >
        {placeholder}
      </TextField>
    );
  }
}

So to sum up - the Field.jsx now does add html (i.e. label) that is not needed and actually hinders UI libraries like Material UI and Polaris.

Does that make sense? Maybe you have another idea on how to approach this issue.

paradoxxxzero commented 5 years ago

Component inheritance is widely not recommended so I don't think this is a good solution. But even in this case why can't you just inherit Field?

@fieldPropsAdapter
export default class FieldMaterial extends Field {
...
}

After giving it some thoughts, a cleaner solution would be a @withLabel decorator applied on all formol fields that does what currently Field render does (wrap field in label and add error/unit divs). This way when you want to add a material field type you could just not use the decorator and implement it yourself in the material field.

What do you think?

paradoxxxzero commented 5 years ago

I made a PoC implementation. Could you give it a look? (For adding material-ui fields you just have to register a field in the types attribute, it will have no labels around but a labelChildren property see https://github.com/Kozea/formol/blob/358ebfcdf52ae71cd4f68bc5beb9721911a5eed8/src/Field.jsx#L82-L85)

(Async test snapshots are broken for some reasons)