Codecov Report

Patch coverage: 80.00% and project coverage change: -0.02% ⚠️

Comparison is base (06570f4) 90.61% compared to head (04399bb) 90.60%.
Report is 4 commits behind head on dev.

❗ Current head 04399bb differs from pull request most recent head 8715620. Consider uploading reports for the commit 8715620 to get more accurate results

@@            Coverage Diff             @@
##              dev    #7472      +/-   ##
==========================================
- Coverage   90.61%   90.60%   -0.02%     
==========================================
  Files         427      427              
  Lines       15199    15206       +7     
==========================================
+ Hits        13773    13777       +4     
- Misses       1426     1429       +3     

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

@henon do you know if it's possible to unit test scrolling with bUnit? If yes, we definitely should ask it from author then.

Do I understand correctly that it also solves this issues #6108? Since its mentioned in your #7454 issue

Do I understand correctly that it also solves this issues #6108? Since its mentioned in your #7454 issue

Yes, it does.

I've tried to write a test for that but I had few problems with it... I might need help for that

I did change the ScrollManager js calls from OnAfterRenderAsync because was calling the same multiple times.

They'll be called only if required when the value Visible change.

Is failing DebouncedTextFieldRerenderTest but I don't know why. In local it passes in 3s

I will reopen a new PR with cleaner commits.

The commits don't matter, in the end we'll squash everything anyway so you can as well reopen this.

As for testing, AFAIK the only way to test this is to check if the overflow css attribute is applied to the correct container. There should be Dialog tests that check similar things. You can print the html with console writeline to see the html structure that gets constructed by the test case. Does this help?

The commits don't matter, in the end we'll squash everything anyway so you can as well reopen this.

Yeah, you can commit as much as you want. There is usually no reason to open new PR since you can do with your branch whatever you want: squash, edit commit history, force push etc.

I did change the ScrollManager js calls from OnAfterRenderAsync because was calling the same multiple times.

What you just did i.e. moving the js calls from OnAfterRenderAsync to the [Parameter] is very bad practice, later it will cause problems with pre-rendering on NET8. These JS calls should be in OnAfterRenderAsync because this event is happening when the browser is attached and JS calls are possible. If multiple calls are not desirable you can use firstTime argument / local variables to check that you have already called your methods.

I did change the ScrollManager js calls from OnAfterRenderAsync because was calling the same multiple times.

What you just did i.e. moving the js calls from OnAfterRenderAsync to the [Parameter] is very bad practice, later it will cause problems with pre-rendering on NET8. These JS calls should be in OnAfterRenderAsync because this event is happening when the browser is attached and JS calls are possible. If multiple calls are not desirable you can use firstTime argument / local variables to check that you have already called your methods.

https://learn.microsoft.com/en-us/aspnet/core/blazor/javascript-interoperability/call-javascript-from-dotnet?view=aspnetcore-7.0#prerendering
for more information

Thank you guys!
I'll change my code and I'll update you!

Just noticed that most of the bugs were just because of the js functions.. for now I would go easy on that to not to risk to make things worse.
Thank you for all the reviews!
Now the only files that are changed are the JS and a new rule for css. It will fix most of the overlay scrolling problems.

So far, the only one that I've noticed not working as expected is mudDrawer. Does not block anything.

Do I need to change description/PR ?

Sure, what would be a better description of the change? So you think this ready to merge?

Sure, what would be a better description of the change?

Is basically a small enhancement on the javascript functions "lockScroll" and "unlockScroll".
Everything that was working before now is working also for touchscreen devices.
After this change, the dialog and everything that works with MudOverlay don't need to be modified to work properly.

So you think this ready to merge?

Yes, I think so.