-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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] Tell users prebuild is uploading #9904
Conversation
@svenefftinge: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -340,7 +340,7 @@ export default class StartWorkspace extends React.Component<StartWorkspaceProps, | |||
return; | |||
} | |||
|
|||
if (workspaceInstance.status.phase === "building" || workspaceInstance.status.phase == "preparing") { | |||
if (workspaceInstance.status.phase === "building" || workspaceInstance.status.phase === "preparing") { |
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.
👍
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.
👍
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.
Thanks, @svenefftinge! 🏀
I could not test this in action due to some hiccups of the the preview environment, see relevant discussion (internal).
Left one suggestion to rephrase the title below based on our yesterday's discussion.
return ( | ||
<HeadlessWorkspaceView | ||
instanceId={this.state.workspaceInstance.id} | ||
title="Uploading the prebuild ..." |
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.
issue: Based on a recent discussion, the term uploading here is not something users have seen across the product and do not to learn about, right?
return ( | ||
<HeadlessWorkspaceView | ||
instanceId={this.state.workspaceInstance.id} | ||
title="Uploading the prebuild ..." |
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.
suggestion: Maybe we could rephrase this to something like the following as this actually prepares the prebuild for the next workspace start phase. We could also skip the ellipsis (...) as this is not used in phase titles.
title="Uploading the prebuild ..." | |
title="Preparing Prebuild" |
Alternatively, we could go with:
title="Uploading the prebuild ..." | |
title="Finalizing Prebuild" |
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 find preparing and finalizing too abstract to bridge a few minutes of waiting. "Why does finalizing take so long?"
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.
Agree! The existing titles aim to abstract the sub-states of the loading cycle to make the loading friendlier to the user, that's why we're using Checking, Creating, & Starting in the rest of the workspace start phases. We're currently hijacking the same title to show Prebuild in Progress and do not group or relate the prebuild-in-progress phase with the rest of the workspace start phases.
Using the Uploading term could suffice as a sub-state and also wouldn't hurt to also use it as a title as this is already a good improvement. 🤝
Uploading as title | Uploading as sub-state | Preparing at title |
---|---|---|
thought: We could consider
BEFORE | AFTER (Early design) |
---|---|
Turns out this was the wrong place (CreateWorkspace is correct) and also we don't get updates on the prebuild. |
Description
Tell users that a workspace is uploading
Related Issue(s)
Fixes #
How to test
Release Notes
Documentation