Skip to content

Commit 370f3d2

Browse files
committed
PR review fixes
1 parent 87a6a78 commit 370f3d2

File tree

3 files changed

+63
-53
lines changed

3 files changed

+63
-53
lines changed

App/App.xaml.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
namespace Coder.Desktop.App;
2929

30-
public partial class App : Application, IDispatcherQueueManager
30+
public partial class App : Application, IDispatcherQueueManager, INotificationHandler
3131
{
3232
private const string MutagenControllerConfigSection = "MutagenController";
3333
private const string UpdaterConfigSection = "Updater";
@@ -91,6 +91,7 @@ public App()
9191
services.AddSingleton<IAgentApiClientFactory, AgentApiClientFactory>();
9292

9393
services.AddSingleton<IDispatcherQueueManager>(_ => this);
94+
services.AddSingleton<INotificationHandler>(_ => this);
9495
services.AddSingleton<ICredentialBackend>(_ =>
9596
new WindowsCredentialBackend(WindowsCredentialBackend.CoderCredentialsTargetName));
9697
services.AddSingleton<ICredentialManager, CredentialManager>();
@@ -335,4 +336,13 @@ public void RunInUiThread(DispatcherQueueHandler action)
335336
}
336337
dispatcherQueue.TryEnqueue(action);
337338
}
339+
340+
public void HandleNotificationActivation(IDictionary<string, string> args)
341+
{
342+
var app = (App)Current;
343+
if (app != null && app.TrayWindow != null)
344+
{
345+
app.TrayWindow.Tray_Open();
346+
}
347+
}
338348
}

App/Services/UserNotifier.cs

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,38 @@ public interface IUserNotifier : INotificationHandler, IAsyncDisposable
2121
public void UnregisterHandler(string name);
2222

2323
public Task ShowErrorNotification(string title, string message, CancellationToken ct = default);
24+
/// <summary>
25+
/// This method allows to display a Windows-native notification with an action defined in
26+
/// <paramref name="handlerName"/> and provided <paramref name="args"/>.
27+
/// </summary>
28+
/// <param name="title">Title of the notification.</param>
29+
/// <param name="message">Message to be displayed in the notification body.</param>
30+
/// <param name="handlerName">Handler should be e.g. <c>nameof(Handler)</c> where <c>Handler</c>
31+
/// implements <see cref="Coder.Desktop.App.Services.INotificationHandler" />.
32+
/// If handler is <c>null</c> the action will open Coder Desktop.</param>
33+
/// <param name="args">Arguments to be provided to the handler when executing the action.</param>
2434
public Task ShowActionNotification(string title, string message, string? handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default);
2535
}
2636

2737
public class UserNotifier : IUserNotifier
2838
{
2939
private const string CoderNotificationHandler = "CoderNotificationHandler";
40+
private const string DefaultNotificationHandler = "DefaultNotificationHandler";
3041

3142
private readonly AppNotificationManager _notificationManager = AppNotificationManager.Default;
3243
private readonly ILogger<UserNotifier> _logger;
3344
private readonly IDispatcherQueueManager _dispatcherQueueManager;
3445

3546
private ConcurrentDictionary<string, INotificationHandler> Handlers { get; } = new();
3647

37-
public UserNotifier(ILogger<UserNotifier> logger, IDispatcherQueueManager dispatcherQueueManager)
48+
public UserNotifier(ILogger<UserNotifier> logger, IDispatcherQueueManager dispatcherQueueManager,
49+
INotificationHandler notificationHandler)
3850
{
3951
_logger = logger;
4052
_dispatcherQueueManager = dispatcherQueueManager;
41-
Handlers.TryAdd(nameof(DefaultNotificationHandler), new DefaultNotificationHandler());
53+
var defaultHandlerAdded = Handlers.TryAdd(DefaultNotificationHandler, notificationHandler);
54+
if (!defaultHandlerAdded)
55+
throw new Exception($"UserNotifier failed to be initialized with {nameof(DefaultNotificationHandler)}");
4256
}
4357

4458
public ValueTask DisposeAsync()
@@ -60,6 +74,8 @@ public void RegisterHandler(string name, INotificationHandler handler)
6074

6175
public void UnregisterHandler(string name)
6276
{
77+
if (name == nameof(DefaultNotificationHandler))
78+
throw new InvalidOperationException($"You cannot remove '{name}'.");
6379
if (!Handlers.TryRemove(name, out _))
6480
throw new InvalidOperationException($"No handler with the name '{name}' is registered.");
6581
}
@@ -74,16 +90,11 @@ public Task ShowErrorNotification(string title, string message, CancellationToke
7490
public Task ShowActionNotification(string title, string message, string? handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default)
7591
{
7692
if (handlerName == null)
77-
{
78-
// Use default handler if no handler name is provided
79-
handlerName = nameof(DefaultNotificationHandler);
80-
}
81-
else
82-
{
83-
if (!Handlers.TryGetValue(handlerName, out _))
84-
throw new InvalidOperationException($"No action handler with the name '{handlerName}' is registered. Use null for default");
85-
}
93+
handlerName = nameof(DefaultNotificationHandler); // Use default handler if no handler name is provided
8694

95+
if (!Handlers.TryGetValue(handlerName, out _))
96+
throw new InvalidOperationException($"No action handler with the name '{handlerName}' is registered.");
97+
8798
var builder = new AppNotificationBuilder()
8899
.AddText(title)
89100
.AddText(message)
@@ -125,15 +136,3 @@ public void HandleNotificationActivation(IDictionary<string, string> args)
125136
});
126137
}
127138
}
128-
129-
public class DefaultNotificationHandler : INotificationHandler
130-
{
131-
public void HandleNotificationActivation(IDictionary<string, string> _)
132-
{
133-
var app = (App)Microsoft.UI.Xaml.Application.Current;
134-
if (app != null && app.TrayWindow != null)
135-
{
136-
app.TrayWindow.Tray_Open();
137-
}
138-
}
139-
}

