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(antd): FormItem adds more attribute configuration #3727

Merged
merged 4 commits into from
Feb 27, 2023

Conversation

Alex-xd
Copy link

@Alex-xd Alex-xd commented Feb 17, 2023

Before submitting a pull request, please make sure the following is done...

  • Ensure the pull request title and commit message follow the Commit Specific in English.
  • Fork the repo and create your branch from master or formily_next.
  • If you've added code that should be tested, add tests!
  • If you've changed APIs, update the documentation.
  • Ensure the test suite passes (npm test).
  • Make sure your code lints (npm run lint) - we've done our best to make sure these rules match our internal linting guidelines.

Please do not delete the above content


What have you changed?

  1. FormItem component adds requiredMark attribute configuration, follow Antd .
  2. Added enableOutlineFeedback property configuration for FormItem component. When there is a sub-form in the custom component, the current field verification status will be applied to the sub-form in all range, which is an interactive bug. This kind of scenario can be solved by disabling enableOutlineFeedback.

image

@CLAassistant
Copy link

CLAassistant commented Feb 17, 2023

CLA assistant check
All committers have signed the CLA.

@Alex-xd Alex-xd changed the title feat<antd>: FormItem adds requiredMark and enableOutlineFeedback attribute configuration feat(antd): FormItem adds requiredMark and enableOutlineFeedback attribute configuration Feb 17, 2023
@Alex-xd Alex-xd changed the title feat(antd): FormItem adds requiredMark and enableOutlineFeedback attribute configuration feat(antd): FormItem adds more attribute configuration Feb 17, 2023
@janryWang
Copy link
Collaborator

asterisk 就是 requiredMark,为啥要新增这个属性

@Alex-xd
Copy link
Author

Alex-xd commented Feb 20, 2023

asterisk 就是 requiredMark,为啥要新增这个属性

因为我看asterisk是field.required的派生属性,做的是【必填态/选填态切换】,而requiredMark是基于前者,做【必填态/选填态样式切换】这件事,所以我理解不能混淆asterisk和requiredMark。@janryWang

@janryWang
Copy link
Collaborator

asterisk

asterisk 就是用来控制星号是否展示的,没明白 requiredMark 的作用有啥特殊的呢

@Alex-xd
Copy link
Author

Alex-xd commented Feb 20, 2023

asterisk 就是用来控制星号是否展示的,没明白 requiredMark 的作用有啥特殊的呢

asterisk 控制星号是否展示;
requiredMark 控制星号以什么形态展示;

可以枚举出以下5个场景你看下,可以帮助理解上面的定义:

场景 requiredMark asterisk label样式
1 true true image
2 true false image
3 "optional" true image
4 "optional" false image
5 false true|false image

场景1、2 也就是默认情况,本次PR扩展出场景345的支持

另外 requiredMark 是单纯的全局性 label 样式控制(已提交新的commit,将其提升到FormLayout),其值与 field 的状态无关。而 asterisk 会通过外层的field.required派生

@janryWang

@janryWang
Copy link
Collaborator

这里直接把中文文案写死在代码里了,不能这么搞的,其实你的需求就是希望支持 asterisk 设置 false,但是 required 还生效吧

@Alex-xd
Copy link
Author

Alex-xd commented Feb 21, 2023

这里直接把中文文案写死在代码里了,不能这么搞的

这块多语言怎么弄比较好?有没有可以参考的其他用例? @janryWang

@Alex-xd
Copy link
Author

Alex-xd commented Feb 21, 2023

其实你的需求就是希望支持 asterisk 设置 false,但是 required 还生效吧

是的。准确的讲,我的需求是支持场景3、4的样式

所以补充一下你说的:asterisk 设置 false,但是 required 还生效,并且非必填时显示“(可选)”文案在label后面

image

我们设计师按照Antd的组件为基础去出设计规范,目前在表单项上面,统一用的是这套样式。可以先看一下Antd的FormItem 关于requiredMark的文档,所以 @formily/antd 在这块还没有和antd组件设计规范对齐。

@janryWang
Copy link
Collaborator

建议直接自定义一个 FormItem 吧

@Alex-xd
Copy link
Author

Alex-xd commented Feb 22, 2023

