-
Notifications
You must be signed in to change notification settings - Fork 5
chore: add connect/disconnect notifications on Coder Connect #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7ff7914
08d523f
9e12405
ea7d39b
87a6a78
370f3d2
2411356
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
using System.Collections.Generic; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Coder.Desktop.App.Views; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.Windows.AppNotifications; | ||
using Microsoft.Windows.AppNotifications.Builder; | ||
|
@@ -20,17 +21,40 @@ public interface IUserNotifier : INotificationHandler, IAsyncDisposable | |
public void UnregisterHandler(string name); | ||
|
||
public Task ShowErrorNotification(string title, string message, CancellationToken ct = default); | ||
public Task ShowActionNotification(string title, string message, string handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default); | ||
/// <summary> | ||
/// This method allows to display a Windows-native notification with an action defined in | ||
/// <paramref name="handlerName"/> and provided <paramref name="args"/>. | ||
/// </summary> | ||
/// <param name="title">Title of the notification.</param> | ||
/// <param name="message">Message to be displayed in the notification body.</param> | ||
/// <param name="handlerName">Handler should be e.g. <c>nameof(Handler)</c> where <c>Handler</c> | ||
/// implements <see cref="Coder.Desktop.App.Services.INotificationHandler" />. | ||
/// If handler is <c>null</c> the action will open Coder Desktop.</param> | ||
/// <param name="args">Arguments to be provided to the handler when executing the action.</param> | ||
public Task ShowActionNotification(string title, string message, string? handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default); | ||
} | ||
|
||
public class UserNotifier(ILogger<UserNotifier> logger, IDispatcherQueueManager dispatcherQueueManager) : IUserNotifier | ||
public class UserNotifier : IUserNotifier | ||
{ | ||
private const string CoderNotificationHandler = "CoderNotificationHandler"; | ||
private const string DefaultNotificationHandler = "DefaultNotificationHandler"; | ||
|
||
private readonly AppNotificationManager _notificationManager = AppNotificationManager.Default; | ||
private readonly ILogger<UserNotifier> _logger; | ||
private readonly IDispatcherQueueManager _dispatcherQueueManager; | ||
|
||
private ConcurrentDictionary<string, INotificationHandler> Handlers { get; } = new(); | ||
|
||
public UserNotifier(ILogger<UserNotifier> logger, IDispatcherQueueManager dispatcherQueueManager, | ||
INotificationHandler notificationHandler) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should wrap it with a marker type like |
||
{ | ||
_logger = logger; | ||
_dispatcherQueueManager = dispatcherQueueManager; | ||
var defaultHandlerAdded = Handlers.TryAdd(DefaultNotificationHandler, notificationHandler); | ||
if (!defaultHandlerAdded) | ||
throw new Exception($"UserNotifier failed to be initialized with {nameof(DefaultNotificationHandler)}"); | ||
} | ||
|
||
public ValueTask DisposeAsync() | ||
{ | ||
return ValueTask.CompletedTask; | ||
|
@@ -50,6 +74,8 @@ public void RegisterHandler(string name, INotificationHandler handler) | |
|
||
public void UnregisterHandler(string name) | ||
{ | ||
if (name == nameof(DefaultNotificationHandler)) | ||
throw new InvalidOperationException($"You cannot remove '{name}'."); | ||
if (!Handlers.TryRemove(name, out _)) | ||
throw new InvalidOperationException($"No handler with the name '{name}' is registered."); | ||
} | ||
|
@@ -61,8 +87,11 @@ public Task ShowErrorNotification(string title, string message, CancellationToke | |
return Task.CompletedTask; | ||
} | ||
|
||
public Task ShowActionNotification(string title, string message, string handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default) | ||
public Task ShowActionNotification(string title, string message, string? handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default) | ||
{ | ||
if (handlerName == null) | ||
handlerName = nameof(DefaultNotificationHandler); // Use default handler if no handler name is provided | ||
|
||
if (!Handlers.TryGetValue(handlerName, out _)) | ||
throw new InvalidOperationException($"No action handler with the name '{handlerName}' is registered."); | ||
|
||
|
@@ -90,19 +119,19 @@ public void HandleNotificationActivation(IDictionary<string, string> args) | |
|
||
if (!Handlers.TryGetValue(handlerName, out var handler)) | ||
{ | ||
logger.LogWarning("no action handler '{HandlerName}' found for notification activation, ignoring", handlerName); | ||
_logger.LogWarning("no action handler '{HandlerName}' found for notification activation, ignoring", handlerName); | ||
return; | ||
} | ||
|
||
dispatcherQueueManager.RunInUiThread(() => | ||
_dispatcherQueueManager.RunInUiThread(() => | ||
{ | ||
try | ||
{ | ||
handler.HandleNotificationActivation(args); | ||
} | ||
catch (Exception ex) | ||
{ | ||
logger.LogWarning(ex, "could not handle activation for notification with handler '{HandlerName}", handlerName); | ||
_logger.LogWarning(ex, "could not handle activation for notification with handler '{HandlerName}", handlerName); | ||
} | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,12 @@ | |
using Microsoft.UI.Windowing; | ||
using Microsoft.UI.Xaml; | ||
using Microsoft.UI.Xaml.Controls; | ||
using Microsoft.UI.Xaml.Documents; | ||
using Microsoft.UI.Xaml.Media.Animation; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Runtime.InteropServices; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Windows.Graphics; | ||
using Windows.System; | ||
|
@@ -33,17 +36,23 @@ public sealed partial class TrayWindow : Window | |
private int _lastWindowHeight; | ||
private Storyboard? _currentSb; | ||
|
||
private VpnLifecycle curVpnLifecycle = VpnLifecycle.Stopped; | ||
private RpcLifecycle curRpcLifecycle = RpcLifecycle.Disconnected; | ||
|
||
private readonly IRpcController _rpcController; | ||
private readonly ICredentialManager _credentialManager; | ||
private readonly ISyncSessionController _syncSessionController; | ||
private readonly IUpdateController _updateController; | ||
private readonly IUserNotifier _userNotifier; | ||
private readonly TrayWindowLoadingPage _loadingPage; | ||
private readonly TrayWindowDisconnectedPage _disconnectedPage; | ||
private readonly TrayWindowLoginRequiredPage _loginRequiredPage; | ||
private readonly TrayWindowMainPage _mainPage; | ||
|
||
public TrayWindow(IRpcController rpcController, ICredentialManager credentialManager, | ||
public TrayWindow( | ||
IRpcController rpcController, ICredentialManager credentialManager, | ||
ISyncSessionController syncSessionController, IUpdateController updateController, | ||
IUserNotifier userNotifier, | ||
TrayWindowLoadingPage loadingPage, | ||
TrayWindowDisconnectedPage disconnectedPage, TrayWindowLoginRequiredPage loginRequiredPage, | ||
TrayWindowMainPage mainPage) | ||
|
@@ -52,6 +61,7 @@ public TrayWindow(IRpcController rpcController, ICredentialManager credentialMan | |
_credentialManager = credentialManager; | ||
_syncSessionController = syncSessionController; | ||
_updateController = updateController; | ||
_userNotifier = userNotifier; | ||
_loadingPage = loadingPage; | ||
_disconnectedPage = disconnectedPage; | ||
_loginRequiredPage = loginRequiredPage; | ||
|
@@ -142,9 +152,54 @@ private void SetPageByState(RpcModel rpcModel, CredentialModel credentialModel, | |
} | ||
} | ||
|
||
private void MaybeNotifyUser(RpcModel rpcModel) | ||
{ | ||
// This method is called when the state changes, but we don't want to notify | ||
// the user if the state hasn't changed. | ||
var isRpcLifecycleChanged = rpcModel.RpcLifecycle == RpcLifecycle.Disconnected && curRpcLifecycle != rpcModel.RpcLifecycle; | ||
var isVpnLifecycleChanged = (rpcModel.VpnLifecycle == VpnLifecycle.Started || rpcModel.VpnLifecycle == VpnLifecycle.Stopped) && curVpnLifecycle != rpcModel.VpnLifecycle; | ||
|
||
if (!isRpcLifecycleChanged && !isVpnLifecycleChanged) | ||
{ | ||
return; | ||
} | ||
|
||
Comment on lines
+157
to
+166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need any of these lines since they're essentially covered by what's below right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately no, I ignore the intermediate states of the Vpn lifecycle on purpose (and I don't update the prevRpcLifecycle for that reason). |
||
var oldRpcLifeycle = curRpcLifecycle; | ||
var oldVpnLifecycle = curVpnLifecycle; | ||
curRpcLifecycle = rpcModel.RpcLifecycle; | ||
curVpnLifecycle = rpcModel.VpnLifecycle; | ||
|
||
var messages = new List<string>(); | ||
|
||
if (oldRpcLifeycle != RpcLifecycle.Disconnected && curRpcLifecycle == RpcLifecycle.Disconnected) | ||
{ | ||
messages.Add("Disconnected from Coder background service."); | ||
} | ||
|
||
if (oldVpnLifecycle != curVpnLifecycle) | ||
{ | ||
switch (curVpnLifecycle) | ||
{ | ||
case VpnLifecycle.Started: | ||
messages.Add("Coder Connect started."); | ||
break; | ||
case VpnLifecycle.Stopped: | ||
messages.Add("Coder Connect stopped."); | ||
break; | ||
} | ||
} | ||
|
||
if (messages.Count == 0) return; | ||
if (_aw.IsVisible) return; | ||
|
||
var message = string.Join(" ", messages); | ||
_userNotifier.ShowActionNotification(message, string.Empty, null, null, CancellationToken.None); | ||
} | ||
ibetitsmike marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private void RpcController_StateChanged(object? _, RpcModel model) | ||
{ | ||
SetPageByState(model, _credentialManager.GetCachedCredentials(), _syncSessionController.GetState()); | ||
MaybeNotifyUser(model); | ||
} | ||
|
||
private void CredentialManager_CredentialsChanged(object? _, CredentialModel model) | ||
|
@@ -297,7 +352,7 @@ private void Window_Activated(object sender, WindowActivatedEventArgs e) | |
} | ||
|
||
[RelayCommand] | ||
private void Tray_Open() | ||
public void Tray_Open() | ||
deansheather marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
MoveResizeAndActivate(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this just be
if (TrayWindow != null) TrayWindow.Tray_Open();
?