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

Customized rendering - implementing with material-ui #60

Closed
rosskevin opened this issue May 24, 2016 · 35 comments
Closed

Customized rendering - implementing with material-ui #60

rosskevin opened this issue May 24, 2016 · 35 comments

Comments

@rosskevin
Copy link
Contributor

I like the react-formal approach and would like to use it with material-ui. There is an existing formsy-material-ui (TextField ref) implementation but I would prefer the react-formal approach.

There is a slight impedance mismatch between react-formal's rendering of messages and that of material-ui. Material-ui has error text passed in as a prop (see the error examples):

<TextField
      hintText="Hint Text"
      errorText="This field is required"
      floatingLabelText="Floating Label Text"
    />

Whereas it appears that react-formal appends a css class to the element.

I'm new to react but a deeply experienced developer - looking for some guidance on the best way to do this.

  1. What is the best way to substitute a different element for rendering? Override getComponent?
  2. In order to propagate a validation message to the material-ui TextField as a prop, is there a way to do this with the MessageTrigger here?

If I can get some assistance, I'm happy to add a react-formal-material-ui decoration project or PR it here somewhere it makes sense.

@jquense
Copy link
Owner

jquense commented May 24, 2016

You can pass any component to the Field type and assign components to type strings via Form.addTypes. Provided the material-ui inputs follow idiomatic react input value/change patterns you should just be able to use them as is. If you want to wrap that up, or need to do control how Field injects props you can create simpler wrappers...check out how react-formal-inputs does it for react-widgets.

Let me know if I'm missing something or you get stuck. I do have a pending update that should make the way the Field injects valid state less opinionated

@rosskevin
Copy link
Contributor Author

Great thank you, I'll close this until I bump into an issue.

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

@jquense - I'm working on this now, successfully rendering the input.

I had to wrap MUI TextField as a controlled input, and register with Form the same way you did react-formal-inputs. Let's call this higher order component MyTextField just for the sake of this discussion.

e.g.

const types = {
  text: MyTextField,
  string: MyTextField,
  password: MyTextField
}
Form.addInputTypes(types)

Validation

MUI passes the validation message as the errorText prop to their TextField. What is the best way for MyTextField to listen/receive/get the validation message so I can pass it as a prop to the MUI TextField?

@rosskevin rosskevin reopened this Jun 17, 2016
@jquense
Copy link
Owner

jquense commented Jun 17, 2016

hey there,

The best way is going to be in the next release where Fields are automatically given the messages for the field as well as the value. That way you can do something like

function MyTextField({ value, errors, ...props }) {
  return <MuiTextBox value={value} errorText={formatErrors(errors)} {...props} />
}

@rosskevin
Copy link
Contributor Author

oh, great, I was diving into react-input-message and working with that. I noticed the API change/diff there vs. the latest.

How can I help get it to what you mentioned? I've got time and need to get this behind me. I can keep adding to my open PR/branch if you would like.

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

I see the commit 62051bd, do you want me to pull/use master?

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

you can try against master, I'd be interested if it's all working in practice., you'll have to rebuild though locally

@rosskevin
Copy link
Contributor Author

Tests fail on master, if that's expected, I can work on them.

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

tests are passing for me...

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

I pulled master, rm -Rf node_modules && npm install && npm run test just to be sure.