建议直接自定义一个 FormItem 吧

。。。所以多语言目前无解么?

现在我们好几个大项目里就是拷贝了 @formily/antd/src/form-item 的源码,代码就改了 requiredMark 那一点东西,99%的代码都是冗余的,所以才有了这个PR 🙄 @janryWang

@janryWang
Copy link
Collaborator

建议直接自定义一个 FormItem 吧

。。。所以多语言目前无解么?

现在我们好几个大项目里就是拷贝了 @formily/antd/src/form-item 的源码,代码就改了 requiredMark 那一点东西,99%的代码都是冗余的,所以才有了这个PR 🙄 @janryWang

其实还好的,因为 FormItem 组件本身就是用来做 UI 渲染的,没有啥核心逻辑,UI 渲染,那就跟当前业务团队的设计规范息息相关了,跟设计规范实在有冲突,那就自定义

@Alex-xd
Copy link
Author

Alex-xd commented Feb 22, 2023

建议直接自定义一个 FormItem 吧

。。。所以多语言目前无解么?

现在我们好几个大项目里就是拷贝了 @formily/antd/src/form-item 的源码,代码就改了 requiredMark 那一点东西,99%的代码都是冗余的,所以才有了这个PR 🙄 @janryWang

其实还好的,因为 FormItem 组件本身就是用来做 UI 渲染的,没有啥核心逻辑,UI 渲染,那就跟当前业务团队的设计规范息息相关了,跟设计规范实在有冲突,那就自定义

但这是 Antd 的原生样式,为什么不能官方支持去对齐呢?如果 Antd 之外的自定义样式我们自己去拉一个组件改我认为还可以接受

@janryWang
Copy link
Collaborator

好吧,看来是我之前实现的时候漏掉了(也有可能是4.x后续迭代新增的属性),不过我对这个 PR 还有两个问题:

  1. Antd 的 requiredMark 是在 Form 组件上的,从 PR 上看,我一直以为是 FormItem 的属性
  2. 多语言最好是继承 antd 的 key

@Alex-xd
Copy link
Author

Alex-xd commented Feb 22, 2023

好吧,看来是我之前实现的时候漏掉了(也有可能是4.x后续迭代新增的属性),不过我对这个 PR 还有两个问题:

  1. Antd 的 requiredMark 是在 Form 组件上的,从 PR 上看,我一直以为是 FormItem 的属性
  2. 多语言最好是继承 antd 的 key
  1. 在这个commit里已将 requiredMark 放到 FormLayout 上了哈
  2. 多语言问题已解决,详见commit。 感谢~ @janryWang

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Base: 96.57% // Head: 96.61% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (e51dfda) compared to base (5f95cdf).
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff                @@
##           formily_next    #3727      +/-   ##
================================================
+ Coverage         96.57%   96.61%   +0.03%     
================================================
  Files               152      152              
  Lines              6603     6668      +65     
  Branches           1835     1854      +19     
================================================
+ Hits               6377     6442      +65     
- Misses              199      226      +27     
+ Partials             27        0      -27     
Impacted Files Coverage Δ
packages/path/src/index.ts 68.69% <0.00%> (ø)
packages/path/src/parser.ts 92.42% <0.00%> (ø)
packages/path/src/shared.ts 50.87% <0.00%> (ø)
packages/vue/src/shared/h.ts 86.48% <0.00%> (ø)
packages/path/src/tokenizer.ts 91.74% <0.00%> (ø)
packages/core/src/models/Field.ts 99.07% <0.00%> (ø)
packages/reactive/src/internals.ts 95.12% <0.00%> (ø)
packages/core/src/models/BaseField.ts 100.00% <0.00%> (ø)
packages/core/src/shared/internals.ts 100.00% <0.00%> (ø)
packages/json-schema/src/transformer.ts 100.00% <0.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@janryWang janryWang merged commit 71be0a5 into alibaba:formily_next Feb 27, 2023
@janryWang
Copy link
Collaborator

需要再改改,文件路径不能直接依赖 es目录的,会重复打包

@Alex-xd
Copy link
Author

Alex-xd commented Feb 27, 2023

需要再改改,文件路径不能直接依赖 es目录的,会重复打包

#3742 @janryWang

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.

3 participants