MudPopover: Fix Event Order#11389
Conversation
…gic slightly on scroll handling and added a viewport check.
github-actions
bot
added
bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #11389 +/- ##
=======================================
Coverage 91.10% 91.10%
=======================================
Files 465 465
Lines 14407 14407
Branches 2788 2788
=======================================
Hits 13126 13126
Misses 642 642
Partials 639 639 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes event handling and repositioning issues for MudPopover, improving scroll handling and ensuring popovers are only shown when in the viewable area. Key changes include:
- Adding an isInViewport helper to determine element visibility within scrollable containers.
- Refactoring scroll event listener management, including storing and removing custom handlers.
- Adjusting popover repositioning using a setInterval tied to transition timing.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/MudBlazor/TScripts/mudPopover.js | Updated popover logic to improve viewport checks, scroll event handling, and repositioning during transitions. |
| src/MudBlazor.UnitTests.Viewer/TestComponents/Tooltip/TooltipScrollTest.razor | Added a test component to verify tooltip behavior under scroll conditions. |
Comments suppressed due to low confidence (1)
src/MudBlazor/TScripts/mudPopover.js:227
- [nitpick] The isInViewport function currently returns false as soon as a scrollable container is encountered. Consider calculating the intersection of the element's bounding rectangle with its parent's visible area to better handle partial visibility scenarios.
if (isScrollableY || isScrollableX) { return false; }
|
The animation is fairly clunky, especially noticeable when slowed down: Video7.mp4and may not finish if you extend the mud-open-dialog-center animation: Video8.mp4 |
|
|
I changed the function to look at all parents and have video showing what 25 fpd (frames per duration/delay) and 12 fpd do. In each video the first is at the default .1s and the second is at 3s. New commit is at 12fpd. |
danielchalmers
left a comment
danielchalmers
left a comment
There was a problem hiding this comment.
Looks really smooth now
Description
Resolves #11303
Resolves #11288
Made 3 significant changes
How Has This Been Tested?
Visually tested in Docs and Viewer the following: Tooltips, Overlay, Popover, Autocomplete, Select, Menu
Type of Changes
Checklist
dev).