-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: add agent status to tray app #21
Conversation
- Adds agent and stopped workspace statuses to the tray app - The Vpn.Service.Manager now tracks the current list of workspaces and agents from the tunnel - Deletes remnants of the old Package - Moves App to be completely unpackaged
@@ -61,6 +70,14 @@ public TrayWindowViewModel(IRpcController rpcController, ICredentialManager cred | |||
|
|||
private void UpdateFromRpcModel(RpcModel rpcModel) | |||
{ | |||
// Ensure we're on the UI thread. |
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.
It seems backward to me that the view model needs to understand the thread the view runs on to not break shit, and it seems I'm not the only one. microsoft/microsoft-ui-xaml#2795
But, this is apparently what needs to be done. No changes here, I'm just grumpy.
}); | ||
} | ||
|
||
// For every workspace that doesn't have an agent, add a dummy agent. |
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.
I think we shouldn't create dummy agents --- the VPN should be giving us the full agent information. Maybe just log that we got a workspace that appeared not to have any agents?
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.
Stopped workspaces fall into this category. We want them to appear as grayed out "agents", but stopped workspaces don't have any agents.
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.
The alternative way would be to render workspaces with no agents separately in the UI, but this results in us having to duplicate a bunch of XAML for rendering which is worse
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.
I think it's fine to create grayed out view models for stopped workspaces, but
a) let's check that they are actually stopped, stopping etc -- we get this over the protocol
b) can you make the comment reflect what we are doing -- maybe call them stopped instead of dummy.
I think we need to have a larger design session about how to present this information in a clear way. Our UX is very much conflating workspaces and agents right now, and I'm concerned it's going to get confusing.
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.
Oh I didn't see this comment before merge, I'll fix this in my next PR.
I'll fix build soon |
Closes #5