-
-
Notifications
You must be signed in to change notification settings - Fork 365
feat(BootstrapInputNumber): add OnBlurAsync parameter #4519
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
WalkthroughThe changes introduce a new public parameter Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4519 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 631 631
Lines 27058 27063 +5
Branches 3925 3926 +1
=========================================
+ Hits 27058 27063 +5 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
test/UnitTest/Components/InputNumberTest.cs (2)
141-156: LGTM! The test implementation looks good.The test effectively verifies the new
OnBlurAsyncfunctionality:
- Follows the AAA pattern
- Uses async/await correctly
- Has clear assertions
- Maintains consistency with existing test patterns
Consider adding a few more test cases:
- Verify the callback parameter value
- Test when callback is null
- Test error scenarios
Example:
[Theory] [InlineData(42)] [InlineData(-1)] public async Task OnBlurAsync_PassesCorrectValue(int value) { var actualValue = 0; var cut = Context.RenderComponent<BootstrapInputNumber<int>>(builder => { builder.Add(a => a.Value, value); builder.Add(a => a.OnBlurAsync, v => { actualValue = v; return Task.CompletedTask; }); }); await cut.InvokeAsync(() => cut.Find("input").Blur()); Assert.Equal(value, actualValue); }
141-141: Add XML documentation for the new test method.Consider adding XML documentation to describe the test's purpose and expectations, maintaining consistency with the codebase's documentation standards.
Example:
/// <summary> /// Verifies that the OnBlurAsync callback is properly invoked when the input loses focus /// </summary> /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns> [Fact] public async Task OnBlurAsync_Ok()src/BootstrapBlazor/Components/InputNumber/BootstrapInputNumber.razor.cs (1)
86-91: Improve XML documentation for OnBlurAsync parameter.The documentation should be in English and provide more details about the parameter's purpose and usage.
/// <summary> - /// 获得/设置 失去焦点回调方法 默认 null + /// Gets or sets the async callback that is invoked when the input loses focus. + /// The callback receives the current value as a parameter. + /// Default value is null. /// </summary> [Parameter] public Func<TValue, Task>? OnBlurAsync { get; set; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/BootstrapBlazor/Components/InputNumber/BootstrapInputNumber.razor.cs (2 hunks)
- test/UnitTest/Components/InputNumberTest.cs (1 hunks)
🔇 Additional comments (1)
src/BootstrapBlazor/Components/InputNumber/BootstrapInputNumber.razor.cs (1)
86-91: Verify similar event handler patterns in the codebase.The implementation looks good and meets the PR objectives. Let's verify the consistency with other async event handlers in the codebase.
Also applies to: 290-294
✅ Verification successful
Let me generate new shell scripts to better verify the patterns in the codebase.
The previous search had some limitations:
- The regex for async handlers was too strict
- The error handling pattern search had syntax issues
- We should look at the actual implementations, not just file names
The new search will:
- Look for OnBlur async handlers with surrounding context
- Find other async event handlers for pattern comparison
- Fix the regex for error handling patterns
Based on the search results, I can now provide a final response:
OnBlurAsync implementation is consistent with codebase patterns
The implementation of
OnBlurAsyncinBootstrapInputNumber.razor.csfollows the established patterns in the codebase:
- It matches the async event handler pattern used in other components like
TextareaandAutoComplete- The naming convention (OnXxxAsync) is consistent with other async callbacks
- The parameter type (Func<TValue, Task>) follows the same pattern used across the codebase
- The nullability (?) is consistent with other event handlers
The implementation is well-integrated and maintains consistency with the codebase's event handling patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for similar async event handler patterns in the codebase # to ensure consistency in implementation. # Look for other async event handlers echo "Checking for similar async event handler patterns..." rg -l "Func<\w+,\s*Task>.*OnBlur" --type cs # Look for error handling patterns in event handlers echo "Checking error handling patterns in event handlers..." rg "try\s*{.*?await.*?}\s*catch.*?{.*?}" --type cs -ULength of output: 531
Script:
#!/bin/bash # Search for similar async event handler patterns with better regex echo "Checking for OnBlur async handlers..." rg "Func<.*?,\s*Task>.*?OnBlur" --type cs -A 2 echo "Checking for other async event handlers for consistency..." rg "Func<.*?,\s*Task>.*?On\w+" --type cs -A 2 echo "Checking error handling in event handlers..." rg "try.*?\{.*?await.*?\}" --type cs --multilineLength of output: 36432
add OnBlurAsync parameter
Summary of the changes (Less than 80 chars)
简单描述你更改了什么, 不超过80个字符;如果有关联 Issue 请在下方填写相关编号
Description
fixes #4294
Regression?
[If yes, specify the version the behavior has regressed from]
[是否影响老版本]
Risk
[Justify the selection above]
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by CodeRabbit
New Features
OnBlurAsynccallback for theBootstrapInputNumbercomponent, allowing users to trigger actions when the input loses focus.Tests
OnBlurAsyncevent in theInputNumbercomponent.