refactor(Toast): perfect AutoHide logic #6039
Merged
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 GuideRefactored Toast.razor.js to centralize progress element management, streamline event handling, introduce a transitionend auto-hide trigger, refine autohide update behavior, and ensure complete cleanup in dispose. Sequence Diagram: Toast Auto-Hide on Progress Transition EndsequenceDiagram
participant Caller
participant ToastModule as "Toast.razor.js init()"
participant BootstrapToast as "bootstrap.Toast"
participant ProgressElement as "progressElement"
participant EventHandler
Caller->>ToastModule: init(id, invoke, callback)
ToastModule->>ToastModule: Get progressElement from DOM
ToastModule->>ToastModule: Create toastObject (now includes progressElement reference)
alt autohide is true
ToastModule->>ProgressElement: style.transition = `width linear ${delay}s`
end
ToastModule->>EventHandler: on(el, 'shown.bs.toast', ...)
ToastModule->>EventHandler: on(el, 'hidden.bs.toast', ...)
ToastModule->>EventHandler: on(progressElement, 'transitionend', handleTransitionEndCallback)
ToastModule->>BootstrapToast: show()
EventHandler-->>ToastModule: 'shown.bs.toast' event received
alt autohide is true
ToastModule->>ProgressElement: style.width = '100%' (progress animation starts)
end
Note over ProgressElement: Progress bar animates to 100%
EventHandler-->>ToastModule: 'transitionend' event received (from progressElement)
ToastModule->>BootstrapToast: hide() // Toast hiding is triggered by progress completion
EventHandler-->>ToastModule: 'hidden.bs.toast' event received
ToastModule->>Caller: invoke.invokeMethodAsync(callback) // Notify .NET about closure
Sequence Diagram: Toast Update Logic for AutoHide ConfigurationsequenceDiagram
participant Caller
participant ToastModuleUpdate as "Toast.razor.js update()"
participant ToastObject
participant BootstrapToastConfig as "bootstrap.Toast._config"
participant ProgressElementStyle as "progressElement.style"
Caller->>ToastModuleUpdate: update(id)
ToastModuleUpdate->>ToastObject: Get toast data (element, toast, progressElement)
ToastModuleUpdate->>ToastObject: Read autohide & delay from element attributes
alt autohide is true (based on element attributes)
ToastModuleUpdate->>BootstrapToastConfig: toast._config.autohide = true
ToastModuleUpdate->>BootstrapToastConfig: toast._config.delay = newDelay
ToastModuleUpdate->>ProgressElementStyle: progressElement.style.width = '100%'
ToastModuleUpdate->>ProgressElementStyle: progressElement.style.transition = `width linear ${newDelay / 1000}s`
else autohide is false (based on element attributes)
ToastModuleUpdate->>BootstrapToastConfig: toast._config.autohide = false
ToastModuleUpdate->>ProgressElementStyle: progressElement.style.removeProperty('width')
ToastModuleUpdate->>ProgressElementStyle: progressElement.style.removeProperty('transition')
end
Class Diagram: Structure of the Internal Toast ObjectclassDiagram
class ToastObject {
+element: HTMLElement
+invoke: DotNetObjectReference
+callback: string
+toast: BootstrapToast
+progressElement: HTMLElement // Centralized property
+showProgress(): boolean
}
note for ToastObject "Internal object created by Toast.razor.js#init() to manage a toast instance. `progressElement` is now a direct and centralized property, simplifying its access and management."
class BootstrapToast {
<<External Library>>
_config: object
show()
hide()
dispose()
}
class EventHandler {
<<Utility Module>>
static on(element, eventName, handler)
static off(element, eventName, handler)
}
namespace ToastRazorJs {
class Functions {
<<JavaScript Module Functions>>
init(id, invoke, callback): ToastObject
update(id): void
dispose(id): void
}
}
note for Functions "Exported functions in Toast.razor.js that operate on ToastObject instances stored in a Data cache."
Functions ..> ToastObject : Creates & Manages
ToastObject "1" *-- "1" BootstrapToast : aggregates via `toast` property
ToastObject "1" *-- "1" HTMLElement : holds `element` (the main toast DOM element)
ToastObject "1" *-- "1" HTMLElement : holds `progressElement` (the progress bar DOM element)
Functions ..> EventHandler : Uses for event binding and unbinding
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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.
|
|
||
| toast._config.autohide = autoHide; | ||
| toast._config.delay = delay; | ||
|
|
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Specify radix in parseInt
Include a radix of 10 (parseInt(..., 10)) to ensure decimal parsing; without it, leading zeros can cause unexpected results.
Suggested change
| const delay = parseInt(element.getAttribute('data-bs-delay'), 10); |
Comment on lines
+32
to
+34
| EventHandler.on(progressElement, 'transitionend', e => { | ||
| toast.toast.hide(); | ||
| }); |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Filter transitionend by property name
Guard with if (e.propertyName !== 'width') return; so only the width transition triggers hide and other transitions won’t call it.
Suggested change
| EventHandler.on(progressElement, 'transitionend', e => { | |
| toast.toast.hide(); | |
| }); | |
| EventHandler.on(progressElement, 'transitionend', e => { | |
| if (e.propertyName !== 'width') return; | |
| toast.toast.hide(); | |
| }); |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6039 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 701 701
Lines 30955 30955
Branches 4377 4377
=========================================
Hits 30955 30955 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 #6038
Summary By Copilot
This pull request refactors the
Toast.razor.jsfile to improve code maintainability and fix issues related to the toast progress bar and event handling. Key changes include centralizing theprogressElementlogic, updating event handlers, and enhancing the handling of theautohideconfiguration.Refactoring and code improvements:
progressElementlogic by moving its initialization to theinitfunction and ensuring consistent usage across methods. This eliminates redundant queries for the.toast-progresselement.el(the toast element) instead of repeatedly referencingtoast.element. This improves readability and reduces potential errors.Functional enhancements:
transitionendevent on theprogressElementto automatically hide the toast when the progress bar animation completes.updatefunction to handle cases whereautohideis disabled by removing the progress bar's width and transition properties. This ensures proper behavior for non-autohiding toasts.Cleanup and disposal:
disposefunction to cleanly remove all associated event handlers, including the newtransitionendevent on the `progressElementRegression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refactor toast component to improve auto-hide logic by centralizing progress bar handling, adding a transitionend listener to trigger hide on animation completion, handling disabled autohide by clearing styles, and ensuring proper cleanup of event listeners
Bug Fixes:
Enhancements: