Skip to content

revise recent projects flow to be less confusing #464

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

Merged
merged 11 commits into from
Aug 19, 2024
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.withPath
import com.coder.gateway.util.withoutNull
import com.intellij.icons.AllIcons
import com.intellij.ide.BrowserUtil
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,14 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback:
val workspaceWithAgent = deployment?.items?.firstOrNull { it.workspace.name == workspaceName }
val status =
if (deploymentError != null) {
Triple(UIUtil.getBalloonErrorIcon(), UIUtil.getErrorForeground(), deploymentError)
Pair(UIUtil.getErrorForeground(), deploymentError)
} else if (workspaceWithAgent != null) {
Triple(
workspaceWithAgent.status.icon,
Pair(
workspaceWithAgent.status.statusColor(),
workspaceWithAgent.status.description,
)
} else {
Triple(AnimatedIcon.Default.INSTANCE, UIUtil.getContextHelpForeground(), "Querying workspace status...")
Pair(UIUtil.getContextHelpForeground(), "Querying workspace status...")
}
val gap =
if (top) {
@@ -193,11 +191,6 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback:
TopGap.MEDIUM
}
row {
icon(status.first).applyToComponent {
foreground = status.second
}.align(AlignX.LEFT).gap(RightGap.SMALL).applyToComponent {
size = Dimension(JBUI.scale(16), JBUI.scale(16))
}
label(workspaceName).applyToComponent {
font = JBFont.h3().asBold()
}.align(AlignX.LEFT).gap(RightGap.SMALL)
@@ -206,94 +199,42 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback:
font = ComponentPanelBuilder.getCommentFont(font)
}
label("").resizableColumn().align(AlignX.FILL)
actionButton(
object : DumbAwareAction(
CoderGatewayBundle.message("gateway.connector.recent-connections.start.button.tooltip"),
"",
CoderIcons.RUN,
) {
override fun actionPerformed(e: AnActionEvent) {
withoutNull(workspaceWithAgent?.workspace, deployment?.client) { workspace, client ->
jobs[workspace.id]?.cancel()
jobs[workspace.id] =
cs.launch(ModalityState.current().asContextElement()) {
withContext(Dispatchers.IO) {
try {
client.startWorkspace(workspace)
fetchWorkspaces()
} catch (e: Exception) {
logger.error("Could not start workspace ${workspace.name}", e)
}
}
}
}
}
},
).applyToComponent {
isEnabled =
listOf(
WorkspaceStatus.STOPPED,
WorkspaceStatus.FAILED,
).contains(workspaceWithAgent?.workspace?.latestBuild?.status)
}
.gap(RightGap.SMALL)
actionButton(
object : DumbAwareAction(
CoderGatewayBundle.message("gateway.connector.recent-connections.stop.button.tooltip"),
"",
CoderIcons.STOP,
) {
override fun actionPerformed(e: AnActionEvent) {
withoutNull(workspaceWithAgent?.workspace, deployment?.client) { workspace, client ->
jobs[workspace.id]?.cancel()
jobs[workspace.id] =
cs.launch(ModalityState.current().asContextElement()) {
withContext(Dispatchers.IO) {
try {
client.stopWorkspace(workspace)
fetchWorkspaces()
} catch (e: Exception) {
logger.error("Could not stop workspace ${workspace.name}", e)
}
}
}
}
}
},
).applyToComponent { isEnabled = workspaceWithAgent?.workspace?.latestBuild?.status == WorkspaceStatus.RUNNING }
.gap(RightGap.SMALL)
actionButton(
object : DumbAwareAction(
CoderGatewayBundle.message("gateway.connector.recent-connections.terminal.button.tooltip"),
"",
CoderIcons.OPEN_TERMINAL,
) {
override fun actionPerformed(e: AnActionEvent) {
withoutNull(workspaceWithAgent, deployment?.client) { ws, client ->
val link = client.url.withPath("/me/${ws.name}/terminal")
BrowserUtil.browse(link.toString())
}
}
},
)
}.topGap(gap)
if (deploymentError == null || showError) {
Copy link
Member

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.

Copy link
Collaborator Author

@bcpeinhardt bcpeinhardt Aug 19, 2024

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.


connections.forEach { workspaceProjectIDE ->
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)

row {
// There must be a way to make this properly wrap?
label("<html><body style='width:350px;'>" + status.third + "</html>").applyToComponent {
foreground = status.second
if (inLoadingState) {
icon(AnimatedIcon.Default())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding the error icon back is a good idea and fits in this PR :)

label("<html><body style='width:350px;'>" + status.second + "</html>").applyToComponent {
foreground = status.first
}
}
}
connections.forEach { workspaceProjectIDE ->

row {
icon(workspaceProjectIDE.ideProduct.icon)
cell(
ActionLink(workspaceProjectIDE.projectPathDisplay) {
CoderRemoteConnectionHandle().connect { workspaceProjectIDE }
GatewayUI.getInstance().reset()
},
)
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Unchanged files with check annotations Beta

@JvmStatic
fun getBackgroundHostName(
hostname: String,
): String = hostname + "--bg"

Check notice on line 490 in src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

GitHub Actions / Qodana Community for JVM

String concatenation that can be converted to string template

'String' concatenation can be converted to a template
}
}
init {
init()
title = CoderGatewayBundle.message("gateway.connector.view.coder.remoteproject.choose.text", name)

Check warning on line 42 in src/main/kotlin/com/coder/gateway/util/Dialogs.kt

GitHub Actions / Qodana Community for JVM

Incorrect string capitalization

String 'Choose IDE and project for workspace {0}' is not properly capitalized. It should have title capitalization
}
override fun show() {
}
private class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentListModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentListModel?): String? = if (workspace == null) {

Check warning on line 882 in src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

GitHub Actions / Qodana Community for JVM

Redundant nullable return type

'valueOf' always returns non-null type
"Unknown"
} else if (workspace.workspace.outdated) {
"Outdated"