feat(Toast): support async update content use same option #6037
+103
−26
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR enhances the Toast system by enabling asynchronous content updates using the same ToastOption instance, introducing a dynamic JavaScript update logic, refactoring naming and access levels for consistency, updating the sample UI to showcase async notifications, and adding corresponding localization strings. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6037 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 701 701
Lines 30945 30955 +10
Branches 4375 4377 +2
=========================================
+ Hits 30945 30955 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- Rename the private field
_option(used in async updates) to follow your PascalCase/private naming convention (e.g.optionor_option) consistently across the class. - In the JS
updatefunction, add a guard for missingtoast-progresselements to avoid runtime errors when updating toasts that don’t include a progress bar. - The
ToastContainer.Showmethod usesToasts.Contains(option)(reference equality) to detect existing toasts—consider using a unique identifier or value-based comparison to handle reused option instances more reliably.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 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.
| toast: new bootstrap.Toast(el), | ||
| showProgress: () => { | ||
| return toast.toast._config.autohide | ||
| } |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Event handler for 'transitionend' may be attached multiple times.
Attaching a new listener on each update can cause duplicate handlers, triggering toast.hide() repeatedly and leaking memory. Consider removing existing handlers or using a one-time listener.
| private int _delayTs => Options.Value.ToastDelay / 1000; | ||
| private int DelayTs => Options.Value.ToastDelay / 1000; | ||
|
|
||
| private readonly ToastOption _option = new(); |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider creating a new ToastOption instance for each toast step instead of reusing and mutating a shared field.
Consider avoiding the shared, mutable _option and instead build a fresh ToastOption for each step (or extract a small factory helper). This keeps each toast’s state isolated and makes the flow easier to follow.
For example, you can extract a helper:
private ToastOption CreateStep(string content, ToastCategory category,
bool isAutoHide = true, int delay = 3000, bool forceDelay = false)
{
return new ToastOption {
Title = Localizer["ToastsAsyncDemoTitle"],
Content = content,
Category = category,
IsAutoHide = isAutoHide,
Delay = delay,
ForceDelay = forceDelay
};
}Then rewrite OnAsyncClick without mutating a field:
private async Task OnAsyncClick()
{
// Step 1
var first = CreateStep(Localizer["ToastsAsyncDemoStep1Text"],
ToastCategory.Information,
isAutoHide: false,
delay: 3000,
forceDelay: true);
await ToastService.Show(first);
await Task.Delay(first.Delay);
// Step 2
var second = CreateStep(Localizer["ToastsAsyncDemoStep2Text"],
ToastCategory.Information,
isAutoHide: true);
await ToastService.Show(second);
await Task.Delay(2000);
// Step 3
var third = CreateStep(Localizer["ToastsAsyncDemoStep3Text"],
ToastCategory.Success);
await ToastService.Show(third);
}This removes the mutable _option and makes each toast instantiation self-contained.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6036
Summary By Copilot
This pull request introduces several updates to the Toast notification system in the
BootstrapBlazorproject, focusing on improving functionality, usability, and maintainability. Key changes include adding support for asynchronous notifications, refactoring code for consistency and clarity, and enhancing the JavaScript logic for dynamic updates.Enhancements to Toast functionality:
OnAsyncClickmethod) inToasts.razor.cs, showcasing how notifications can display sequential messages with delays.updatefunction inToast.razor.jsto dynamically update Toast configurations and progress bars without reinitializing the component.Showmethod inToastContainer.razor.csto support updating existing Toast content dynamically.Code refactoring for maintainability:
_delayTswithDelayTsas a private property for better naming consistency inToasts.razor.cs.protectedproperties toprivateinToast.razor.csto encapsulate implementation details.Localization updates:
en-US.jsonandzh-CN.json, ensuring multi-language support for the new feature. [1] [2]Minor fixes and improvements:
Toast.razor.jsfor code consistency.ValueListinToasts.razor.csto improve clarity in attribute definitions.Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable asynchronous updates of Toast notifications and dynamic content reconfiguration for existing toasts without reinitialization.
New Features:
OnAsyncClick) that updates a single toast instance in steps.ToastOptioninToastContainer.Show.Enhancements:
updatefunction in the toast JavaScript to adjust delay and progress bar on re-render._delayTstoDelayTsand changing protected members to private.Documentation: