-
Notifications
You must be signed in to change notification settings - Fork 888
feat(site): display build logs on template creation #12271
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
Conversation
…ild-logs-on-template-create
…ild-logs-on-template-create
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 component needs to be refactored to make it easier to extend and use #12272
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.
Extracted from ImportStarterTemplateView
FYI: Tests are on the way, I'm going to re-ask for review when pushing them but I would like to get as early feedback as possible. |
Co-authored-by: Kayla Washburn-Love <mckayla@hey.com>
…ttps://github.com/coder/coder into bq/show-build-logs-on-template-create
@@ -49,6 +49,8 @@ global.TextDecoder = TextDecoder as any; | |||
global.Blob = Blob as any; | |||
global.scrollTo = jest.fn(); | |||
|
|||
window.HTMLElement.prototype.scrollIntoView = function () {}; |
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.
window.HTMLElement.prototype.scrollIntoView = function () {}; | |
window.HTMLElement.prototype.scrollIntoView = jest.fn(); |
templateVersion: TemplateVersion | undefined, | ||
options?: { onDone: () => Promise<unknown> }, | ||
) => { | ||
const [logs, setLogs] = useState<ProvisionerJobLog[] | undefined>(); |
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 should've been more specific, but my original issue was really more wondering why we need undefined
to be a unique state from []
. We could reset the state by just saying setLogs([])
and the onMessage
handler wouldn't need to check if logs
is defined or not, it could just be simplified to setLogs((logs) => [...logs, log])
.
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 use the following:
undefined
- connecting[]
- connected but empty
Wdyt? I use the same idea for fetching server data. query.data && <Loader />
and so on.
); | ||
}; | ||
|
||
const MissingVariablesBanner: FC<{ onFillVariables: () => void }> = ({ |
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.
no inline prop types pls
Demo:
Screen.Recording.2024-02-22.at.11.40.12.mov
Close #12083