MudBlazor icon indicating copy to clipboard operation
MudBlazor copied to clipboard

Fix: MudScrollToTop throws exception with a page change.

Open ScarletKuro opened this issue 1 year ago • 1 comments

Description

fixes #5015 replaces this PR #5052

Made by analogue from mudScrollSpy.js https://github.com/MudBlazor/MudBlazor/blob/66894b28cdee27b576ae5a82841c6bbbf519efc3/src/MudBlazor/TScripts/mudScrollSpy.js#L12

How Has This Been Tested?

with reproducing the steps from the #5015 and verifying that error doesn't appear.

Types of changes

  • [x] 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)

Checklist:

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

ScarletKuro avatar Aug 10 '22 11:08 ScarletKuro

Codecov Report

Merging #5060 (1c28559) into dev (ad4b97d) will decrease coverage by 0.00%. The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev    #5060      +/-   ##
==========================================
- Coverage   90.04%   90.03%   -0.01%     
==========================================
  Files         370      370              
  Lines       13362    13369       +7     
==========================================
+ Hits        12032    12037       +5     
- Misses       1330     1332       +2     
Impacted Files Coverage Δ
...Blazor/Components/Snackbar/SnackBarMessageState.cs 54.54% <0.00%> (-3.52%) :arrow_down:
...c/MudBlazor/Components/Snackbar/SnackbarOptions.cs 66.66% <0.00%> (ø)
.../MudBlazor/Components/Radio/MudRadioGroup.razor.cs 100.00% <0.00%> (ø)
...or/Components/Snackbar/MudSnackbarElement.razor.cs 81.08% <0.00%> (+1.08%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 10 '22 11:08 codecov[bot]

Hi @ScarletKuro and thanks for your work

I'm sorry in that case that our previous discussion has disappointed you and reduced your ambition. You can, of course, propose changes to the JS interop service. I was only reluctant to the reactive part not your improvements to the service. We can accept this PR as it is, but if you want to improve more, feel encouraged to do so.

just-the-benno avatar Aug 14 '22 05:08 just-the-benno

@just-the-benno I'm not disappointed. It was only to to raise the attention that event doesn't really work well with this project because it's almost all the way async which forces you to use async void in some places which is not really good(even though, it's considered to be safe with eventhandlers), and that you might end up with more ugly and boilerplate code, and here is well known reactive which can solve this problems, but it wasn't pretending to be the solution that should be accepted, so don't sweat about it.

The most important is to fix the bug, which this PR does.

ScarletKuro avatar Aug 14 '22 15:08 ScarletKuro

@henon Ready for your review

just-the-benno avatar Aug 14 '22 22:08 just-the-benno

Thanks!

henon avatar Aug 15 '22 17:08 henon