- Notifications
You must be signed in to change notification settings - Fork1k
feat: add preset selector in TasksPage#19012
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0a11e8e toebeb601CompareThere 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.
Pull Request Overview
This PR adds a preset selector to the TasksPage that appears when the selected template version has defined presets. If a preset contains an AI Prompt parameter, it overrides the textarea input and makes it read-only.
- Adds preset selection functionality with automatic default preset selection
- Implements AI prompt override behavior when presets contain AI Prompt parameters
- Refactors test data by moving mock objects to shared test helpers for better reusability
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| site/src/testHelpers/entities.ts | Adds mock preset data and task data for testing preset functionality |
| site/src/pages/TasksPage/TasksPage.tsx | Implements preset selector UI, preset fetching logic, and AI prompt override behavior |
| site/src/pages/TasksPage/TasksPage.stories.tsx | Updates stories to use shared mock data and adds new stories for preset scenarios |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| > | ||
| <SelectTrigger | ||
| id="templateID" | ||
| className="w-80 text-xs [&_svg]:size-icon-xs border-0 bg-surface-secondary h-8 px-3" |
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.
The template and preset selectors use identical styling (w-80 text-xs [&_svg]:size-icon-xs border-0 bg-surface-secondary h-8 px-3). Consider extracting this into a CSS class or constant to avoid duplication.
| className="w-80 text-xs [&_svg]:size-icon-xs border-0 bg-surface-secondary h-8 px-3" | |
| className="select-trigger-style" |
Copilot uses AI. Check for mistakes.
| constMockTasks=[ | ||
| { | ||
| workspace:{ | ||
| ...MockWorkspace, | ||
| latest_app_status:MockWorkspaceAppStatus, | ||
| }, | ||
| prompt:"Create competitors page", | ||
| }, | ||
| { | ||
| workspace:{ | ||
| ...MockWorkspace, | ||
| id:"workspace-2", | ||
| latest_app_status:{ | ||
| ...MockWorkspaceAppStatus, | ||
| message:"Avatar size fixed!", | ||
| }, | ||
| }, | ||
| prompt:"Fix user avatar size", | ||
| }, | ||
| { | ||
| workspace:{ | ||
| ...MockWorkspace, | ||
| id:"workspace-3", | ||
| latest_app_status:{ | ||
| ...MockWorkspaceAppStatus, | ||
| message:"Accessibility issues fixed!", | ||
| }, | ||
| }, | ||
| prompt:"Fix accessibility issues", | ||
| }, | ||
| ]; |
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.
review: moved toentities.ts
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 don't feel super comfortable with extrauseEffects, but I assume that we need to reload it dynamically, right?
| </SelectTrigger> | ||
| <SelectContent> | ||
| {presets | ||
| .sort((a,b)=>{ |
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.
nit: maybe extract to a seperate function with a comment?
| // Extract AI prompt from selected preset | ||
| constselectedPreset=presets?.find((p)=>p.ID===selectedPresetId); | ||
| constpresetAIPrompt=selectedPreset?.Parameters.find( | ||
| (param)=>param.Name===AI_PROMPT_PARAMETER_NAME, |
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.
nit: is it special/magic parameter 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.
always has been
Uh oh!
There was an error while loading.Please reload this page.
Is selecting a preset required? On the create workspace page I added a "None" to make it explicit that choosing a preset is not necessary. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Right now it technically isn't, although presets are currently the only way to expose parameter values on Tasks page. I can add a "none" option for consistency. |
Uh oh!
There was an error while loading.Please reload this page.
| placeholder={textareaPlaceholder} | ||
| className={`border-0 resize-none w-full h-full bg-transparent rounded-lg outline-none flex min-h-[60px] | ||
| text-sm shadow-sm text-content-primary placeholder:text-content-secondary md:text-sm`} | ||
| text-sm shadow-sm text-content-primary placeholder:text-content-secondary md:text-sm${ |
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.
Maybe this could be done later but it can be confusing for users that the Prompt is disabled because of a preset. In the dynamic parameters create workspace page, I label parameters when they are preset as such so user know why they can't be edited.
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 could add a quick tooltip here
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.
Addressed in0fd55e2
f6ecc40 tob3916e2CompareThere 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.
LGTM
9a05a8a intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
that is visible if the selected template version defines presets,with the default preset pre-selected and at the top of the list.