MudMenu: Revert previous refactor#11484
Conversation
github-actions
bot
added
the
bug
There was a problem hiding this comment.
Pull Request Overview
This PR reverts part of a previous refactor of MudMenu to restore the pre-refactor hover behavior and cancellation logic while improving inline documentation.
- Replaces DebounceDispatcher usage with CancellationTokenSource-based delays for hover and leave events.
- Updates inline comments and XML documentation for clarity.
- Removes a unit test method that validated debounce cancellation for pointer events.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/MudBlazor/Components/Menu/MudMenu.razor.cs | Reverts debounce refactor; replaces DebounceDispatcher with CTS-based delays and updates documentation. |
| src/MudBlazor.UnitTests/Components/MenuTests.cs | Removes tests covering debounce cancellation behavior in pointer events. |
Comments suppressed due to low confidence (2)
src/MudBlazor.UnitTests/Components/MenuTests.cs:697
- The test for hover debounce cancellation behavior was removed. Consider adding tests to verify that the new cancellation logic with CancellationTokenSource functions as expected.
public async Task Menu_PointerEvents_Cancellation()
src/MudBlazor/Components/Menu/MudMenu.razor.cs:696
- [nitpick] After cancelling the existing CancellationTokenSource instances in CancelPendingActions, consider disposing them and setting them to null to free resources immediately rather than waiting for the final dispose in the Dispose method.
private void CancelPendingActions()
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #11484 +/- ##
==========================================
- Coverage 91.15% 91.13% -0.03%
==========================================
Files 466 466
Lines 14453 14471 +18
Branches 2800 2808 +8
==========================================
+ Hits 13175 13188 +13
- Misses 641 644 +3
- Partials 637 639 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
versile2
left a comment
versile2
left a comment
There was a problem hiding this comment.
Love all the extra comments. Funny thing is I switched it back and it did not fix it, but I can confirm in the MenuEdgeCasesTest it is fixed. I think the only thing I did different is not surround it in HoverDelay > 0 logic. Either way good job. Thanks!
I kind of don't see the difference, it's using the same |
|
@ScarletKuro Do you think we should merge it in the working state and we can take another look in the future? I imagine it could be used in a few other places as well. Took me a long time to get it working initially so not too keen on changing it again now. |
Description
Fixes #11159 by partially reverting #11117.
The menu logic can be difficult to get around due to the complexity and was broken by the refactor. I don't think the DebounceDispatcher is actually handling the cancellations properly and I want to preserve the more flexible method used previously instead of trying to build on that class.
I tried to include all fixes and improvements made to the code that aren't related to the refactor. No other changes were made by me other than restoring the old code and improving the comments for the future.
How Has This Been Tested?
unit
Type of Changes
Recording.2025-06-11.124225.mp4
Checklist
dev).