- Notifications
You must be signed in to change notification settings - Fork1k
feat: show workspace build logs during tasks creation#19413
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
Should we collapse/hide these by default and only show if a user clicks on show build logs? I think for most Tasks users they may not be interested in seeing the build logs. |
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.
Could you add some stories for the following scenarios:
- Workspace build pending
- Workspace build running
- Workspace build failed
@matifali I would agree with you if the screen had more info. Display the build logs is a way to show to the user that things are in progress. Before make any changes to it, I would like to test this hypothesis and see what users think about that, wdyt? |
@johnstcn they are already there. |
@BrunoQuaresma, I am fine if we want to test this. What I was thinking is a loading animation of some kind (Spinner) and a button to view logs if needed. |
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.
Sorry, missed seeing the stories! UI changes LGTM but also need approval from FE domain expert.
aslilac left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
overall looks good!
site/src/pages/TaskPage/TaskPage.tsx Outdated
| return( | ||
| <> | ||
| <Helmet> | ||
| <title>{pageTitle(ellipsizeText(task.prompt,64)??"Task")}</title> |
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.
| <title>{pageTitle(ellipsizeText(task.prompt,64)??"Task")}</title> | |
| <title>{pageTitle(ellipsizeText(task.prompt,64)||task.workspace.name)}</title> |
...might be nice as a still safe but more descriptive fallback. also maybe|| incaseprompt is""? I think you said that's not possible, but if it isn't I'm kinda confused on why we need the fallback at all
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.
Good catch. I'm going to remove this fallback too 👍
site/src/pages/TaskPage/TaskPage.tsx Outdated
| exportdefaultTaskPage; | ||
| constTaskBuildingWorkspace:FC<{task:Task}>=({ task})=>{ |
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.
| constTaskBuildingWorkspace:FC<{task:Task}>=({ task})=>{ | |
| typeTaskBuildingWorkspaceProps={ | |
| task:Task; | |
| }; | |
| constTaskBuildingWorkspace:FC<TaskBuildingWorkspaceProps>=({ task})=>{ |
| import{LinkasRouterLink}from"react-router"; | ||
| import{TaskStatusLink}from"./TaskStatusLink"; | ||
| exportconstTaskTopbar:FC<{task:Task}>=({ task})=>{ |
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.
same here, give this type a name pls
| </TooltipProvider> | ||
| <h1className="m-0 text-base font-medium truncate"> | ||
| {task.prompt||task.workspace.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.
re: fallbacks
this stands out to me too
| dayjs.extend(duration); | ||
| //ActiveTransition gets the build estimate for the workspace, | ||
| //getActiveTransitionStats gets the build estimate for the workspace, |
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.
ty for the rename
8aafbcb intomainUh oh!
There was an error while loading.Please reload this page.
This is part of#19363
Screenshot:
Video demo:
Screen.Recording.2025-08-19.at.12.32.15.mov