feat(FloatingLabel): add OnBlurAsync parameter #4523
Conversation
Reviewer's Guide by SourceryThe PR moves the OnBlurAsync parameter and its implementation from BootstrapInput to BootstrapInputBase (its parent class) and adds the onblur event handler to the FloatingLabel component. This change allows the blur functionality to be shared across different input components that inherit from BootstrapInputBase. Sequence diagram for OnBlurAsync event handlingsequenceDiagram
participant User as User
participant FloatingLabel as FloatingLabel
participant BootstrapInputBase as BootstrapInputBase
User->>FloatingLabel: Focus out (blur event)
FloatingLabel->>BootstrapInputBase: Call OnBlur()
BootstrapInputBase->>BootstrapInputBase: Check OnBlurAsync
alt OnBlurAsync is not null
BootstrapInputBase->>BootstrapInputBase: Execute OnBlurAsync(Value)
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request involves significant changes to the 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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/BootstrapBlazor/Components/Input/BootstrapInputBase.cs (1)
87-91: Enhance XML documentation for OnBlurAsync.While the property declaration follows the established pattern, the documentation could be more descriptive to help developers understand its purpose and usage.
Consider expanding the documentation like this:
/// <summary> - /// 获得/设置 失去焦点回调方法 默认 null + /// 获得/设置 失去焦点回调方法 + /// <para>当输入框失去焦点时触发的异步回调方法</para> + /// <para>参数为当前输入值，默认为 null</para> /// </summary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/BootstrapBlazor/Components/Input/BootstrapInput.razor.cs (0 hunks)
- src/BootstrapBlazor/Components/Input/BootstrapInputBase.cs (2 hunks)
- src/BootstrapBlazor/Components/Input/FloatingLabel.razor (1 hunks)
💤 Files with no reviewable changes (1)
- src/BootstrapBlazor/Components/Input/BootstrapInput.razor.cs
🔇 Additional comments (3)
src/BootstrapBlazor/Components/Input/FloatingLabel.razor (1)
6-6: LGTM! Consider updating documentation.The addition of
@onblur="@OnBlur"correctly implements the async blur functionality by binding to the base class handler.Let's check if the documentation needs updating:
✅ Verification successful
Let me try a different approach to verify the documentation needs by searching for any documentation files that might reference floating labels or input components.
Let me try one final verification to check if there are any documentation files in the repository root or component directories that might need updating.
Based on the verification results, I can now provide the final response:
The change is correct, no documentation updates required
The
@onblurbinding addition to FloatingLabel component is correct, and after thorough verification:
- No component-specific documentation exists that needs updating
- The only documentation file (docs/repro.md) is for bug reporting guidelines
- The component inherits the functionality from BootstrapInputBase, which already has the documentation for this feature
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for documentation files that might need updating # Test: Search for FloatingLabel documentation fd -e md -e mdx FloatingLabelLength of output: 31
Script:
#!/bin/bash # Search for documentation files in common doc directories fd -e md -e mdx . docs src/BootstrapBlazor/docs # Search for any documentation mentioning FloatingLabel or input components rg -i "floatinglabel|floating label|input component" -g "*.{md,mdx}" # Check if there's a README in the component directory find src/BootstrapBlazor/Components/Input -name "README.md"Length of output: 265
Script:
#!/bin/bash # Check content of docs/repro.md to see if it's relevant cat docs/repro.md # Search for any documentation in the Components directory fd -e md -e mdx . src/BootstrapBlazor/Components # Look for any API documentation patterns rg -i "onblur|floating.*label" README.md || trueLength of output: 2151
src/BootstrapBlazor/Components/Input/BootstrapInputBase.cs (2)
200-209: LGTM! Implementation follows established patterns.The OnBlur method implementation is well-structured:
- Follows the async/await pattern
- Includes null check before callback invocation
- Allows customization in derived classes through the virtual modifier
- Maintains consistency with other callback implementations in the class
200-209: Verify blur event binding in FloatingLabel component.Let's ensure the FloatingLabel component correctly binds to this new OnBlur method.
✅ Verification successful
Let me try a different approach to verify the blur event binding and component integration.
Based on the search results, I can now provide a conclusive response:
Blur event binding is correctly implemented across input components
The verification shows that:
- The
FloatingLabelcomponent (src/BootstrapBlazor/Components/Input/FloatingLabel.razor) correctly binds to theOnBlurmethod using@onblur="@OnBlur"in its template.- This implementation is consistent with other input components in the codebase that inherit from
BootstrapInputBase.- The blur event binding pattern is uniformly implemented across related components:
- FloatingLabel.razor
- BootstrapInput.razor
- AutoComplete.razor
- InputNumber.razor
- Search.razor
- Textarea.razor
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for blur event binding in FloatingLabel component echo "Searching for blur event binding in FloatingLabel component..." rg -l "FloatingLabel" | xargs rg "@onblur|@bind-OnBlur|@onclick:blur" -A 2 -B 2Length of output: 214
Script:
#!/bin/bash # First, let's find all files containing FloatingLabel echo "Files containing FloatingLabel:" fd . -t f | xargs rg -l "FloatingLabel" # Then search for Razor component files with blur event handling echo -e "\nSearching for blur event handling in Razor files:" fd . -t f -e razor | xargs rg "@onblur|@bind-OnBlur|@onclick:blur" -A 2 -B 2 # Look for the base input component implementation echo -e "\nSearching for Input component implementation:" rg -l "BootstrapInputBase" | xargs rg "protected override async Task OnAfterRenderAsync|protected override void BuildRenderTree"Length of output: 6543
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4523 +/- ##
===========================================
- Coverage 100.00% 99.98% -0.02%
===========================================
Files 631 631
Lines 27070 27065 -5
Branches 3927 3927
===========================================
- Hits 27070 27061 -9
- Misses 0 3 +3
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/BootstrapBlazor/Components/InputNumber/BootstrapInputNumber.razor.cs (1)
265-268: Consider adding error handling for JS interop.The
InvokeVoidAsynccall could throw an exception if the JS interop fails. Consider wrapping it in a try-catch block to handle potential failures gracefully.if (NullableUnderlyingType != null && string.IsNullOrEmpty(CurrentValueAsString)) { // set component value empty - await InvokeVoidAsync("clear", Id); + try + { + await InvokeVoidAsync("clear", Id); + } + catch (JSException ex) + { + // Log the error or handle it appropriately + Logger.LogWarning(ex, "Failed to clear input value via JS interop"); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/BootstrapBlazor/Components/AutoComplete/AutoComplete.razor.cs (1 hunks)
- src/BootstrapBlazor/Components/AutoFill/AutoFill.razor.cs (1 hunks)
- src/BootstrapBlazor/Components/InputNumber/BootstrapInputNumber.razor.cs (1 hunks)
- src/BootstrapBlazor/Components/Textarea/Textarea.razor.cs (0 hunks)
💤 Files with no reviewable changes (1)
- src/BootstrapBlazor/Components/Textarea/Textarea.razor.cs
🔇 Additional comments (2)
src/BootstrapBlazor/Components/AutoFill/AutoFill.razor.cs (2)
Line range hint
191-191: LGTM: Improved backward compatibility.The change from collection expression syntax to explicit List initialization enhances compatibility with older C# versions while maintaining the same functionality.
142-156: Consider adding null check for Value parameter in OnBlurAsync.The implementation looks good overall, but there are two points to consider:
- The
Valueparameter passed toOnBlurAsyncshould have a null check to prevent potential null reference issues.- Verify if executing
OnBlurAsyncafterOnSelectedItemChangedis the intended order, as it might affect dependent operations.
add OnBlurAsync parameter
Summary of the changes (Less than 80 chars)
简单描述你更改了什么, 不超过80个字符；如果有关联 Issue 请在下方填写相关编号
Description
fixes #4522
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 Sourcery
Add the OnBlurAsync parameter to the BootstrapInputBase class to support asynchronous operations on input blur events, and refactor the FloatingLabel component to use this new parameter.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
OnBlurAsyncproperty in theBootstrapInputBaseclass, allowing users to specify a callback for when the input loses focus.FloatingLabelcomponent to enhance user interaction.Bug Fixes
OnBlurAsyncfunctionality from multiple components, simplifying user input handling.Refactor
OnBlurmethods across various components to improve event handling and accessibility.