App/Views/TrayWindow.xaml.cs

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.UI.Windowing;
1010
using Microsoft.UI.Xaml;
1111
using Microsoft.UI.Xaml.Controls;
12+
using Microsoft.UI.Xaml.Documents;
1213
using Microsoft.UI.Xaml.Media.Animation;
1314
using System;
1415
using System.Collections.Generic;
@@ -35,8 +36,8 @@ public sealed partial class TrayWindow : Window
3536
private int _lastWindowHeight;
3637
private Storyboard? _currentSb;
3738

38-
private VpnLifecycle prevVpnLifecycle = VpnLifecycle.Stopped;
39-
private RpcLifecycle prevRpcLifecycle = RpcLifecycle.Disconnected;
39+
private VpnLifecycle curVpnLifecycle = VpnLifecycle.Stopped;
40+
private RpcLifecycle curRpcLifecycle = RpcLifecycle.Disconnected;
4041

4142
private readonly IRpcController _rpcController;
4243
private readonly ICredentialManager _credentialManager;
@@ -151,54 +152,54 @@ private void SetPageByState(RpcModel rpcModel, CredentialModel credentialModel,
151152
}
152153
}
153154

154-
private void NotifyUser(RpcModel rpcModel)
155+
private void MaybeNotifyUser(RpcModel rpcModel)
155156
{
156157
// This method is called when the state changes, but we don't want to notify
157158
// the user if the state hasn't changed.
158-
var isRpcLifecycleChanged = rpcModel.RpcLifecycle == RpcLifecycle.Disconnected && prevRpcLifecycle != rpcModel.RpcLifecycle;
159-
var isVpnLifecycleChanged = (rpcModel.VpnLifecycle == VpnLifecycle.Started || rpcModel.VpnLifecycle == VpnLifecycle.Stopped) && prevVpnLifecycle != rpcModel.VpnLifecycle;
159+
var isRpcLifecycleChanged = rpcModel.RpcLifecycle == RpcLifecycle.Disconnected && curRpcLifecycle != rpcModel.RpcLifecycle;
160+
var isVpnLifecycleChanged = (rpcModel.VpnLifecycle == VpnLifecycle.Started || rpcModel.VpnLifecycle == VpnLifecycle.Stopped) && curVpnLifecycle != rpcModel.VpnLifecycle;
160161

161162
if (!isRpcLifecycleChanged && !isVpnLifecycleChanged)
162163
{
163164
return;
164165
}
165-
var message = string.Empty;
166-
// Compose the message based on the lifecycle changes
167-
if (isRpcLifecycleChanged)
168-
message += rpcModel.RpcLifecycle switch
169-
{
170-
RpcLifecycle.Disconnected => "Disconnected from Coder background service.",
171-
_ => "" // This will never be hit.
172-
};
173166

174-
if (message.Length > 0 && isVpnLifecycleChanged)
175-
message += " ";
167+
var oldRpcLifeycle = curRpcLifecycle;
168+
var oldVpnLifecycle = curVpnLifecycle;
169+
curRpcLifecycle = rpcModel.RpcLifecycle;
170+
curVpnLifecycle = rpcModel.VpnLifecycle;
176171

177-
if (isVpnLifecycleChanged)
178-
message += rpcModel.VpnLifecycle switch
179-
{
180-
VpnLifecycle.Started => "Coder Connect started.",
181-
VpnLifecycle.Stopped => "Coder Connect stopped.",
182-
_ => "" // This will never be hit.
183-
};
172+
var messages = new List<string>();
184173

185-
// Save state for the next notification check
186-
prevRpcLifecycle = rpcModel.RpcLifecycle;
187-
prevVpnLifecycle = rpcModel.VpnLifecycle;
174+
if (oldRpcLifeycle != RpcLifecycle.Disconnected && curRpcLifecycle == RpcLifecycle.Disconnected)
175+
{
176+
messages.Add("Disconnected from Coder background service.");
177+
}
188178

189-
if (_aw.IsVisible)
179+
if (oldVpnLifecycle != curVpnLifecycle)
190180
{
191-
return; // No need to notify if the window is not visible.
181+
switch (curVpnLifecycle)
182+
{
183+
case VpnLifecycle.Started:
184+
messages.Add("Coder Connect started.");
185+
break;
186+
case VpnLifecycle.Stopped:
187+
messages.Add("Coder Connect stopped.");
188+
break;
189+
}
192190
}
193191

194-
// Trigger notification
192+
if (messages.Count == 0) return;
193+
if (_aw.IsVisible) return;
194+
195+
var message = string.Join(" ", messages);
195196
_userNotifier.ShowActionNotification(message, string.Empty, null, null, CancellationToken.None);
196197
}
197198

198199
private void RpcController_StateChanged(object? _, RpcModel model)
199200
{
200201
SetPageByState(model, _credentialManager.GetCachedCredentials(), _syncSessionController.GetState());
201-
NotifyUser(model);
202+
MaybeNotifyUser(model);
202203
}
203204

204205
private void CredentialManager_CredentialsChanged(object? _, CredentialModel model)

0 commit comments

Comments
 (0)