feat(CardUpload): add ShowConfirmButton parameter#7474
feat(CardUpload): add ShowConfirmButton parameter#7474ArgoZhang merged 6 commits intodotnetcore:mainfrom Tony-ST0754:tony
Conversation
cardupload增加删除确认属性
|
Thanks for your PR, @Tony-ST0754. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideAdds an optional delete-confirmation behavior to CardUpload file deletion, wires it through the shared upload base component, and exposes a toggle for it in the UploadCards demo/sample page. Sequence diagram for CardUpload deletion with optional confirmationsequenceDiagram
actor User
participant UploadCards
participant CardUpload_TValue_ as CardUpload
participant PopConfirmButton
User->>UploadCards: toggle _showDeleteConfirmButton
UploadCards->>CardUpload: ShowDeleteConfirmButton = _showDeleteConfirmButton
alt ShowDeleteConfirmButton is true
User->>PopConfirmButton: click delete icon
PopConfirmButton-->>User: show confirmation dialog
User->>PopConfirmButton: confirm
PopConfirmButton->>CardUpload: OnConfirm()
CardUpload->>CardUpload: OnCardFileDelete(item)
else ShowDeleteConfirmButton is false
User->>CardUpload: click delete button
CardUpload->>CardUpload: OnCardFileDelete(item)
end
Class diagram for updated upload components with ShowDeleteConfirmButtonclassDiagram
class UploadBase_TValue_ {
}
class FileListUploadBase_TValue_ {
+bool ShowDeleteButton
+bool ShowDeleteConfirmButton
+string RemoveIcon
}
class CardUpload_TValue_ {
+bool ShowDeleteButton
+bool ShowDeleteConfirmButton
+Task OnCardFileDelete(UploadFile item)
+bool GetDeleteButtonDisabledString(UploadFile item)
+bool GetShowProgress(UploadFile item)
}
class UploadCards {
-bool _showProgress
-bool _showZoomButton
-bool _showDeleteButton
-bool _showDeleteConfirmButton
-bool _showDownloadButton
}
class PopConfirmButton {
+Placement Placement
+Color Color
+string ConfirmIcon
+Color ConfirmButtonColor
+string Content
+string Icon
+bool IsAsync
+Task OnConfirm()
}
UploadBase_TValue_ <|-- FileListUploadBase_TValue_
FileListUploadBase_TValue_ <|-- CardUpload_TValue_
UploadCards --> CardUpload_TValue_ : configures
CardUpload_TValue_ --> PopConfirmButton : uses when ShowDeleteConfirmButton
class UploadFile {
}
class Placement {
}
class Color {
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider making the PopConfirmButton
Contenttext localizable (or at least configurable via a parameter) instead of hard-coding"确定删除数据吗?"so it can be reused in non-Chinese environments. - The delete icon used in
PopConfirmButtonis hard-coded (fa-regular fa-trash-can) instead of reusing the existingRemoveIconparameter; aligning these would keep icon behavior consistent between the confirm and non-confirm delete buttons.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider making the PopConfirmButton `Content` text localizable (or at least configurable via a parameter) instead of hard-coding `"确定删除数据吗?"` so it can be reused in non-Chinese environments.
- The delete icon used in `PopConfirmButton` is hard-coded (`fa-regular fa-trash-can`) instead of reusing the existing `RemoveIcon` parameter; aligning these would keep icon behavior consistent between the confirm and non-confirm delete buttons.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Upload/CardUpload.razor:70-79` </location>
<code_context>
- @onclick="() => OnCardFileDelete(item)">
- <i class="@RemoveIcon"></i>
- </button>
+ @if (ShowDeleteConfirmButton)
+ {
+ <PopConfirmButton Placement="Placement.Top"
+ Color="Color.Danger"
+ ConfirmIcon="fa-solid fa-triangle-exclamation text-danger"
+ ConfirmButtonColor="Color.Danger"
+ Content="确定删除数据吗?"
+ Icon="fa-regular fa-trash-can"
+ IsAsync="true"
+ OnConfirm="@(async()=>await OnCardFileDelete(item))"/>
+ }
+ else
</code_context>
<issue_to_address>
**issue (bug_risk):** The confirm-button path drops the existing disabled logic on delete, which may regress UX/behavior.
In the non-confirm path, the delete button is bound to `GetDeleteButtonDisabledString(item)` so it can be disabled (e.g., during uploads or particular states). The new `PopConfirmButton` path has no equivalent disabled/`IsDisabled` binding, so users can still open the confirm dialog when the old button would have been disabled. Please pass through the same condition to `PopConfirmButton` (or otherwise share the enable/disable logic) to keep behavior consistent.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Upload/CardUpload.razor:76` </location>
<code_context>
+ Color="Color.Danger"
+ ConfirmIcon="fa-solid fa-triangle-exclamation text-danger"
+ ConfirmButtonColor="Color.Danger"
+ Content="确定删除数据吗?"
+ Icon="fa-regular fa-trash-can"
+ IsAsync="true"
</code_context>
<issue_to_address>
**suggestion:** The confirmation text is hard-coded in Chinese, which may conflict with localization/globalization of the component.
Since this is part of the main library, a hard-coded Chinese `Content` string makes the component harder to reuse and localize. Please either expose this text as a parameter (e.g., `DeleteConfirmMessage`) for callers to supply localized content, or hook into whatever localization mechanism the rest of the library uses.
Suggested implementation:
```
Content="@DeleteConfirmMessage"
```
In the backing class for `CardUpload` (either in `CardUpload.razor`’s `@code` block or in `CardUpload.razor.cs` if it’s a partial class), add a public parameter to expose the message:
- If using `@code` in `CardUpload.razor`:
```csharp
@code {
[Parameter]
public string DeleteConfirmMessage { get; set; } = "确定删除数据吗?";
// existing members...
}
```
- If using a code-behind partial class (`CardUpload.razor.cs`):
```csharp
public partial class CardUpload
{
[Parameter]
public string DeleteConfirmMessage { get; set; } = "确定删除数据吗?";
// existing members...
}
```
If the library already has a localization mechanism (e.g., `IStringLocalizer` or a custom localization service), you should replace the hard-coded default `"确定删除数据吗?"` with a localized resource lookup, for example:
```csharp
public string DeleteConfirmMessage { get; set; }
protected override void OnInitialized()
{
base.OnInitialized();
DeleteConfirmMessage ??= Localizer["DeleteConfirmMessage"];
}
```
adjusting `Localizer` and the resource key to match your existing localization conventions.
</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 #7474 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32922 32936 +14
Branches 4574 4576 +2
=========================================
+ Hits 32922 32936 +14
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:
|
cardupload增加删除确认属性
Link issues
fixes #7477
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add an option for CardUpload to require a confirmation dialog before deleting files and demonstrate its usage in the upload cards sample.
New Features:
Enhancements: