Fix Avalonia notification in 2 steps only without strong coupling - #85
Fix Avalonia notification in 2 steps only without strong coupling#85maxensas wants to merge 4 commits into
Conversation
…rst show call) + UseOverlayWindow as default like WPF imp (debatable) + automatically set the main window as host if instanciated + Fix default position without parent window.
|
Hi @maxensas, thanks for the PR — the direction is right (decouple consumers from 1. Side effects in the manager constructorpublic AvaloniaNotificationManager(INotificationConfiguration config, INotificationEventService events)
{
...
var appLifetime = (IClassicDesktopStyleApplicationLifetime)Application.Current!.ApplicationLifetime!;
var win = appLifetime?.MainWindow;
if (win is not null)
{
SetHost(win);
}
UseOverlayWindow = true;
}Problems:
Please move the auto-host logic out of the constructor. Either:
The 2. Sample: drop the static service providerprivate static IServiceProvider _serviceProvider;Please make it an instance field, and cache the resolved services ( 3. Style
4. Stray TODO// this pattern can be used to refactor for action/func and async TasksPlease remove this comment (or open a follow-up issue if you want to track the refactor). Once these are addressed I'm happy to merge. Thanks again! |
|
All right, thanks for the fast review i will polish this when i catch up some time. |
|
Thanks for the quick turnaround @maxensas — the WIP commit covers most of it:
One remaining thing to fix before merge:
|
|
Hi @Platonenkov, regarding the initialization method, I knew I was making a mistake, hence the explicit WIP name of the commit. Better ideas after a good night's sleep :) Apologies if I took some freedom with the refactoring regarding the dispatcher access. My PR now seems consistent with the prerequisites. Tell me if it doesnt. For desktop apps, I usually use DI with MVVM, hence my following suggestion. It's generally best to set up services upstream after app launch and have a single provider per application, hence the static one. Since this is a sample project, we can be flexible, but if you want to manage multiple views in a next iteration, you'll need to refactor accordingly. Thanks for your time ! |
|
Hi @maxensas — to make this concrete, I prepared the remaining fixes as a patch against the current PR head ( What the patch changes
Verification
(No Avalonia test project in the repo, so these fixes weren't unit-tested directly — happy to follow up with an PatchFrom ba6b8964d91074321c522f2874803673fdd87e8a Mon Sep 17 00:00:00 2001
From: Aleksandr Platonenkov <platonenkov87@gmail.com>
Date: Mon, 8 Jun 2026 12:32:54 -0300
Subject: [PATCH] fix(avalonia): address review findings in
AvaloniaNotificationManager and ActionExtensions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* SetHost: split overloaded SetHost(TopLevel, bool init) into a clean public
SetHost(TopLevel) and a private EnsureHostInitialized() — removes the public
API magic flag and stops _initializedHost from latching when TopLevel is
not yet available (used to silently disable the manager forever if first
Show() ran before MainWindow was assigned).
* EnsureHostInitialized: guarded by a lock to close the race where two
concurrent first Show() calls could create two AvaloniaNotificationHost
instances and orphan the first one's cards.
* Position: setter now persists a pending value so configuring
manager.Position before the first Show() is no longer silently lost when
the lazy host is created later; SetHost and EnsureOverlayWindow honor it.
* Dismiss / DismissAll: routed through a new ExistingHost helper that does
NOT trigger lazy init nor EnsureOverlayWindow — a defensive DismissAll()
no longer pops an empty topmost overlay window.
* EnsureOverlayWindow: BindToParent is null-safe (early-returns on null),
so it is now called unconditionally — matches the unconditional
ApplyPositionOnScreen call and avoids the case where overlay was
positioned but never bound when _parentWindow was null.
* InvokeOnUiThreadAsync: now returns a Task and propagates exceptions via
TaskCompletionSource instead of the previous fire-and-forget
Dispatcher.UIThread.Post(async () => await action()) which silently
swallowed any exception thrown by the Func<Task> as an unobserved task.
---
.../AvaloniaNotificationManager.cs | 110 +++++++++++++-----
.../Extensions/ActionExtensions.cs | 35 +++++-
2 files changed, 112 insertions(+), 33 deletions(-)
diff --git a/src/Notification.Avalonia/AvaloniaNotificationManager.cs b/src/Notification.Avalonia/AvaloniaNotificationManager.cs
index ccaa87c..7059b67 100644
--- a/src/Notification.Avalonia/AvaloniaNotificationManager.cs
+++ b/src/Notification.Avalonia/AvaloniaNotificationManager.cs
@@ -28,6 +28,8 @@ namespace Notification.Avalonia
private NotificationOverlayWindow _overlayWindow;
private bool _useOverlayWindow;
private bool _initializedHost;
+ private NotificationPosition? _pendingPosition;
+ private readonly object _initLock = new object();
/// <summary>
/// Initializes a new instance of the <see cref="AvaloniaNotificationManager"/> class.
@@ -52,27 +54,53 @@ namespace Notification.Avalonia
/// <summary>
/// Attach the notification manager to a TopLevel (Window or similar).
- /// Must be called before showing notifications.
+ /// Optional — called automatically on first show via auto-discovery
+ /// (<see cref="Application.Current"/>'s main window/view).
/// </summary>
- public void SetHost(TopLevel host, bool init = false)
+ public void SetHost(TopLevel host)
{
- _initializedHost = true;
- if (init)
- {
- host = TopLevel;
- if (host is null)
- {
- return;
- }
- }
-
+ if (host is null)
+ throw new ArgumentNullException(nameof(host));
+
_parentWindow = host as Window;
+ NotificationPosition position =
+ _pendingPosition ?? _config?.MessagePosition ?? NotificationPosition.BottomRight;
+
_host = new AvaloniaNotificationHost(host)
{
MaxItems = (int)(_config?.MaxOverlayWindowCount ?? 5),
- Position = _config?.MessagePosition ?? NotificationPosition.BottomRight
+ Position = position
};
+
+ _initializedHost = true;
+ }
+
+ /// <summary>
+ /// Lazily discovers the main window/view via <see cref="Application.Current"/>
+ /// and attaches the manager to it. Idempotent and side-effect-free if the
+ /// platform top-level is not yet available — retries on next access.
+ /// </summary>
+ private void EnsureHostInitialized()
+ {
+ if (_initializedHost)
+ return;
+
+ lock (_initLock)
+ {
+ if (_initializedHost)
+ return;
+
+ TopLevel topLevel = TopLevel;
+ if (topLevel is null)
+ {
+ // Do not latch the flag — retry on next access once the
+ // application top-level becomes available.
+ return;
+ }
+
+ SetHost(topLevel);
+ }
}
/// <summary>
@@ -111,10 +139,7 @@ namespace Notification.Avalonia
{
get
{
- if (!_initializedHost)
- {
- SetHost(null, init: true);
- }
+ EnsureHostInitialized();
if (_useOverlayWindow)
{
EnsureOverlayWindow();
@@ -124,6 +149,21 @@ namespace Notification.Avalonia
}
}
+ /// <summary>
+ /// Returns the currently active host WITHOUT triggering lazy init or
+ /// creating an overlay window. Used by Dismiss/DismissAll so a no-op
+ /// dismiss cannot pop an empty overlay window.
+ /// </summary>
+ private AvaloniaNotificationHost ExistingHost
+ {
+ get
+ {
+ if (_useOverlayWindow)
+ return _overlayWindow?.Host;
+ return _host;
+ }
+ }
+
/// <summary>
/// Gets the top Level Avalonia control, platform agnostic (desktop or mobile/browser).
/// </summary>
@@ -150,12 +190,19 @@ namespace Notification.Avalonia
{
get
{
- AvaloniaNotificationHost host = ActiveHost;
- return host?.Position ?? NotificationPosition.BottomRight;
+ if (_host != null)
+ return _host.Position;
+ if (_overlayWindow?.Host != null)
+ return _overlayWindow.Host.Position;
+ return _pendingPosition ?? _config?.MessagePosition ?? NotificationPosition.BottomRight;
}
set
{
- // Apply to both hosts so position is preserved when switching modes
+ // Remember the user's choice so it survives even if the position
+ // is set BEFORE the first Show() (i.e. before lazy host init runs).
+ _pendingPosition = value;
+
+ // Apply to both hosts so position is preserved when switching modes.
if (_host != null)
_host.Position = value;
if (_overlayWindow?.Host != null)
@@ -282,21 +329,24 @@ namespace Notification.Avalonia
}
/// <summary>
- /// Dismiss a notification by ID.
+ /// Dismiss a notification by ID. No-op if no notifications have been shown
+ /// yet — does NOT trigger lazy host initialization or create an overlay window.
/// </summary>
public void Dismiss(Guid notificationId)
{
- ActiveHost?.Close(notificationId);
+ ExistingHost?.Close(notificationId);
_events?.Raise(new NotificationLifecycleEventArgs(
notificationId, NotificationLifecycleStage.Dismissed, null, null));
}
/// <summary>
- /// Dismiss all active notifications.
+ /// Dismiss all active notifications. No-op if no notifications have been
+ /// shown yet — does NOT trigger lazy host initialization or create an
+ /// overlay window.
/// </summary>
public void DismissAll()
{
- ActiveHost?.CloseAll();
+ ExistingHost?.CloseAll();
}
private void EnsureOverlayWindow()
@@ -309,7 +359,11 @@ namespace Notification.Avalonia
if (_overlayWindow != null)
return;
- NotificationPosition position = _host?.Position ?? _config?.MessagePosition ?? NotificationPosition.BottomRight;
+ NotificationPosition position =
+ _pendingPosition
+ ?? _host?.Position
+ ?? _config?.MessagePosition
+ ?? NotificationPosition.BottomRight;
_overlayWindow = new NotificationOverlayWindow();
_overlayWindow.Host.MaxItems = (int)(_config?.MaxOverlayWindowCount ?? 5);
@@ -321,10 +375,8 @@ namespace Notification.Avalonia
};
_overlayWindow.ApplyPositionOnScreen(_parentWindow, position);
- if (_parentWindow != null)
- {
- _overlayWindow.BindToParent(_parentWindow);
- }
+ // BindToParent is null-safe (early-returns on null) — call unconditionally.
+ _overlayWindow.BindToParent(_parentWindow);
_overlayWindow.Show();
}).InvokeOnUiThread();
diff --git a/src/Notification.Avalonia/Extensions/ActionExtensions.cs b/src/Notification.Avalonia/Extensions/ActionExtensions.cs
index e7fd38a..f51a79f 100644
--- a/src/Notification.Avalonia/Extensions/ActionExtensions.cs
+++ b/src/Notification.Avalonia/Extensions/ActionExtensions.cs
@@ -28,16 +28,43 @@ public static class ActionExtensions
}
/// <summary>
- /// Executes the asynchronous action on the UI thread.
+ /// Executes the asynchronous action on the UI thread and returns a Task
+ /// that completes (or faults) when the action finishes. Exceptions are
+ /// propagated to the caller via the returned Task instead of being
+ /// silently swallowed as unobserved task exceptions.
/// </summary>
- public static void InvokeOnUiThreadAsync(this Func<Task> action)
+ public static Task InvokeOnUiThreadAsync(this Func<Task> action)
{
if (action is null)
- return;
+ return Task.CompletedTask;
+
+ if (Dispatcher.UIThread.CheckAccess())
+ {
+ try
+ {
+ return action() ?? Task.CompletedTask;
+ }
+ catch (Exception ex)
+ {
+ return Task.FromException(ex);
+ }
+ }
+ TaskCompletionSource<object> tcs = new TaskCompletionSource<object>();
Dispatcher.UIThread.Post(async () =>
{
- await action();
+ try
+ {
+ Task inner = action();
+ if (inner != null)
+ await inner.ConfigureAwait(false);
+ tcs.TrySetResult(null);
+ }
+ catch (Exception ex)
+ {
+ tcs.TrySetException(ex);
+ }
});
+ return tcs.Task;
}
}
--
2.53.0.windows.3 |
Following up on issue #82, I tried using Avalonia notifications, but it didn't work.
Rather than opening a new issue to explain what goes wrong, I found solutions to several issues I encountered.
Step 1 and 2 for use case.
we shouldn't know anything about manager that add a framework dependency if called outside..
This also fix my implementation of your package in my Xiletrade project :
https://github.com/maxensas/xiletrade/blob/dev/src/Xiletrade.UI.Avalonia/Program.cs
This allow _overlayWindow to be instancied when calling Show method (it works because already in UI thread but would be better to await using Task)