-
-
Notifications
You must be signed in to change notification settings - Fork 363
feat(Select): redesign IsClearable function #5626
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
Conversation
Reviewer's Guide by SourceryThis pull request modifies the Select component to correctly handle nullable value types when the Sequence diagram for OnClearValue when ValueCanBeNull is truesequenceDiagram
participant SelectComponent
SelectComponent->SelectComponent: OnClearValue()
SelectComponent->SelectComponent: ValueCanBeNull()
activate SelectComponent
SelectComponent->SelectComponent: Set SelectedItem to null
SelectComponent->SelectComponent: Set CurrentValueAsString to string.Empty
SelectComponent->SelectComponent: Set Value to default(TValue)
SelectComponent->SelectComponent: Invoke ValueChanged(default(TValue))
deactivate SelectComponent
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for your PR, @trycatchfinnally. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @trycatchfinnally - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider extracting the logic within
OnClearValueinto smaller, well-named methods to improve readability. - The
ValueCanBeNullmethod could be a useful extension method onType.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@dotnet-policy-service agree |
|
@microsoft-github-policy-service agree |
|
大佬们,提个建议,既然增加了这一项,那也可以再加一个配置属性,AutoAddNullItem , 启用时把Null项加到列表里 |
|
@sourcery-ai review |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5626 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 653 653
Lines 29344 29342 -2
Branches 4176 4180 +4
=========================================
- Hits 29344 29342 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Issue
fixes #5629
对于引用类型,或可空值类型,清除选中项应将选中值设置为null,而不应是取默认第一项的值,目前,主流ui框架均支持直接清除,建议增加此功能
此部分代码涉及到select组件 GetSelectedRow和OnClearValue方法
Summary by Sourcery
This pull request includes several changes to the
Selectcomponent and its related files in theBootstrapBlazorproject. The most important changes involve enhancements to theSelectcomponent's clearable functionality, updates to theSelects.razorsample, and improvements to the unit tests.Enhancements to
Selectcomponent:GetClearablemethod to ensureIsClearableonly works if the value is nullable. (src/BootstrapBlazor/Components/Select/Select.razor.cs)IsNullableto check if the value type is nullable. (src/BootstrapBlazor/Components/Select/Select.razor.cs)Updates to
Selects.razorsample:IsClearabledemo block with new examples and descriptions. (src/BootstrapBlazor.Server/Components/Samples/Selects.razor)MockModelclass to support nullable properties in theIsClearabledemo. (src/BootstrapBlazor.Server/Components/Samples/Selects.razor.cs)IsClearabledemo description in both English and Chinese. (src/BootstrapBlazor.Server/Locales/en-US.json,src/BootstrapBlazor.Server/Locales/zh-CN.json) [1] [2]Improvements to unit tests:
IsClearable_Oktest to check for nullable and non-nullable types. (test/UnitTest/Components/SelectTest.cs) (Ff1c59c4L154R175)IsNullable_Oktest to verify theIsNullablemethod for different data types. (test/UnitTest/Components/SelectTest.cs)test/UnitTest/Components/SelectTest.cs) [1] [2]