ant-design / pro-components

🏆 Use Ant Design like a Pro!
https://pro-components.antdigital.dev
MIT License
4.04k stars 1.29k forks source link

fix(field,table): 优化 ProTable 中未指定 placeholder 字段时的默认显示 #6180

Closed kiner-tang closed 1 year ago

kiner-tang commented 1 year ago
github-actions[bot] commented 1 year ago

🎊 PR Preview has been successfully built and deployed to https://pro-components-preview-pr-6180.surge.sh

codecov[bot] commented 1 year ago

Codecov Report

Base: 99.40% // Head: 98.90% // Decreases project coverage by -0.50% :warning:

Coverage data is based on head (fb25cb4) compared to base (4b522af). Patch coverage: 100.00% of modified lines in pull request are covered.

:exclamation: Current head fb25cb4 differs from pull request most recent head 9ba2f57. Consider uploading reports for the commit 9ba2f57 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6180 +/- ## ========================================== - Coverage 99.40% 98.90% -0.51% ========================================== Files 313 320 +7 Lines 9747 9739 -8 Branches 3273 3273 ========================================== - Hits 9689 9632 -57 - Misses 50 99 +49 Partials 8 8 ``` | [Impacted Files](https://codecov.io/gh/ant-design/pro-components/pull/6180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design) | Coverage Δ | | |---|---|---| | [packages/field/src/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvZmllbGQvc3JjL2luZGV4LnRzeA==) | `100.00% <100.00%> (ø)` | | | [packages/layout/src/style/index.ts](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvbGF5b3V0L3NyYy9zdHlsZS9pbmRleC50cw==) | `38.00% <0.00%> (-62.00%)` | :arrow_down: | | [packages/field/src/components/Slider/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvZmllbGQvc3JjL2NvbXBvbmVudHMvU2xpZGVyL2luZGV4LnRzeA==) | `81.25% <0.00%> (-18.75%)` | :arrow_down: | | [packages/provider/src/useStyle/token.ts](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvcHJvdmlkZXIvc3JjL3VzZVN0eWxlL3Rva2VuLnRz) | `90.00% <0.00%> (-10.00%)` | :arrow_down: | | [...ackages/layout/src/components/PageHeader/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvbGF5b3V0L3NyYy9jb21wb25lbnRzL1BhZ2VIZWFkZXIvaW5kZXgudHN4) | `93.42% <0.00%> (-5.27%)` | :arrow_down: | | [packages/field/src/components/DatePicker/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvZmllbGQvc3JjL2NvbXBvbmVudHMvRGF0ZVBpY2tlci9pbmRleC50c3g=) | `96.61% <0.00%> (-1.78%)` | :arrow_down: | | [...kages/table/src/components/ColumnSetting/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvdGFibGUvc3JjL2NvbXBvbmVudHMvQ29sdW1uU2V0dGluZy9pbmRleC50c3g=) | `98.86% <0.00%> (-0.58%)` | :arrow_down: | | [packages/form/src/BaseForm/BaseForm.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvZm9ybS9zcmMvQmFzZUZvcm0vQmFzZUZvcm0udHN4) | `97.77% <0.00%> (-0.02%)` | :arrow_down: | | [...kages/table/src/components/EditableTable/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvdGFibGUvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUYWJsZS9pbmRleC50c3g=) | `99.25% <0.00%> (-0.01%)` | :arrow_down: | | [packages/provider/src/index.tsx](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design#diff-cGFja2FnZXMvcHJvdmlkZXIvc3JjL2luZGV4LnRzeA==) | `99.28% <0.00%> (-0.01%)` | :arrow_down: | | ... and [27 more](https://codecov.io/gh/ant-design/pro-components/pull/6180/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ant-design)

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

chenshuai2144 commented 1 year ago

代码比较多,能简单描述下你的思路吗

kiner-tang commented 1 year ago

代码比较多,能简单描述下你的思路吗

这边的思路就是将所有是用户选择的表单组件列举出来,使用tableForm.selectPlaceholder,而其他的使用tableForm.inputPlaceholder,而针对范围选择的组件,我们使用数组,其他则直接使用相应的placeholder

chenshuai2144 commented 1 year ago

你怎么知道用户的是什么,我觉得不如简单点,不要给用户传 placeholder

kiner-tang commented 1 year ago

你怎么知道用户的是什么,我觉得不如简单点,不要给用户传 placeholder

这个只是默认的placeholder呀,用户可以自己传placeholder覆盖的呀,只有用户没传placeholder的时候才会使用这个逻辑

image
kiner-tang commented 1 year ago

按照目前线上的逻辑,如果用户不传placeholder的话,有些类型是会显示异常的,比如树形选择框没显示placeholder image 以及范围选择框异常 image

chenshuai2144 commented 1 year ago

我们自己的自己处理吧,用了renderForm 就不要给 placeholder props 了。

树形选择框没显示placeholder,这个是个bug,应该修复一下

chenshuai2144 commented 1 year ago

这个issue的问题就是应该给 placeholder 数组的时候给了一个 placeholder。导致显示异常。还不如不显示

kiner-tang commented 1 year ago

这个issue的问题就是应该给 placeholder 数组的时候给了一个 placeholder。导致显示异常。还不如不显示

不是吧,我理解是这边显示的是一个范围选择器,应该传一个placeholder数组过去,但issue给的示例没有传自定义的placeholder,那么就应该采用默认的placeholder,但我们以前的逻辑,默认的placeholder不是数组,并且固定成了tableForm.inputPlaceholder,所以导致显示异常了。 image

chenshuai2144 commented 1 year ago

我们没法知道用户 renderFormItem 到底给的是什么的,可能是个input 可能是的daterange

kiner-tang commented 1 year ago

我们没法知道用户 renderFormItem 到底给的是什么的,可能是个input 可能是的daterange

image

我们不是应该根据用户显示指定的valueType判断类型吗?而且我们这里设置的也是默认placeholder,如果用户一定要在renderFormItem里传一个与valueType相悖的组件,那么用户自己自定义placeholder是不是也可以?

wyhooo commented 1 year ago

我们自己的自己处理吧,用了renderForm 就不要给 placeholder props 了。

树形选择框没显示placeholder,这个是个bug,应该修复一下

我也觉得应该如果传了 renderFormItem 的话应该交给用户自行维护 placeholder,如果用户没指定,则用组件的默认行为而不是统一注入。这里有个类似的 issue

kiner-tang commented 1 year ago

我们没法知道用户 renderFormItem 到底给的是什么的,可能是个input 可能是的daterange

那当用户传入renderFormItem时,我们不指定默认的placeholder,如果没传入的话,我们就按照这个PR的方式处理一下默认的placeholder如何? @chenshuai2144

lgtm-com[bot] commented 1 year ago

This pull request introduces 2 alerts when merging d8d2512a213f2c31c922a3101d99665502966c1c into 4b522af533473de0e7ad6eb4a0296acc007f10eb - view on LGTM.com

new alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.

chenshuai2144 commented 1 year ago

看起来已经好了,能合并了吗

kiner-tang commented 1 year ago

看起来已经好了,能合并了吗

恩恩,已经按照要求,传入renderFormItem时不指定placeholder,并且为各个原子组件,没有默认placeholder的,都添加上了默认的placeholder

kiner-tang commented 1 year ago

看起来已经好了,能合并了吗

恩恩,已经按照要求,传入renderFormItem时不指定placeholder,并且为各个原子组件,没有默认placeholder的,都添加上了默认的placeholder

先稍等一下,我再最后检查一下有没有错漏的地方

kiner-tang commented 1 year ago

@chenshuai2144 应该没问题了