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

feat: Field support messageVariables #80

Merged
merged 2 commits into from
Mar 3, 2020
Merged

feat: Field support messageVariables #80

merged 2 commits into from
Mar 3, 2020

Conversation

zombieJ
Copy link
Member

@zombieJ zombieJ commented Feb 11, 2020

close #79

<Field
  name="username"
  messageVariables={{ label: '用户名' }}
  rules={[{ required, message: '${label}是被需要的' }]}
/>

@vercel
Copy link

vercel bot commented Feb 11, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/react-component/field-form/8kd49rqll
✅ Preview: https://field-form-git-tmpl.react-component.now.sh

@zombieJ
Copy link
Member Author

zombieJ commented Feb 11, 2020

@mgcrea, how do you think about this prop name?


it('messageVariables', async () => {
const wrapper = renderForm(
{ required: "You miss '${label}'!" },
Copy link
Member

@afc163 afc163 Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${} 万一这个字符串写成 ` 的格式就会很复杂。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const username = '姓名';

...

{ required: `miss \${label} ${username}` },

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${} 会被转化成字符串拼接的,应该没什么影响

Copy link
Member

@afc163 afc163 Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

写起来是不是比较难看?

Copy link
Contributor

@chenshuai2144 chenshuai2144 Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\${label} 有一些理解成本,第一眼不知道是什么东西

Copy link
Member Author

@zombieJ zombieJ Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${xxx} is used for validateMessages for static message template:

const myMessages: ValidateMessages = {
  default: '${name} 看起来怪怪的……',
  required: '你需要一个 ${name}',
  types: {
    number: '嗨,这个 ${name} 不是一个合格的 ${type}',
  },
  enum: '${name} 不在 ${enum} 中呢',
  whitespace: '${name} 不可以是空的啦',
  pattern: {
    mismatch: '${name} 并不符合格式:${pattern}',
  },
};

If user use `, it's no need to use ${xxx}.

Copy link
Member

@afc163 afc163 Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我的意思是如果这个字符串里需要用到别的变量(比如 username)改造成 ` 写法后,原有的 ${label} 写法就会和新的 ${username} 混淆在一起,需要转义。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

现在的 form 已经是支持 ${xxx} 了,这个 PR 只是拓展自定义变量的能力。

Current form is already support ${xxx}. This PR is aim to extends the ability of message variables.

@mgcrea
Copy link
Contributor

mgcrea commented Feb 11, 2020

Not sure I see the point of not simply using template strings? Is there a specific use-case?

To me it looks to be out of scope as every end user will have specific needs (eg. interfacing with a specific i18n lib, etc.) and you can't grow the API to cover all use cases.

const label = '用户名';
return <Field
  name="username"
  rules={[{ required, message: `${label}是被需要的` }]}
/>;

@afc163

This comment has been minimized.

@chenshuai2144
Copy link
Contributor

他应该只支持string,类似 模板字符串,i18 要转化为string之后再使用

@zombieJ
Copy link
Member Author

zombieJ commented Feb 12, 2020

Not sure I see the point of not simply using template strings? Is there a specific use-case?

To me it looks to be out of scope as every end user will have specific needs (eg. interfacing with a specific i18n lib, etc.) and you can't grow the API to cover all use cases.

const label = '用户名';
return <Field
  name="username"
  rules={[{ required, message: `${label}是被需要的` }]}
/>;

In antd, Field is wrapped as Form.Item. Which used like:

<Form>
  <Form.Item name="username" label="User Name">
    <Input />
  </Form.Item>
  <Form.Item name="password" label="Password">
    <Input />
  </Form.Item>
</Form>

It's useful to support label prop in validateMessages instead of name only:

<Form validateMessages={{ required: 'You need ${label}' }}>
  <Form.Item name="username" label="User Name" rules={[{ required: true }]}>
    <Input />
  </Form.Item>
  <Form.Item name="password" label="Password" rules={[{ required: true }]}>
    <Input />
  </Form.Item>
</Form>

@mgcrea
Copy link
Contributor

mgcrea commented Feb 27, 2020

It's useful to support label prop in validateMessages instead of name only:

Indeed just stumbled on this issue while I was trying to use ${label} in <Form /> validateMessages prop. But I don't really see why we would need a messageVariables on the <Form.Item /> instead of just mapping the props directly like your last example (eg. label would always point to <Form.Item /> label prop).

If we want to allow using arbitrary props we could have something like validateMessagePlaceholders={["name", "label"]} at the <Form /> but I'm not sure this would be really useful if we add label support by default (which is a pretty common use case).


Just to add that in my case I would actually need a lowercased version of the label prop. So maybe we should instead add a validateMessages API like.

(formItemProps: {name: string, label?: string) => string

That would remove the need for a messageVariables prop I think.

eg:

const validateMessages = {
  required: ({label}) => `The input ${label.toLowerCase()} is required`}

@zombieJ zombieJ mentioned this pull request Feb 28, 2020
@zombieJ
Copy link
Member Author

zombieJ commented Feb 28, 2020

@mgcrea, this prop only works in rc-field-form. Form.Item of antd will wrap this prop. Is that OK?

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.

支持自定义的模板字符串
4 participants