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

How about getFieldProps takes over all the properties of a form control #1533

Closed
benjycui opened this issue Apr 28, 2016 · 12 comments
Closed
Assignees

Comments

@benjycui
Copy link
Contributor

Now, getFieldProps will take two arguments, then return some properties which will be transfered to form control.

<Select
  {...getFieldProps('select', {
    rules: [{ required: true }],
  })}
  placeholder="please select"
/>

However, not all the developers know that getFieldProps has occupied onChange(and value and maybe more event triggers). So, they may write something like this:

<Select
  {...getFieldProps('select', {
    rules: [{ required: true }],
  })}
  placeholder="please select"
  onChange={handleChange} // Oops...
/>

It seems that getFieldProps is broken now. Actually, if we want to add an event handler to Select, we have to do this:

<Select
  {...getFieldProps('select', {
    rules: [{ required: true }],
    onChange: handleChange, // Works fine.
  })}
  placeholder="please select"
/>

A developer have to know what getFieldProps will return now. It is not easy, if you are not familiar with getFieldProps. Because the return value of getFieldProps depends on the second argument.

But.. If getFieldProps take over all the properties of Select:

<Select
  {...getFieldProps('select', {
    rules: [{ required: true }],
    props: { // Just pass all the properties to the second argument
      placeholder: 'please select',
      onChange: handleChange,
    },
  })}
/>

We don't need to care about what will getFieldProps return. What does a developer need to know is: if you use getFieldProps, all the properties should be passed to option.props.

After getFieldProps takes over all the properties, it can check those properties and throw some warning:

  1. If user set option.props.defaultValue, warn: 'Please set option.initialValue, instead of ...'
  2. If user set option.props.defaultChecked, warn: 'Please set option.initialValue and options. valuePropName, instead of ...'
  3. ....
@superRaytin
Copy link
Contributor

+1 Currently, this is an error-prone point

@afc163
Copy link
Member

afc163 commented Apr 29, 2016

I think it is ugly for writing or reading. Make Form more complex.

@yiminghe
Copy link
Contributor

+1 warning is important

@afc163
Copy link
Member

afc163 commented Apr 30, 2016

{
  getFormControl(<Select {...props} />, {
    name: 'select',
    rules: [{ required: true }],
  })
}

or

// like Form.create()(XxForm);

{
  createFormControl({
    name: 'select',
    rules: [{ required: true }],
  })(<Select {...props} />)
}

Keep Component written in original way.

@afc163
Copy link
Member

afc163 commented May 1, 2016

Or this:

import { Select } from 'antd';

// // like Form.create()(XxForm);
const FormSelect = Form.createControl({
  name: 'select',
  rules: [ ... ],
})(Select);

...

<FormItem>
  <FormSelect {...props} />
</FormItem>

@betarabbit
Copy link
Contributor

👍 For the 1st step, should add warning messages for onChange and other props.

@benjycui
Copy link
Contributor Author

Prefer: #1533 (comment)

For it looks like react-dnd's connector... http://gaearon.github.io/react-dnd/docs-drag-source-connector.html#example

If you think it's OK, I will implement this feature in antd@2.0

@afc163 afc163 mentioned this issue Aug 19, 2016
19 tasks
@benjycui
Copy link
Contributor Author

最近这类问题又开始多起来了,应该是 antd 新增了用户所致。

@afc163
Copy link
Member

afc163 commented Sep 1, 2016

@benjycui Need conclusion here.

@benjycui
Copy link
Contributor Author

发现之前的回复没有了。。。。

@benjycui
Copy link
Contributor Author

getFieldDecorator will replace getFieldProps, and they have the same parameters, the difference is the return value:

+ getFieldDecorator('userName', { ... })(
    <Input placeholder="请输入账户名"
-     {...getFieldProps('userName', { ... })}
    />
+ )

And getFieldDecorator will throw warning if you set value or defaultValue(and so on) for these props will break the function of Form.create.

Also, getFieldDecorator will handle conflict between custom events and trigger/validateTrigger.

@lock
Copy link

lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants