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: add hideRequiredMark attribute types and add demo #20263

Merged
merged 20 commits into from Dec 18, 2019
Merged

feat: add hideRequiredMark attribute types and add demo #20263

merged 20 commits into from Dec 18, 2019

Conversation

txp1035
Copy link
Contributor

@txp1035 txp1035 commented Dec 15, 2019

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / document update
  • Component style update
  • TypeScript definition update
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

#20221

💡 Background and solution

hideRequiredMark属性本来是为了方便去掉*的,现在需要在去掉星号的同时其他未必选的label后加副标题。

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese hideRequiredMark属性增加选填功能

☑️ Self Check before Merge

  • Doc is updated/provided
  • Demo is updated/provided
  • TypeScript definition is updated/provided
  • Changelog is provided

@netlify
Copy link

netlify bot commented Dec 15, 2019

Deploy preview for ant-design ready!

Built with commit cdb3a06

https://deploy-preview-20263--ant-design.netlify.com

@txp1035 txp1035 mentioned this pull request Dec 15, 2019
5 tasks
@txp1035
Copy link
Contributor Author

txp1035 commented Dec 15, 2019

关于需求理解:Form中增加可选字段labelSelectiveFilling,类型为Boolean值,当有这个字段的时候,如果没有配置规则必填则为选填,如果配置了必填则不显示选填。@ycjcl868

@txp1035
Copy link
Contributor Author

txp1035 commented Dec 15, 2019

我感觉字段名应该还需要讨论下。

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 15, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cdb3a06:

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

codecov bot commented Dec 15, 2019

Codecov Report

Merging #20263 into feat-form-rule will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           feat-form-rule   #20263      +/-   ##
==================================================
- Coverage           97.56%   97.55%   -0.01%     
==================================================
  Files                 294      294              
  Lines                6845     6836       -9     
  Branches             1880     1837      -43     
==================================================
- Hits                 6678     6669       -9     
  Misses                167      167
Impacted Files Coverage Δ
components/form/Form.tsx 100% <ø> (ø) ⬆️
components/form/context.tsx 100% <ø> (ø) ⬆️
components/form/FormItemLabel.tsx 100% <100%> (ø) ⬆️
components/modal/confirm.tsx 94.33% <0%> (-0.21%) ⬇️
components/select/index.tsx 97.36% <0%> (-0.2%) ⬇️
components/avatar/index.tsx 98.38% <0%> (-0.06%) ⬇️
components/result/index.tsx 100% <0%> (ø) ⬆️
components/input/Input.tsx 100% <0%> (ø) ⬆️
components/button/button.tsx 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 007af07...cdb3a06. Read the comment docs.

@ycjcl868
Copy link
Contributor

字段名后面再改也可以

@txp1035
Copy link
Contributor Author

txp1035 commented Dec 15, 2019

待讨论问题:
1、属性名字:目前是labelSelectiveFilling,感觉太长了。又想了个labelOptional
2、demo位置,目前排在第4,order和布局一样都是3,需要调整这个值吗?
image

3、目前需求逻辑是否正确

@txp1035 txp1035 changed the title [WIP] feat: add labelSelectiveFilling feat: add labelSelectiveFilling Dec 15, 2019
@ycjcl868
Copy link
Contributor

ycjcl868 commented Dec 16, 2019

2、这个 PR #20235 ,我加了 max-width,rebase 下就可以了

@ycjcl868
Copy link
Contributor

/rebase

