Skip to content

allow user customize workspace timeout #2

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 10, 2023
Merged

allow user customize workspace timeout #2

merged 1 commit into from
Feb 10, 2023

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Feb 6, 2023 •
edited
Loading

Test it on gitpod-io/gitpod#16220

This PR add new command which can customize workspace timeout flexible

image

image

image

filiptronicek
filiptronicek reviewed Feb 6, 2023
View reviewed changes
filiptronicek
filiptronicek reviewed Feb 6, 2023
View reviewed changes
filiptronicek
filiptronicek reviewed Feb 6, 2023
View reviewed changes
Copy link
Member

@filiptronicek filiptronicek left a comment

Choose a reason for hiding this comment

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

question: @iQQBot do you think it's better to display this error after the user already submits their timeout extension, or should we just not show this Command Palette option at all when the user does not have the correct subscription?

image

cc @loujaybee

@iQQBot iQQBot mentioned this pull request Feb 7, 2023
Merged
10 tasks
@iQQBot iQQBot force-pushed the pd/custom-timeout branch from 86c274f to 8036819 Compare February 7, 2023 17:06
@iQQBot iQQBot marked this pull request as ready for review February 7, 2023 17:06
@iQQBot
Copy link
Contributor Author

iQQBot commented Feb 7, 2023

@loujaybee The current implementation keeps the original command and the extend icon in the bottom right corner behavior to avoid breaking the user experience, and instead of it with a new command customize workspace timeout

Do we want to merge these two? it would break user experience, i.e. user needs one more step to input time.

iQQBot
iQQBot commented Feb 7, 2023
View reviewed changes
gitpod-shared/src/features.ts
@@ -416,7 +440,9 @@ export async function registerWorkspaceTimeout(context: GitpodExtensionContext):
return;
}
extendTimeoutStatusBarItem.tooltip = `Workspace Timeout: ${instance.status.timeout}. Click to extend.`;
extendTimeoutStatusBarItem.color = instance.status.timeout === '180m' ? new vscode.ThemeColor('notificationsWarningIcon.foreground') : undefined;

// TODO: query default timeout, currently all paid plan default timeout is 60m.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this icon was highlighted if the workspace timeout was extended to 180 minutes, but now that this condition has been changed with the ability to customize the timeout, how should we handle this highlighting logic?

And in the feature if the user already customizes workspace timeout time, it will also disable closed timeout, do we need to tell the user?

Copy link
Member

@loujaybee loujaybee Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

And in the feature if the user already customizes workspace timeout time, it will also disable closed timeout, do we need to tell the user?

Interesting question. We can document this, but I don't think there's much we can do help the user in the product? Could we add it to the copy from the confirmation toast modal? 💭

@iQQBot iQQBot force-pushed the pd/custom-timeout branch from 8036819 to e121e6b Compare February 7, 2023 17:16
akosyakov
akosyakov reviewed Feb 8, 2023
View reviewed changes
akosyakov
akosyakov reviewed Feb 8, 2023
View reviewed changes
akosyakov
akosyakov reviewed Feb 8, 2023
View reviewed changes
akosyakov
akosyakov reviewed Feb 8, 2023
View reviewed changes
@loujaybee
Copy link
Member

loujaybee commented Feb 9, 2023 •
edited
Loading

do you think it's better to display this error after the user already submits their timeout extension, or should we just not show this Command Palette option at all when the user does not have the correct subscription?

I agree that delaying the feedback isn't ideal, but I wouldn't hide the command, that could be more confusing if there are screenshots or docs leading the user to never discover that they need to upgrade their plan. Altering the input UI to show a warning for users not on the correct plan might make more sense, is that possible?

@iQQBot iQQBot force-pushed the pd/custom-timeout branch from e121e6b to 33011c8 Compare February 9, 2023 15:21
@loujaybee
Copy link
Member

loujaybee commented Feb 9, 2023 •
edited
Loading

The current implementation keeps the original command and the extend icon in the bottom right corner behavior to avoid breaking the user experience, and instead of it with a new command customize workspace timeout. Do we want to merge these two? it would break user experience, i.e. user needs one more step to input time.

I agree having extend here is confusing. But, let's not remove the extend timeout in this change. I'd suggest we add this new configurable command, ensure that we have product analytics on how many users use the command pallette action + the values that they are setting, and then we can review and take a decision on what alternative constant timeout values users might need. Removing the extend command would break some users.

Let's keep this change to:

  • Allowing a user to set a custom timeout value.
  • Limiting the user to 24hour limit.
  • Fixing any bugs (if there are any)

I would suggest we remove extend functionality separately:

What do you think @iQQBot ?

EDIT: Regarding the timeout icon behaviour

Can we update it so that the icon prompts the user for a custom value? That way we wouldn’t break those users current workflows (they’d require one extra step to input the value - but the trade-off is they get more choice)? We could add future actions for faster actions if users need more optimal methods. We simply don’t know if keeping the 180m is meaningful in any way to users right now.

EDIT: Regarding the timeout icon highlighting

We could highlight if timeout is not default (e.g. timeout != 60m) I’m not convinced the highlighting has much utility, but doing this would at least be consistent? If we have no API for retrieving the default timeout, then I guess we cannot highlight reasonably. In which case I think our best shot is just to ignore the highlighting. Allow the user to update, and we can implement highlighting in future potentially when we have an API to retrieve the default value.

@iQQBot iQQBot force-pushed the pd/custom-timeout branch from 33011c8 to 292faa4 Compare February 9, 2023 16:00
filiptronicek
filiptronicek reviewed Feb 10, 2023
View reviewed changes
filiptronicek
filiptronicek reviewed Feb 10, 2023
View reviewed changes
@iQQBot iQQBot force-pushed the pd/custom-timeout branch 5 times, most recently from ffdb171 to 4c13d18 Compare February 10, 2023 15:36
filiptronicek
filiptronicek approved these changes Feb 10, 2023
View reviewed changes
Copy link
Member

@filiptronicek filiptronicek left a comment

Choose a reason for hiding this comment

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

All good! :shipit:

@iQQBot iQQBot merged commit b513e54 into master Feb 10, 2023
@filiptronicek filiptronicek deleted the pd/custom-timeout branch February 10, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov akosyakov left review comments

@loujaybee loujaybee loujaybee left review comments

@filiptronicek filiptronicek filiptronicek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iQQBot @loujaybee @akosyakov @filiptronicek