FAILED TESTS:
  Field
    form fields
      ✖ should inject onError
        Node.js (darwin; U; rv:v6.2.1)
      TypeError: Cannot read property 'should' of undefined
          at Context.<anonymous> (/Users/kross/projects/react-formal/test.js:34837:50 <- webpack:///test/field.jsx:300:24)

      ✖ should propagate onError to form
        Node.js (darwin; U; rv:v6.2.1)
      TypeError: $(...).render(...).find(...).props(...) is not a function
          at Context.<anonymous> (/Users/kross/projects/react-formal/test.js:34856:50 <- webpack:///test/field.jsx:325:24)

      ✖ should properly prefix nested errors
        Node.js (darwin; U; rv:v6.2.1)
      TypeError: onError is not a function
          at Context.<anonymous> (/Users/kross/projects/react-formal/test.js:34870:8 <- webpack:///test/field.jsx:343:6)

@rosskevin
Copy link
Contributor Author

I think I grabbed a different commit. I'll get back to you (sorry!)

@rosskevin
Copy link
Contributor Author

Ok, after second guessing myself, just confirmed tests fail. Did a clean checkout/test.

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

sorry yeah those fail, I thought I left them out.

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

I am receiving an error Object with the master code (actually my babel-6 branch). Unfortunately my input doesn't appear to be triggering the form validation, which I believe is unrelated to the master code.

For example, when using the default input, onKeyDown, I will get an error message for email. When I add the custom type, I receive onBlur, onChange in props and I setValue then propagate to these, yet I receive no field message.

Code:

// @flow
import React, {Component, Element} from 'react'
import autobind from 'autobind-decorator'
import Events from '../ui/events'
import Logger from '../util/logger'
import {TextField as MuiTextField} from 'material-ui'

declare function EventHandler (e:Event):void

type Props = {
  value:?any,
  name:string,
  errors:?any,
  onChange:EventHandler,
  onBlur:EventHandler
}

type State = {
  value:string
}

/**
 * Create a controlled MUI TextField.  By default, the MUI component doesn't respond to standardized controlled inputs.
 * @see - https://facebook.github.io/react/docs/forms.html#controlled-components
 */
class TextField extends Component<void, Props, State> {
  props:Props
  state:State = {
    value: this.props.value || ''
  };

  handleValueChange (e:Event) {
    const target = Events.target(e, HTMLInputElement)
    this.setState({
      value: target.value
    })
  }

  @autobind
  handleChange (e:Event) {
    this.handleValueChange(e)
    this.props.onChange(e)
  }

  @autobind
  handleBlur (e:Event) {
    this.handleValueChange(e)
    this.props.onBlur(e)
  }

  @autobind
  handleKeyDown (e:Event) {
    this.handleValueChange(e)
  }

  render ():Element {
    const { name, ...rest } = this.props
    return (
      <MuiTextField
        name={name}
        errorText='foo'
        {...rest}
        value={this.state.value}
        onChange={this.handleChange}
        onKeyDown={this.handleKeyDown}
        onBlur={this.handleBlur}
      />
    )
  }
}

export default TextField

Question

What do I need to do to propagate events/trigger validation?

@rosskevin
Copy link
Contributor Author

Got it! signature of props methods is onChange(string) not onChange(Event)

@rosskevin
Copy link
Contributor Author

Ok, so now that I fixed my value propagation:

The code on master receives the errors object, but it doesn't contain the message for the field. I'm expecting message Please enter your email address to be here somewhere (it's displayed on the page using <Form.Message for='email' />)

sign_in_-_mysite_com

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

it's not going to contain any messages while the field is valid,fields don't start invalid, you'd need to blur or fire onChange, or set the initial error state of the form. Form validation is lazy, meaning it validates as you interact with it.

as an aside you shouldn't be tracking the value in state, just call onChange when the value changes use the value passed in. in fact you need zero logic in your custom input aside from extracting the error text and pulling the value from the change event, just pass the props on through otherwise

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

^^^ This image is of an invalid field after I have changed it. The normal message is showing up, but no values are in the errors object passed to the field.

(also, thanks for the tips on state and logic)

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

can you post the code in a gist then so I can try it out or read through it? it's also not passing the className in so something doesn't consider it invalid

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

https://gist.github.com/rosskevin/e166b1a174f961ac61a091930dfcefa7

[debug][TextField]  errors Object {}

sign_in_-_mysite_com

(don't judge the looks! I'm just wiring up right now)

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

hmm looks ok to me.. are onBlur and onChange getting called? is the form onChange being called? when I get a moment I'll double check its not actually broken but tests are working for the relevant code.

@jquense
Copy link
Owner

jquense commented Jun 17, 2016

also can you try with a built in input and see if that works?

@rosskevin
Copy link
Contributor Author

rosskevin commented Jun 17, 2016

onBlur && onChange are getting called. Form onChange is not called.

  1. Open page
  2. delete email value
  3. click off field

TextField:

[debug][TextField]  onChange
[debug][Signin]  field onChange
[debug][TextField]  errors Object {}
[debug][TextField]  errors Object {}
[debug][TextField]  onBlur
[debug][Signin]  field onBlur
[debug][TextField]  errors Object {}

No form change.

Generic input (instrumented from Signin):

[debug][Signin]  field onChange
[debug][Signin]  field onBlur

No form change

I updated the gist with the log statements.

@jquense
Copy link
Owner

jquense commented Jun 18, 2016

I think maybe material UI inputs don't fire onChange?

@rosskevin
Copy link
Contributor Author

From the logs it shows that they are firing onChange.

@jquense
Copy link
Owner

jquense commented Jun 18, 2016

Ok sorry I read that wrong. if the form isn't firing an onChange something else is really wrong. I'll take a look tomorrow

@rosskevin
Copy link
Contributor Author

I'm sorry, noticed a code error as I ran it through flow. I incorrectly hooked up the form onChange. Here is a proper log:

  1. Open page
  2. delete email value
  3. click off field
TextField onChange
Signin form onChange
Signin field onChange
TextField errors Object {}
TextField errors Object {}
TextField onBlur
Signin field onBlur
TextField errors Object {}

form.onChange is called.

@jquense
Copy link
Owner

jquense commented Jun 18, 2016

does the value look empty on the form change? what is that one. (thanks for helping me debug)

@rosskevin
Copy link
Contributor Author

form on change value is as expected:

form onChange Object {password: "password1", email: ""} 

@jquense
Copy link
Owner

jquense commented Jun 19, 2016

ok! I had a chance to look at this, and figured it out! There is a bug. I'll push a fix soon.

@jquense
Copy link
Owner

jquense commented Jun 19, 2016

ok should be fixed. you'll need to reinstall all the dependencies as the bug was actually in react-input-message

@rosskevin
Copy link
Contributor Author

Confirmed fixed on master! (merged into PR #63). Thank you for taking time to look.

@jquense
Copy link
Owner

jquense commented Jun 19, 2016

hey are you thinking about publishing your wrappers when you get them working (like react-formal-inputs)? Others have also expressed interest in having mui wrappers and I don't have the time or experience with it to do it myself. if you were up for it I think others would be appreciative

@rosskevin
Copy link
Contributor Author

Yes, I'll seed something and start adding to it.

@rosskevin
Copy link
Contributor Author

Published: https://github.com/rosskevin/react-formal-material-ui

Of course this is early on and depends on my personal branch, but we have a place to add mui specific integration code now.

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

No branches or pull requests

2 participants