Codecov Report

Attention: Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.54%. Comparing base (28bc599) to head (2463575).
Report is 379 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #9458      +/-   ##
==========================================
+ Coverage   89.82%   90.54%   +0.71%     
==========================================
  Files         412      406       -6     
  Lines       11878    12705     +827     
  Branches     2364     2462      +98     
==========================================
+ Hits        10670    11504     +834     
+ Misses        681      641      -40     
- Partials      527      560      +33     

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

If you go the mud menu docs page, open the menu, then click somewhere else on the page, this line will execute:

Handlers only fire when the parent modifies the destination parameter.
By the way, originally this was supposed to be removed in v7:

But I forgot. It was added for backward compatibility since I was making it for v6, and in the original code, it was doing this:

In good code, the VisibleChanged shouldn't be called when the parent modifies the parameter.

I don't think your code is correct. It should be:

if (AutoClose)
{
	await _visibleState.SetValueAsync(false);
	await OnClosed.InvokeAsync();
}

private async Task OnVisibleParameterChangedAsync(ParameterChangedEventArgs<bool> args)
{
	if (!args.Value)
	{
		await OnClosed.InvokeAsync(); // I'm not sure we should call OnClosed when Visible was modified by the parent, but it really depends on what you are trying to achieve.
	}
}

Something along those lines

@ScarletKuro I made changes like that a bit ago, could you check again?

A functionality I can see that will be required in the future is the possibility to prevent closing in some cases due to application specific logic. This could be accomplished by a separate OnClosing event that is fired first and the overlay will close only if the Cancel arg of the ClosingEventArgs was not set true by its handler. But this can be in another PR and maybe only if it is really needed.

Awesome! Great PR @danielchalmers

A functionality I can see that will be required in the future is the possibility to prevent closing in some cases due to application specific logic. This could be accomplished by a separate OnClosing event that is fired first and the overlay will close only if the Cancel arg of the ClosingEventArgs was not set true by its handler. But this can be in another PR and maybe only if it is really needed.

you mean like with WPF that it has property e.Handled = true in the args (for example KeyPressEventArgs) and it was called then you ignore the default behavior (close)?

Yes, that is what I mean. Window has also a Closing event that can be cancelled, so when the user clicks the X you can first check if you really want to close the window and abort if you have unsaved changes, etc.

Yes, that is what I mean. Window has also a Closing event that can be cancelled, so when the user clicks the X you can first check if you really want to close the window and abort if you have unsaved changes, etc.

Sounds good to me, tho it will be breaking if we add it later, because the signature:

public EventCallback OnClosed { get; set; }

needs to be changed to something like EventCallback<CloseArgs>

No, you would add a separate event OnClosing that is fired first. OnClosed would be fired after that if the close really happened