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

fix: Form.Item noStyle support validation status #44576

Merged
merged 8 commits into from Sep 4, 2023
Merged

fix: Form.Item noStyle support validation status #44576

merged 8 commits into from Sep 4, 2023

Conversation

zombieJ
Copy link
Member

@zombieJ zombieJ commented Sep 1, 2023

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

resolve #42603

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Fix Form.Item with noStyle not consume useStatus.
🇨🇳 Chinese 修复 Form.Item 配置 noStyle 时,被绑定的元素无法消费 useStatus 的问题。

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

🚀 Summary

🤖 Generated by Copilot at fd00e75

Added a new feature to support noStyle Form.Item status inheritance from the parent Form.Item. Refactored the validation logic and feedback icon generation for form items by using helper functions and a new StatusProvider component. Improved the code readability and clarity by reordering imports, adding comments and displayNames. Updated the documentation and tests accordingly.

🔍 Walkthrough

🤖 Generated by Copilot at fd00e75

  • Simplify and refactor the logic of getting the validate status for Form.Item components by using the getStatus function from ./components/form/util.ts (link, link)
  • Introduce the StatusProvider component in ./components/form/FormItem/StatusProvider.tsx to create the feedback icon and the status context based on the props and the meta (link, link, link)
  • Enable the noStyle Form.Item to inherit the parent status if the self status is not configured by using the StatusProvider component and the FormItemInputContext (link, link, link, link)
  • Add a displayName to the FormItemInputContext in ./components/form/context.tsx to improve the debugging experience (link)
  • Fix a missing dependency error by adding the React import to ./components/form/FormItem/index.tsx (link)
  • Update the assertions in ./components/form/__tests__/index.test.tsx to use the toHaveClass matcher and check for the new class names that indicate the status and the form item input state of the select components (link)
  • Add comments to ./components/form/__tests__/index.test.tsx to explain the expected behavior of the noStyle Form.Items with different status configurations (link, link, link)
  • Reorder the imports in ./components/form/FormItem/ItemHolder.tsx, ./components/form/FormItem/index.tsx, ./components/form/context.tsx and ./components/form/__tests__/index.test.tsx to group React-related imports together and separate them from other imports (link, link, link, link)

@stackblitz
Copy link

stackblitz bot commented Sep 1, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

size-limit report 📦

Path Size
./dist/antd.min.js 389.85 KB (+197 B 🔺)
./dist/antd-with-locales.min.js 448.69 KB (+169 B 🔺)

@argos-ci
Copy link

argos-ci bot commented Sep 1, 2023

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 1 change Sep 2, 2023, 9:14 AM

Comment on lines 1419 to 1424
{/* should follow parent status */}
<Form.Item validateStatus="error">
<Form.Item noStyle>
<Select className="custom-select-b" />
</Form.Item>
</Form.Item>
Copy link
Member

Choose a reason for hiding this comment

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

这里其实逻辑有点矛盾了。

从上一个用例可以看出 noStyle 的作用是不影响 status 样式,那么这个用例应该有同样的作用,这个 Item 的无状态 应该保留,而不是继承了上一个 Item 的 Error 样式。

这样看应该新增一个区别于 noStyle 的属性用来消除自身的 status 状态,而 noStyle 本身不应该对 status 有影响

Copy link
Member

Choose a reason for hiding this comment

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

这个用例的行为应该是对齐了 4.18.x,也就是没有做 FormInput 改造的版本,如果希望不 break 用户的话可能应该保证这个用例不变。

Copy link
Member Author

Choose a reason for hiding this comment

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

实验了一下,这个设计应该就是为了让上面的状态传下去。默认情况下,上面的字段是不绑定 name 的,所以也不会有状态。
下面又加了一个 test case,特意测了一下下传上的情况。也是符合预期的。

@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (9a2ef0e) 100.00% compared to head (086b640) 100.00%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #44576   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          665       666    +1     
  Lines        11280     11294   +14     
  Branches      3049      3053    +4     
=========================================
+ Hits         11280     11294   +14     
Files Changed Coverage Δ
components/form/FormItem/ItemHolder.tsx 100.00% <100.00%> (ø)
components/form/FormItem/StatusProvider.tsx 100.00% <100.00%> (ø)
components/form/FormItem/index.tsx 100.00% <100.00%> (ø)
components/form/context.tsx 100.00% <100.00%> (ø)
components/form/util.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@afc163
Copy link
Member

afc163 commented Sep 1, 2023

测试掉了

@@ -133,7 +133,7 @@ const validateMessages = {
| messageVariables | 默认验证字段的信息 | Record&lt;string, string> | - | 4.7.0 |
| name | 字段名,支持数组 | [NamePath](#namepath) | - | |
| normalize | 组件获取值后进行转换,再放入 Form 中。不支持异步 | (value, prevValue, prevValues) => any | - | |
| noStyle | 为 `true` 时不带样式,作为纯字段控件使用 | boolean | false | |
| noStyle | 为 `true` 时不带样式,作为纯字段控件使用。当自身没有 `validateStatus` 而父元素存在有 `validateStatus` 的 Form.Item 会继承父元素的 `validateStatus` | boolean | false | |
Copy link
Member

Choose a reason for hiding this comment

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

这句话出现了三次 validateStatus,好绕口。。

Copy link
Member 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.

不如直接加个 codesandbox 解释。

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| noStyle |`true` 时不带样式,作为纯字段控件使用。当自身没有 `validateStatus` 而父元素存在有 `validateStatus` 的 Form.Item 会继承父元素的 `validateStatus` | boolean | false | |
| noStyle |`true` 时不带样式,作为纯字段控件使用。会继承父组件 `validateStatus`[例子](codesandbox link) | boolean | false | |

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.

Form.Item.useStatus does not work for custom inputs wrapped in Form.Item noStyle=true
5 participants