-
Notifications
You must be signed in to change notification settings - Fork 15
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
bcpeinhardt
merged 11 commits into
main
from
bcpeinhardt/436-add-labels-to-workspace-actions
Aug 19, 2024
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a51beea
revise recent projects flow to be less confusing
bcpeinhardt c704fa5
swap hourglass for spinner
bcpeinhardt 6aeb88d
make the link just start the workspace
bcpeinhardt 7cd80fb
disable link when appropriate and show loading states
bcpeinhardt 9c876af
remove underline for disabled link
bcpeinhardt cc07fac
re-add wrapping to description label
bcpeinhardt 93101b3
add back status icon in error case
bcpeinhardt 901c2c3
run linter
bcpeinhardt a6b196b
update changelog
bcpeinhardt 3202bb8
decide icon in one place
bcpeinhardt 3020473
use let instead of git add .
bcpeinhardt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,10 +17,8 @@ | |
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.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,21 @@ | |
val workspaceWithAgent = deployment?.items?.firstOrNull { it.workspace.name == workspaceName } | ||
val status = | ||
if (deploymentError != null) { | ||
Triple(UIUtil.getBalloonErrorIcon(), UIUtil.getErrorForeground(), deploymentError) | ||
Triple(UIUtil.getErrorForeground(), deploymentError, UIUtil.getBalloonErrorIcon()) | ||
} else if (workspaceWithAgent != null) { | ||
val inLoadingState = listOf(WorkspaceStatus.STARTING, WorkspaceStatus.CANCELING, WorkspaceStatus.DELETING, WorkspaceStatus.STOPPING).contains(workspaceWithAgent?.workspace?.latestBuild?.status) | ||
Check warning on line 179 in src/main/kotlin/com/coder/gateway/views/CoderGatewayRecentWorkspaceConnectionsView.kt
|
||
|
||
Triple( | ||
workspaceWithAgent.status.icon, | ||
workspaceWithAgent.status.statusColor(), | ||
workspaceWithAgent.status.description, | ||
if (inLoadingState) { | ||
AnimatedIcon.Default() | ||
} else { | ||
null | ||
}, | ||
) | ||
} else { | ||
Triple(AnimatedIcon.Default.INSTANCE, UIUtil.getContextHelpForeground(), "Querying workspace status...") | ||
Triple(UIUtil.getContextHelpForeground(), "Querying workspace status...", AnimatedIcon.Default()) | ||
} | ||
val gap = | ||
if (top) { | ||
|
@@ -193,11 +198,6 @@ | |
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 +206,44 @@ | |
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) | ||
|
||
val enableLinks = listOf(WorkspaceStatus.STOPPED, WorkspaceStatus.CANCELED, WorkspaceStatus.FAILED, WorkspaceStatus.STARTING, WorkspaceStatus.RUNNING).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 { | ||
// There must be a way to make this properly wrap? | ||
label("<html><body style='width:350px;'>" + status.third + "</html>").applyToComponent { | ||
foreground = status.second | ||
status.third?.let { | ||
icon(it) | ||
} | ||
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
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 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 | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.