Skip to content

MudMenu Nested Fix#11117

Merged
ScarletKuro merged 3 commits intoMudBlazor:devfrom
versile2:fix/menunesting
Mar 31, 2025
Merged

MudMenu Nested Fix#11117
ScarletKuro merged 3 commits intoMudBlazor:devfrom
versile2:fix/menunesting

Conversation

@versile2
Copy link
Contributor

@versile2 versile2 commented Mar 31, 2025

Description

Refactored the cancellation and Task.Delay logic into DebounceDispatcher
Made pointerleave 2x the hoverdelay as pointerenter for smoother transitions
Updated MudGlobal xml to read that behavior
Modified overlay to only have one overlay that is not repositioned allowing PointerEnter and PointerLeave to operate as intended
Added exception to PointerLeave for the Parent menu that does not activate by hover
Resolves #11115

How Has This Been Tested?

Visually tested in Viewer (WASM) and Docs (Server)
Added multiple unit tests to cover all scenarios and test the actual timing

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

versile2 added 2 commits March 31, 2025 13:35
@versile2
Refactor MudMenu for improved clarity and performance
cd190b0 
- Updated `MudOverlay` components with a new property.
- Introduced `DebounceDispatcher` in `MudMenu.razor.cs` for hover delay management to replayce Task.Delay.
- Replaced cancellation tokens with debouncers to streamline hover actions.
- Enhanced overall responsiveness and usability of the `MudMenu` component by increasing the leave to be 2x delay.

- V2
@versile2
unit tests and typo
8533d42
@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Mar 31, 2025
@versile2 versile2 requested a review from ScarletKuro March 31, 2025 20:24
@codecov
Copy link

codecov bot commented Mar 31, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.99%. Comparing base (dd76e57) to head (c6904d9).
Report is 2 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev   #11117      +/-   ##
==========================================
+ Coverage   90.97%   90.99%   +0.02%     
==========================================
  Files         431      431              
  Lines       14056    14055       -1     
  Branches     2728     2722       -6     
==========================================
+ Hits        12788    12790       +2     
+ Misses        651      650       -1     
+ Partials      617      615       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ScarletKuro
ScarletKuro requested changes Mar 31, 2025
View reviewed changes
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 31, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@versile2 versile2 requested a review from ScarletKuro March 31, 2025 20:50
@ScarletKuro ScarletKuro merged commit 11a8013 into MudBlazor:dev Mar 31, 2025
6 checks passed
@ScarletKuro
Copy link
Member

ScarletKuro commented Mar 31, 2025 •
edited
Loading

Maybe in future DebounceDispatcher we should use the TimeProvider so it would be easier to test and the tests wouldn't rely on Task.Delay(hoverDelay + 100); which may be flaky.
https://andrewlock.net/exploring-the-dotnet-8-preview-avoiding-flaky-tests-with-timeprovider-and-itimer/

@versile2
Copy link
Contributor Author

versile2 commented Mar 31, 2025

Maybe in future DebounceDispatcher we should use the TimeProvider so it would be easier to test and the tests wouldn't rely on Task.Delay(hoverDelay + 100); which may be flaky. https://andrewlock.net/exploring-the-dotnet-8-preview-avoiding-flaky-tests-with-timeprovider-and-itimer/

very cool

@danielchalmers
Copy link
Member

danielchalmers commented Apr 1, 2025

2x delay is not ideal as it no longer matches common standard as seen in google docs etc. and now has inconsistent close timing:

Video3.mp4

@ScarletKuro
Copy link
Member

ScarletKuro commented Apr 1, 2025 •
edited
Loading

As I understand the nested MudMenu got broken because of #10551?
We could potentially just go to version before that change.
@danielchalmers feel free to take this into your hands.

@versile2
Copy link
Contributor Author

versile2 commented Apr 1, 2025 •
edited
Loading

There were a number of issues actually.

  1. The odd issue mentioned from the overlay PR to 10551, which is why I switched to DebounceDispatcher (and easier to read)
  2. Each sub menu creating it's own overlay
  3. Overlay getting re positioned for nested popovers causing PointerLeave to fire at the wrong time.
  4. No good tests to prevent most of these problems from reoccuring

That being said I made a judgement call on the 2x hoverdelay, didn't see any spec - feel free to change it back.

Screen.Recording.2025-04-01.083442.mp4

@danielchalmers danielchalmers mentioned this pull request Apr 1, 2025
Merged
7 tasks
@versile2 versile2 deleted the fix/menunesting branch April 9, 2025 19:10
This was referenced Apr 11, 2025
Closed
Merged
@danielchalmers danielchalmers mentioned this pull request Jun 11, 2025
Merged
7 tasks
This was referenced Aug 1, 2025
Closed
Open
This was referenced Aug 18, 2025
Closed
Closed
Merged
@dependabot dependabot bot mentioned this pull request Aug 27, 2025
Merged
@dependabot dependabot bot mentioned this pull request Sep 8, 2025
Closed
@dependabot dependabot bot mentioned this pull request Sep 8, 2025
Closed
@dependabot dependabot bot mentioned this pull request Sep 22, 2025
Closed
This was referenced Oct 5, 2025
Merged
Open
Open
Open
@dependabot dependabot bot mentioned this pull request Nov 3, 2025
Merged
@dependabot dependabot bot mentioned this pull request Nov 12, 2025
Closed
@dependabot dependabot bot mentioned this pull request Dec 21, 2025
Closed
This was referenced Dec 29, 2025
Closed
Closed
This was referenced Jan 9, 2026
Closed
Closed
Closed
Merged
@dependabot dependabot bot mentioned this pull request Feb 10, 2026
Merged
This was referenced Feb 23, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ScarletKuro ScarletKuro ScarletKuro approved these changes

Assignees

No one assigned

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MudMenu with nested submenu closes automatically on hover

3 participants

@versile2 @ScarletKuro @danielchalmers