Conversation
Reviewer's GuideAdds an async-close capability to Drawer via DrawerOption.CloseAsync, wires Drawer instance into DrawerOption at render time, and updates tests to validate closing through the option instead of directly on the Drawer component. Sequence diagram for DrawerOption.CloseAsync interactionsequenceDiagram
actor Caller
participant DrawerOption
participant Drawer
Caller->>DrawerOption: CloseAsync()
activate DrawerOption
alt Drawer instance is set
DrawerOption->>Drawer: Close()
activate Drawer
Drawer-->>DrawerOption: completed Task
deactivate Drawer
else Drawer instance is null
DrawerOption-->>Caller: return completed Task
end
DrawerOption-->>Caller: completed Task
deactivate DrawerOption
Updated class diagram for DrawerOption and Drawer relationshipclassDiagram
class DrawerOption {
int? ZIndex
internal Drawer Drawer
Task CloseAsync()
}
class Drawer {
Task Close()
}
class DrawerContainer {
void RenderDrawer(RenderTreeBuilder builder, DrawerOption option)
}
DrawerOption --> Drawer : holds_reference
DrawerContainer --> DrawerOption : configures
DrawerContainer ..> Drawer : renders
Flow diagram for DrawerContainer RenderDrawer component reference captureflowchart TD
A[RenderDrawer called with DrawerOption] --> B[Create Drawer component]
B --> C[Apply parameters from DrawerOption]
C --> D[Capture component reference]
D --> E[Set option.Drawer to Drawer instance]
E --> F[DrawerOption.CloseAsync can call Drawer.Close]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Drawer/DrawerOption.cs:96-101` </location>
<code_context>
+ /// <summary>
+ /// 关闭抽屉弹窗方法
+ /// </summary>
+ public async Task CloseAsync()
+ {
+ if (Drawer != null)
+ {
+ await Drawer.Close();
+ }
+ }
}
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid unnecessary async state machine allocation in `CloseAsync` when only forwarding to `Drawer.Close()`.
Since this method only forwards to `Drawer?.Close()`, you can remove `async/await` and return the task directly:
```csharp
public Task CloseAsync() => Drawer?.Close() ?? Task.CompletedTask;
```
This keeps the API unchanged while avoiding the async state machine allocation and extra overhead when `Drawer` is null.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7506 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 748 748
Lines 32978 32986 +8
Branches 4583 4584 +1
=========================================
+ Hits 32978 32986 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a CloseAsync method to the DrawerOption class, enabling programmatic closing of drawers through the option object. This enhancement provides a cleaner API for consumers who need to close drawers without direct access to the Drawer component instance.
Changes:
- Added
CloseAsync()public method and internalDrawerproperty toDrawerOption - Updated
DrawerContainerto capture the Drawer component reference and assign it to the option - Replaced direct Drawer component test with a more comprehensive service-level test that validates the new API
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Drawer/DrawerOption.cs | Added CloseAsync method and internal Drawer property to enable programmatic drawer closing |
| src/BootstrapBlazor/Components/Drawer/DrawerContainer.cs | Added component reference capture to link DrawerOption with Drawer instance |
| test/UnitTest/Components/DrawerTest.cs | Removed redundant Close_Ok test that directly tested Drawer.Close() |
| test/UnitTest/Services/DrawerServiceTest.cs | Added comprehensive test for CloseAsync functionality through DrawerService |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| internal Drawer? Drawer { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// 关闭抽屉弹窗方法 |
There was a problem hiding this comment.
The documentation comment uses Chinese text ("关闭抽屉弹窗方法") which is inconsistent with other documentation in the file that uses English. For consistency with the codebase style, this should be in English, such as "Closes the drawer".
| /// 关闭抽屉弹窗方法 | |
| /// Closes the drawer. |
| button = cut.Find("button"); | ||
| await cut.InvokeAsync(() => button.Click()); | ||
|
|
||
| // 测试 Option 关闭方法 |
There was a problem hiding this comment.
The test comment uses Chinese text ("测试 Option 关闭方法") which is inconsistent with the rest of the test file that uses English comments. For consistency and maintainability, this should be in English, such as "Test Option CloseAsync method".
| // 测试 Option 关闭方法 | |
| // Test Option CloseAsync method |
Link issues
fixes #7490
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add an asynchronous close operation to drawer options and wire it through the drawer container, updating tests to cover the new behavior.
New Features:
Enhancements:
Tests: