-
Notifications
You must be signed in to change notification settings - Fork 915
feat: add ephemeral parameter dialog for workspace start/restart #18413
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
base: main
Are you sure you want to change the base?
Conversation
- Add EphemeralParametersDialog component to inform users about ephemeral parameters - Modify WorkspaceReadyPage to check for ephemeral parameters before start/restart - Update BuildParametersPopover to show ephemeral parameter info for non-classic flow - Update WorkspaceParametersPageExperimental button text based on template version - Only trigger for templates with use_classic_parameter_flow = false Co-authored-by: jaaydenh <1858163+jaaydenh@users.noreply.github.com>
- Fix TypeScript errors in EphemeralParametersDialog and BuildParametersPopover - Use correct Button component pattern with asChild and Link Co-authored-by: jaaydenh <1858163+jaaydenh@users.noreply.github.com>
@@ -22,6 +22,8 @@ const badgeVariants = cva( | |||
"border border-solid border-border-warning bg-surface-orange text-content-warning shadow", | |||
destructive: | |||
"border border-solid border-border-destructive bg-surface-red text-highlight-red shadow", | |||
green: |
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.
This is only meant to be used for ephemeral parameters and no other use cases. It felt like calling this ephemeral is too specific for the badge component so I left it as a generic green for now.
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.
Do we just use the previous value in the workspace settings page?
Provides options to continue without setting values or navigate to parameters page
Do we use the previous values? Or send no values, and assume the default value.
If the user clicks "Continue", from a frontend perspective, it should be the exact same behavior as before which means the ephemeral parameters are not given new values. |
resolves #17709
FYI, blink created a first draft which was heavily modified.
Summary
This PR implements ephemeral parameter handling for workspace start/restart operations when templates use dynamic parameters (
use_classic_parameter_flow = false
).Changes
1. EphemeralParametersDialog Component
site/src/components/EphemeralParametersDialog/
2. WorkspaceReadyPage Updates
checkEphemeralParameters()
function usingAPI.getDynamicParameters
handleStart
andhandleRestart
to check for ephemeral parametersuse_classic_parameter_flow = false
3. BuildParametersPopover Updates
WorkspaceParametersPageExperimental