Refractor WindowNotificationManager

[timunie]

What does the pull request do?

Improves the WindowNotificationManager a little bit:

• Allow usage from XAML
• Allow any Visual as host, so it can be added into UserControl
• Added an overload to .Show to allow more customization
• Added ability to specify Card-classes (more customization)
• Don't need to wait for control to load before .Show can be called

What is the current behavior?

WindowNotificationManager is more limited

What is the updated/expected behavior with this PR?

Allow more flexibility for low cost

How was the solution implemented (if it's not obvious)?

• Added DirectProperty called Host to have finer control over the Host registration
• added overload to Show with more options

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation -> Should be done once PR is accepted

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #12134
Fixes #4462

[MrJul]

The CI reports several binary breaking changes, those likely won't be accepted in 11.x.

• WindowNotificationManager(TopLevel?) has been removed (the parameter has been changed to Visual?).
  Suggestion: keep the old WindowNotificationManager(TopLevel?) and the new parameterless constructor. I don't think WindowNotificationManager(Visual? host) is needed at all since you added a settable Host property that does the same thing.
  WindowNotificationManager(TopLevel?) might be made obsolete instead. (I don't know if that's accepted: it's a source breaking change if you having warnings as errors enabled, but at least it doesn't break binary compatibility.)

• IManagedNotificationManager.Show has new parameters: even if they're optional, the old overload must be absolutely be kept. Optional parameters are a source concept, existing binaries will still search for an overload taking one parameter and won't find it.
  Technically adding a new overload to an interface is also a breaking change in itself, but since it's marked with NotClientImplementable it should be fine.
  Suggestion: I'm not sure what this new overload enables that wasn't possible before? All the parameters should probably be moved to a class so we don't break things if we need to add a new notification parameter. But we already have exactly that: the Notification class, accepted by Show().

I also wonder about the NotificationType property added to NotificationCard: why specifically this one and not all the other properties from INotification (Title, Expiration, etc.)?
In my opinion, either everything is in INotification (current), or everything is in NotificationCard, but having all on the notification and one property on the card seems a bit wonky to me.

[timunie]

@MrJul it wasn't meant to be a breaking change. I thought it won't be as it allowed more things than before, but I was wrong. Will try to fix those breaking changes.

I also wonder about the NotificationType property added to NotificationCard: why specifically this one and not all the other properties from INotification (Title, Expiration, etc.)?
In my opinion, either everything is in INotification (current), or everything is in NotificationCard, but having all on the notification and one property on the card seems a bit wonky to me.

The reason is, that INotification is not (client)implementable anymore. Why all other things can be adjusted easily, the NotificationType cannot. So I added it as a property in this case. I could also add the other propeties (I really like the idea) but I want to have the 👍 from @danwalmsley first.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0038042-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0038244-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[timunie]

Updated the PR. Would be nice if anyone can try and give me some feedback.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0038942-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0038961-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[maxkatz6]

@timunie

Allow any Visual as host, so it can be added into UserControl

It doesn't seem like it works this way.
When Host is not a top-level, code still attempts to get AdornerLayer from it, which will traverse to the same top level adorner level.
Also, defining WindowNotificationManager from the XAML and then specifying Host different from the Parent will cause an error, as WindowNotificationManager was already attached to the XAML parent, and cannot be detached to the Host. Not sure how it can be avoided with this API.

My idea is:

1. Remove Host property, but keep WindowNotificationManager(TopLevel) constructor. This way it's still possible to create WindowNotificationManager from the TopLevel from C# code, but it won't be possible to mistakenly cause double parenting from XAML. As XAML will use WindowNotificationManager() ctor without any host (it will be automatically hosted inside of the Parent).

Don't need to wait for control to load before .Show can be called

It still doesn't work if WindowNotificationManager was added from XAML, but Parent isn't attached to the root when we call this method. But I would say this behavior is expected.

Also, less related to this PR changes, but other nice improvements we can do in 11.x:
2. Make Notification properties settable, so it can be used from XAML. Title and Message can be bindable DirectProperties.
3. If Host is a TopLevel, also listen for TopLevel.TemplateApplied event, so we can re-Install WindowNotificationManager when TopLevel template is changed (to fix #4462)

I am going to push these 3 changes. Let me know if it looks good to you.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0038976-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[timunie]

Naming is not perfect anymore with this PR, so we may want to rename this control in 12.0 ... BUT the new functionality is 😍 with the last recent changes from @maxkatz6 , so I'm fine with it.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0039004-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[dhhunter]

Hi, thank you all for all of the great work you do on Avalonia!

I am curious as to when the customizable Show method will be available? I am currently using 11.0.10 and the new customization is necessary for my use case!

[timunie]

@dhhunter in 11.1 it will be available. Beta package is currently in work. However you can also use nightly builds until 11.1 is there.

[dhhunter]

@timunie Awesome! Thank you! How do I retrieve the generated NotificationCard after calling Show now? Previously, I was able to hack it with:

var cards = notificationManager.GetVisualChildren()
    .FirstOrDefault() is ReversibleStackPanel panel
    ? panel.Children.OfType<NotificationCard>()
    : Array.Empty<NotificationCard>();

// Get the first Notification if it exists...
var card = cards.FirstOrDefault();

[timunie]

@dhhunter I'd use Interactions to show Notifications and return the Notification from there. https://github.com/AvaloniaUI/Avalonia.Samples?tab=readme-ov-file#view-interaction-samples