MudOverlay: Remove `OnClick` callback about mudblazor HOT 8 OPEN

We just make OnClick obsolete with a message to use VisibleChanged instead. After it has been obsoleted for a time we can remove it in a future major version.

from mudblazor.

I'm planning to replace all other usages of OnClick with the VisibleChanged event like I did in #8914 in order to avoid subtle bugs. How do we stop people from falling into this same anti-pattern without removing OnClick entirely? @henon @ScarletKuro

from mudblazor.

@henon

/// <summary>
/// Triggers an event callback when the overlay is clicked.
/// </summary>
[Parameter]
[Obsolete("Use VisibleChanged with AutoClose to handle the overlay disappearing in a universal way.")]
public EventCallback<MouseEventArgs> OnClick { get; set; }

Can we make AutoClose true by default? It seems like that's the only way it's actually used in the whole library so it would make sense if we made that switch while moving away from OnClick. The pattern is always "OnClick = make visible false".

from mudblazor.

OK, we can change AutoClose to true by default. Maybe we should think if obsoleting OnClick is even necessary. What if someone wants to use AutoClose="false" and doesn't want to close on click but still get the click event to do something, i.e. show a message or whatever? Once we remove it that wouldn't be possible any longer.

As for prevending people to use OnClick instead of AutoClose we can also just show a warning box in the docs.

from mudblazor.

Or maybe we add an OnClosing eventcallback with class CloseEventArgs which contain a writable property bool Cancel which the user can use to prevent or allow the overlay from closing based on their business logic? In the overlay we await the OnClosing eventcallback and based on the value of Cancel update Visible accordingly?

WPF has such logic, would this be ok for Blazor as well @ScarletKuro ?

from mudblazor.

ping @ScarletKuro

from mudblazor.

I'm going to put this on hold due to the potential for issues to pop up and some components (MudDrawer) really need further refactoring which will take more time.

from mudblazor.

WPF has such logic, would this be ok for Blazor as well @ScarletKuro ?

Yeah

from mudblazor.