Codecov Report

Merging #4609 (57c02f2) into dev (6f5264b) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##              dev    #4609   +/-   ##
=======================================
  Coverage   91.30%   91.30%           
=======================================
  Files         362      362           
  Lines       12499    12499           
=======================================
  Hits        11412    11412           
  Misses       1087     1087           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f5264b...57c02f2. Read the comment docs.

Thanks for this
IMO the better solution is to have a DisposeAsync await and trap the error there. Thats the pattern we use in other places.
@just-the-benno @henon.

Really not a fan of AndForget

Hi @diegofrata, and thanks for submitting the PR.

I guess, the Console.WriteLine() is an artefact of a "testing" sessions. I also forget multiple times to take it out. I would remove it instead of changing it to Debug.WriteLine.

I agree with @mikes-gh regarding the DisposeAsync. It is consistent with other components and makes sense because you want to detach the event if the drawer is not needed anymore. (If I understood it correctly).

Oh, I must have missed it. I saw AndForget being used in a few other places and understood it as pattern to be followed. I will refactor it and amend the PR! Thanks for the feedback!

Can we centralise the error trapping in the extension method?
So calling the extension. method is just a best efforts?

I would still advocate DisposeAsync as well though

I probably should have been clearer.
I mean

https://github.com/diegofrata/MudBlazor/blob/57c02f2d321e9de2d9638e62779bba3078ac812a/src/MudBlazor/Extensions/ElementReferenceExtensions.cs#L96

What I am saying applies. to all our ElementRef extension methods. Trapping JSDisconnectedException might make sense here instead of in the component. When calling the extension method when there is no JSRuntime available should we just return a ValueTask.CompletedTask. These extension methods were written before JSDisconnectedException existed.

I may be wrong and haven't done tests.

The bug this PR fixes has already been fixed by #5562. Thus we are closing this PR. Nevertheless, thank you, all contributions are appreciated!