@@ -64,6 +67,9 @@ const FormItemLabel: React.FC<FormItemLabelProps & { required: boolean; prefixCl
title={typeof label === 'string' ? label : ''}
>
{labelChildren}
{!required && labelSelectiveFilling && (
<span className={`${labelClsBasic}-tag`}>(选填)</span>
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
Contributor

@ycjcl868 ycjcl868 Dec 16, 2019

Choose a reason for hiding this comment

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

加一个中文和英文,剩下的再补充。

PageHeader: {
back: '返回',
},

@@ -71,6 +73,7 @@ const InternalForm: React.FC<FormProps> = (props, ref) => {
name,
labelAlign,
labelCol,
labelSelectiveFilling,
Copy link
Member

Choose a reason for hiding this comment

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

这个属性是什么意思?

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

Choose a reason for hiding this comment

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

改成 showLabelOptionalText,当这一项为 true 时,hideRequiredMark 默认为 true

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

Choose a reason for hiding this comment

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

<Form hideRequiredMark />
<Form hideRequiredMark={false} />
<Form hideRequiredMark={{ showLabelOptionalText: true }} />

这样吧,不存在既要 * 又要 (可选) 的情况。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hideRequiredMark这个功能有了吗

Copy link
Member

Choose a reason for hiding this comment

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

有的。

Comment on lines 73 to 75
{typeof hideRequiredMark === 'object' &&
Object.prototype.toString.call(hideRequiredMark) === '[object Object]' &&
hideRequiredMark.showLabelOptionalText && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

typeof hideRequiredMark === 'object'这一段ts能识别,Object.prototype.toString.call(hideRequiredMark) === '[object Object]'这一段才是真正判断,有好的写法吗?

Copy link
Contributor Author

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.

typeof hideRequiredMark === 'object',够用就行。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

填个null就gg了

Copy link
Contributor Author

@txp1035 txp1035 Dec 17, 2019

Choose a reason for hiding this comment

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

Object.prototype.toString.call(hideRequiredMark) === '[object Object]' && (hideRequiredMark as object).showLabelOptionalTextts不识别或者断言?

Copy link
Member

Choose a reason for hiding this comment

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

null ts 会报错的。

Copy link
Contributor Author

@txp1035 txp1035 Dec 17, 2019

Choose a reason for hiding this comment

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

哈哈哈,呆了,前面还有类型定义 😛

Comment on lines 76 to 77
<LocaleReceiver componentName="Form" defaultLocale={defaultLocale.Form}>
{(locale: { optionalText: string }) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

defaultLocale时间处理建议统一收口掉?感觉自动识别时区来做默认值更好?关于本地类型也能通过cli自动生成收口在一个位置方便使用?

@txp1035
Copy link
Contributor Author

txp1035 commented Dec 17, 2019

请问每次测试都必须全量更新快照吗?有更快的方法吗?

@afc163
Copy link
Member

afc163 commented Dec 17, 2019

npm test -- components/form -u
npm test -- components/form/__tests__/index.test.js -u
npm test -- --watch -u


## zh-CN

隐藏必选标记
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
隐藏必选标记
隐藏必选标记,展示(可选)

@afc163
Copy link
Member

afc163 commented Dec 17, 2019

PR 的标题修改一下,覆盖率需要提升上来。

@txp1035
Copy link
Contributor Author

txp1035 commented Dec 17, 2019

标题修改一下,覆盖率需要提升上来。

image
没有变更呢

@txp1035 txp1035 changed the title feat: add labelSelectiveFilling feat: 隐藏必选标记,展示(可选)demo补充和展示(可选)功能 Dec 17, 2019
Co-Authored-By: 偏右 <afc163@gmail.com>
Co-Authored-By: 偏右 <afc163@gmail.com>
txp1035 and others added 6 commits December 17, 2019 18:04
Co-Authored-By: 偏右 <afc163@gmail.com>
Co-Authored-By: 偏右 <afc163@gmail.com>
Co-Authored-By: 偏右 <afc163@gmail.com>
Co-Authored-By: 偏右 <afc163@gmail.com>
wrapperCol?: ColProps;
}

export interface HideRequiredMarkProps {
showLabelOptionalText: boolean;
Copy link
Contributor

@ycjcl868 ycjcl868 Dec 18, 2019

Choose a reason for hiding this comment

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

labelOptionalText: string | ReactNode

Copy link
Member

Choose a reason for hiding this comment

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

optionalMark 如何?

@afc163
Copy link
Member

afc163 commented Dec 18, 2019

image

这里的信息补充一下。

@txp1035
Copy link
Contributor Author

txp1035 commented Dec 18, 2019

我想到一个点:hideRequiredMark属性只是对样式的修改吗?我今天正好在用这个,我以为为true会自动过滤掉必选规则。 @afc163 @ycjcl868
hideRequiredMark只是显示隐藏*没什么问题,现在加上可选的话,用户设定了必选规则,样式上是可选,结果是会验证,表单下方还是会有一行红字提示

@afc163
Copy link
Member

afc163 commented Dec 18, 2019

只是对样式的修改,不涉及校验规则。

(可选) 字样不能在 required 为 true 的表单项上加。

@txp1035 txp1035 changed the title feat: add hideRequiredMark attribute types and add demo [WIP]feat: add hideRequiredMark attribute types and add demo Dec 18, 2019
@txp1035
Copy link
Contributor Author

txp1035 commented Dec 18, 2019

变更点:
1、修复必选规则后展示可选文字问题
2、关于可选文字是默认值还是开放扩展问题亦或两者都需要讨论
@ycjcl868 @afc163 @zombieJ

@ycjcl868
Copy link
Contributor

ci broken

@txp1035 txp1035 changed the title [WIP]feat: add hideRequiredMark attribute types and add demo feat: add hideRequiredMark attribute types and add demo Dec 18, 2019
@@ -22,6 +25,9 @@ const FormItemLabel: React.FC<FormItemLabelProps & { required: boolean; prefixCl
required,
}) => {
if (!label) return null;
console.log(label, required);
Copy link
Contributor

Choose a reason for hiding this comment

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

console

@@ -60,4 +60,7 @@ export default {
PageHeader: {
back: 'Back',
},
Form: {
optionalText: ' (optional)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
optionalText: ' (optional)',
optionalText: '(optional)',

Copy link
Contributor Author

@txp1035 txp1035 Dec 18, 2019

Choose a reason for hiding this comment

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

这里是因为偏右故意加的空格

@ycjcl868 ycjcl868 merged commit 3f34650 into ant-design:feat-form-rule Dec 18, 2019
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.

None yet

4 participants