Skip to content
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

[dashboard] Move inactive workspaces out of sight #10450

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Jun 3, 2022

Description

Many users are manually deleting workspaces, keeping their workspaces list clean.
Users should feel less urge to manually clean up and cleaning up should be simpler.

This PR, makes the inactive workspaces section on the /workspaces list collapsable (collapsed by default) to give a cleaner impression. When expanded there is a button to delete them.

  • TODO: Keep workspaces of the last 12 hours active

Related Issue(s)

Fixes #1053
Fixes #4233

How to test

Use the preview env and start a few workspaces, stop them. Check the workspaces list.

Release Notes

Added action to delete all inactive workspaces

Documentation

Screenshot 2022-06-03 at 12 02 12

Screenshot 2022-06-03 at 12 02 18

Screenshot 2022-06-03 at 12 02 24

@svenefftinge
Copy link
Member Author

@gtsiolis Can you take a look?
Also, I wonder if we should not display the empty state, in case there are no active workspaces. I.e. the "inactive workspaces" section would move below that empty space?

@roboquat roboquat added size/L and removed size/M labels Jun 3, 2022
@svenefftinge svenefftinge marked this pull request as ready for review June 3, 2022 15:06
@svenefftinge svenefftinge requested a review from a team June 3, 2022 15:06
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 3, 2022
@@ -145,8 +145,12 @@ export class WorkspaceModel implements Disposable, Partial<GitpodClient> {
}

protected isActive(info: WorkspaceInfo): boolean {
const lastSessionStart = WorkspaceInfo.lastActiveISODate(info);
const twentyfourHoursAgo = new Date(Date.now() - 1000 * 60 * 60 * 24).toISOString();
Copy link
Member

Choose a reason for hiding this comment

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

In timeutil.ts there are helpers for this - hoursBefore(lastSessionStart, 24)

(info.workspace.pinned || (!!info.latestInstance && info.latestInstance.status?.phase !== "stopped")) &&
(info.workspace.pinned ||
(!!info.latestInstance && info.latestInstance.status?.phase !== "stopped") ||
twentyfourHoursAgo.localeCompare(lastSessionStart) < 0) &&
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, we may be able to use isDateGreaterOrEqual(d1, d2) for the comparison here. Unsure if this works with the locale however.

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

Code-wise looks good to me, thanks for the change. Approving but adding hold for George to have a look at designs.

/hold

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 6, 2022

/werft run

👍 started the job as gitpod-build-sefftinge-make-it-easier-to-delete-4233.4
(with .werft/ from main)

@gtsiolis gtsiolis self-requested a review June 6, 2022 13:05
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up and +1 for making this happen, @svenefftinge! 🍷

Left some comments below containing bits from the design thinking while reviewing UX changes here.

Overall, I love the idea of hiding the inactive workspaces. Since we already have a block element for separating the active from inactive workspaces, I'd suggest keeping this block element instead of introducing a line separator. I'd expect more changes to land in the workspaces list soon, see relevant discussion (internal).

Also made a prototype (internal) to visualize this better. Feedback is welcome.

components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
components/dashboard/src/workspaces/Workspaces.tsx Outdated Show resolved Hide resolved
@svenefftinge
Copy link
Member Author

svenefftinge commented Jun 9, 2022

/werft run

👍 started the job as gitpod-build-sefftinge-make-it-easier-to-delete-4233.5
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/make-it-easier-to-delete-4233 branch from 59a5657 to f3577e4 Compare June 9, 2022 07:03
@svenefftinge
Copy link
Member Author

Screenshot 2022-06-09 at 14 23 22

Screenshot 2022-06-09 at 14 23 16

@svenefftinge svenefftinge force-pushed the sefftinge/make-it-easier-to-delete-4233 branch from f3577e4 to adf23cd Compare June 9, 2022 13:02
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-sefftinge-make-it-easier-to-delete-4233.8 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/make-it-easier-to-delete-4233 branch from adf23cd to 1a22dc8 Compare June 9, 2022 13:03
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-sefftinge-make-it-easier-to-delete-4233.10 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/make-it-easier-to-delete-4233 branch from 1a22dc8 to baf3eb9 Compare June 9, 2022 13:03
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-sefftinge-make-it-easier-to-delete-4233.12 because the annotations in the pull request description changed
(with .werft/ from main)

@svenefftinge svenefftinge force-pushed the sefftinge/make-it-easier-to-delete-4233 branch from baf3eb9 to d5940c9 Compare June 9, 2022 13:25
@roboquat roboquat merged commit 28c4b23 into main Jun 9, 2022
@roboquat roboquat deleted the sefftinge/make-it-easier-to-delete-4233 branch June 9, 2022 14:01
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 14, 2022
@svenefftinge svenefftinge restored the sefftinge/make-it-easier-to-delete-4233 branch June 15, 2022 07:42
@david-bakin
Copy link

david-bakin commented Jun 19, 2022

(As a user) I'm fine with hiding (folding away) inactive workspaces - but there is still a distinction between pinned and not-pinned, isn't there?

There used to be a separation on the workspaces page between pinned and unpinned. I found that useful. I'd like it back!

(Another suggestion, inappropriate for this PR, but just to provide food for thought: Users are allowed to name their workspaces - but the name appears at the end of the repo name - and there isn't much space for it! See here where the names are "latest" and "tapscript-<something-to-long-to-show>":

image

Maybe put the name first - or on a separate line if it gets too long? Names are useful - in some use cases (much) more useful than branch names to keep what's in different workspaces clear in your head - it would be nice to have longer ones. In fact, one reason I spend time deleting inactive workspaces is because the names are relatively useless to tell me which one's which, so I delete the known useless ones to remove some confusion. Simply folding them away doesn't really help this use case ...)

@svenefftinge
Copy link
Member Author

The pinned workspaces are considered "active" and therefore always in the upper section. This is the same behavior as before.
Sound like you would benefit from a clear indicator of the pinned status as well, since giving your workspaces names feels like you care and want to keep them for longer? We can certainly improve how the name is displayed as suggested. Can you file an issue for this?

@david-bakin
Copy link

Can you file an issue for this?

#10773 - ty for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it easier to delete multiple or all workspaces batch delete of workspaces
5 participants