revise recent projects flow to be less confusing #464
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,8 @@ import com.coder.gateway.services.CoderRestClientService | |
| import com.coder.gateway.services.CoderSettingsService | ||
| import com.coder.gateway.util.humanizeConnectionError | ||
| import com.coder.gateway.util.toURL | ||
| import com.coder.gateway.util.withoutNull | ||
| import com.intellij.icons.AllIcons | ||
| import com.intellij.openapi.Disposable | ||
| import com.intellij.openapi.actionSystem.AnActionEvent | ||
| import com.intellij.openapi.application.ModalityState | ||
|
|
@@ -56,6 +54,7 @@ import kotlinx.coroutines.delay | |
| import kotlinx.coroutines.isActive | ||
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.withContext | ||
| import java.awt.Color | ||
| import java.awt.Component | ||
| import java.awt.Dimension | ||
| import java.util.Locale | ||
|
|
@@ -175,15 +174,15 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback: | |
| val workspaceWithAgent = deployment?.items?.firstOrNull { it.workspace.name == workspaceName } | ||
| val status = | ||
| if (deploymentError != null) { | ||
| Triple(UIUtil.getErrorForeground(), deploymentError, UIUtil.getBalloonErrorIcon()) | ||
| } else if (workspaceWithAgent != null) { | ||
| Triple( | ||
| workspaceWithAgent.status.statusColor(), | ||
| workspaceWithAgent.status.description, | ||
| null, | ||
| ) | ||
| } else { | ||
| Triple(UIUtil.getContextHelpForeground(), "Querying workspace status...", AnimatedIcon.Default()) | ||
| } | ||
| val gap = | ||
| if (top) { | ||
|
|
@@ -193,11 +192,6 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback: | |
| TopGap.MEDIUM | ||
| } | ||
| row { | ||
| label(workspaceName).applyToComponent { | ||
| font = JBFont.h3().asBold() | ||
| }.align(AlignX.LEFT).gap(RightGap.SMALL) | ||
|
|
@@ -206,94 +200,48 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback: | |
| font = ComponentPanelBuilder.getCommentFont(font) | ||
| } | ||
| label("").resizableColumn().align(AlignX.FILL) | ||
| }.topGap(gap) | ||
|
|
||
| val enableLinks = listOf(WorkspaceStatus.STOPPED, WorkspaceStatus.CANCELED, WorkspaceStatus.FAILED, WorkspaceStatus.STARTING, WorkspaceStatus.RUNNING).contains(workspaceWithAgent?.workspace?.latestBuild?.status) | ||
| val inLoadingState = listOf(WorkspaceStatus.STARTING, WorkspaceStatus.CANCELING, WorkspaceStatus.DELETING, WorkspaceStatus.STOPPING).contains(workspaceWithAgent?.workspace?.latestBuild?.status) | ||
|
|
||
| // We only display an API error on the first workspace rather than duplicating it on each workspace. | ||
| if (deploymentError == null || showError) { | ||
| row { | ||
| if (inLoadingState) { | ||
| icon(AnimatedIcon.Default()) | ||
| } | ||
|
There was a problem hiding this comment. Not at all a blocker and completely optional, more just idly musing, but I wonder if we want to turn the pair back into a triple and put a nullable icon in there, which would let us also show a spinner while the query is running as well (like it used to) and bring back the error icon for API errors (if we want, honestly the red color is probably sufficient). There was a problem hiding this comment. I think adding the error icon back is a good idea and fits in this PR :) |
||
| if (status.third != null) { | ||
| icon(status.third!!) | ||
| } | ||
|
There was a problem hiding this comment. idk if this is more Kotlin-y but I think you can also do something like this over the assertion: Admittedly untested though There was a problem hiding this comment. I low key hate that they named a function There was a problem hiding this comment.
I think the main issue with assertions is that they bypass the safety provided to you by the language, which feels dangerous for an unclear benefit. If you see a There was a problem hiding this comment. Yeah I meant use withoutNull inside the if check, but you're totally right the There was a problem hiding this comment. ahaha yeah and I swear I have to look it up every time because I always forget it is called |
||
| label("<html><body style='width:350px;'>" + status.second + "</html>").applyToComponent { | ||
| foreground = status.first | ||
| } | ||
| } | ||
| } | ||
|
|
||
| connections.forEach { workspaceProjectIDE -> | ||
| row { | ||
| icon(workspaceProjectIDE.ideProduct.icon) | ||
| if (enableLinks) { | ||
| cell( | ||
| ActionLink(workspaceProjectIDE.projectPathDisplay) { | ||
| withoutNull(deployment?.client, workspaceWithAgent?.workspace) { client, workspace -> | ||
| CoderRemoteConnectionHandle().connect { | ||
| if (listOf(WorkspaceStatus.STOPPED, WorkspaceStatus.CANCELED, WorkspaceStatus.FAILED).contains(workspace.latestBuild.status)) { | ||
| client.startWorkspace(workspace) | ||
|
Comment on lines
+233
to
+234
There was a problem hiding this comment. I opened #465 for future improvement |
||
| } | ||
| workspaceProjectIDE | ||
| } | ||
| GatewayUI.getInstance().reset() | ||
| } | ||
| }, | ||
| ) | ||
| } else { | ||
| label(workspaceProjectIDE.projectPathDisplay).applyToComponent { | ||
| foreground = Color.GRAY | ||
| } | ||
| } | ||
| label("").resizableColumn().align(AlignX.FILL) | ||
| label(workspaceProjectIDE.ideName).applyToComponent { | ||
| foreground = JBUI.CurrentTheme.ContextHelp.FOREGROUND | ||
|
|
||
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 might want to add this check back or something similar to it. It is poorly named but basically it was making sure we only display an API error on the first workspace rather than duplicating it on each workspace (it was pretty noisy, especially since the API errors can get long).
For example, say you have 5 workspaces and your token expires, you see a message about being unauthorized and to check your token 5 times on the page.
Or we could group workspaces under a deployment heading and show the API error under the deployment heading, but that might be out of scope for this PR.
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 looks like atm the moment I actually have this inside the connections.foEach loop (which makes me think with multiple WorkpaceProjectIDE combos it would render multiple times). I didn't notice because I was visually testing with one recent connection.
I'll pull it out of the loop and add the check.
^^^ My adhd is going to quickly become a driving force for building out E2E suites.