Popover: Prevent the Popover from going outside the browser window in some situations

[Saman-00]

Description

popover-content is positioned based on the parent element of the MudPopover component, and when this parent element is outside the browser window, the popup also goes outside the window and is partially visible or not visible at all, this seems to be a common occurrence in MudDataGrid's simple filter popover when the grid has a scroll.

This behavior can be viewed here: https://try.mudblazor.com/snippet/cOGeEScJpoYZVWLx
This commit attempts to fix the issue by preventing the popover from going outside the window.
It potentially fixes these issues: Fixes #8180, fixes #8173, fixes #7032, fixes #7707

This video shows how it currently is: https://github.com/MudBlazor/MudBlazor/assets/5804803/803e4884-a5b0-4452-ae4c-afe2552c3ed8
This one shows how it will behave after the change: https://github.com/MudBlazor/MudBlazor/assets/5804803/5f30914b-9bd0-4680-a603-246639298a47

How Has This Been Tested?

This change has been tested visually, I made a component out of the TryMudBlazor snippet and tested manually.

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.49%. Comparing base (28bc599) to head (2fae789).
Report is 176 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8815      +/-   ##
==========================================
+ Coverage   89.82%   90.49%   +0.66%     
==========================================
  Files         412      419       +7     
  Lines       11878    12189     +311     
  Branches     2364     2380      +16     
==========================================
+ Hits        10670    11030     +360     
+ Misses        681      627      -54     
- Partials      527      532       +5     

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

[danielchalmers]

Strangely the notification popover still seems to get clipped (#7032 (comment))

Shrink the window until it changes layout, click the hamburger menu, then click the notifications icon to see

[Saman-00]

Strangely the notification popover still seems to get clipped (#7032 (comment))

Shrink the window until it changes layout, click the hamburger menu, then click the notifications icon to see

Do you mean that the popover still goes outside the window after my changes? because I can't reproduce that.

But my notification area looks wrong, maybe that is the reason

[danielchalmers]

That seems to be working now, not sure if i was on the wrong branch before (🤦 )

Seems to be a miscalculation with the scrolling now:

video6.mp4

See notifications:

[danielchalmers]

What do you think about this by the way #1167 (comment)? I want to get the tooltip layout fixed but something needs to be adjusted in the javascript (try settings display: contents on a tooltip then hover over it).

[ScarletKuro]

Hi

Seems to be a miscalculation with the scrolling now:

@Saman-00 can you fix that? Then we can merge if there no other issues.

[Saman-00]

That can be fixed, I just need to not add scrolling offset when position: fixed.

But now I think my approach to the problem might have been wrong from the beginning.
What this PR tries to achieve is force every popover to be within the viewport, but I don't think that is the intended behavior for popover, if you view the Popover example in the doc with these changes, you will notice that everything looks wrong.

All of the popovers jump into view when window resizes.

The only time I can think of where this change would make sense is either when position is fixed, or when LockScroll="true", because in both situations, it doesn't make sense to render the element outside the window, so we force them to stay within the window.
But in case of LockScroll, we can't use it for that either because it is an attribute of Overlay and not Popover, and we can't check the document body's class because the same thing will happen as the image above whenever an Overlay with LockScroll parameter is displayed.

Should I just close this PR or add it only when position: fixed?
What do you think, do you think there is a better solution?

[danielchalmers]

@Saman-00 Would it make sense if

• The popover always initially spawns in the viewport
• If you resize the window, it resizes proportionally by dimensions; If it was out of view it stays out, but if it was in view it stays in.
• If you scroll the viewport, it will stay where it is on the page, not the viewport, so you can scroll it out of view

Me and @Garderoben talked about scroll locking before and the current behavior where you can freely scroll was preferred. MUI does lock it on the other hand.

[ScarletKuro]

Maybe we should not even try to fix it, if it's too much work.
There is plan to replace the popover with https://floating-ui.com/
Because nobody wants to maintain our popover js code and I had this arguments to use it:

1. It's used by most websites in the world, so it's always maintained.
2. The JavaScript code is not that large.
3. Our popover is buggy, and nobody wants to touch the JS part.
4. There's no need for the PopoverProvider.
5. We need to stop reinventing the wheel, especially for something as complicated as a popover. Popovers cannot be created solely in C# and mostly rely on JS, which makes it challenging for us to ensure quality.