From b7234a6ce16d139528fadeeef40f558acab0f37e Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 9 Jun 2022 18:43:49 -0500 Subject: [PATCH] fix: push create workspace UX to templates page (#2142) --- site/src/AppRouter.tsx | 35 +++---- site/src/components/PageHeader/PageHeader.tsx | 15 +++ .../CreateWorkspacePage.test.tsx | 12 +-- .../CreateWorkspacePage.tsx | 18 ++-- .../CreateWorkspacePageView.stories.tsx | 5 - .../CreateWorkspacePageView.tsx | 95 +++---------------- .../pages/TemplatePage/TemplatePageView.tsx | 2 +- .../pages/TemplatesPage/TemplatesPageView.tsx | 5 +- .../WorkspacesPage/WorkspacesPageView.tsx | 18 ++-- .../createWorkspaceXService.ts | 82 ++-------------- 10 files changed, 78 insertions(+), 209 deletions(-) diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index 8faffb5061..8c8880c9de 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -56,15 +56,6 @@ export const AppRouter: FC = () => ( } /> - - - - - } - /> @@ -77,14 +68,24 @@ export const AppRouter: FC = () => ( } /> - - - - } - /> + + + + + } + /> + + + + } + /> + diff --git a/site/src/components/PageHeader/PageHeader.tsx b/site/src/components/PageHeader/PageHeader.tsx index 6ea052d33c..f2224be821 100644 --- a/site/src/components/PageHeader/PageHeader.tsx +++ b/site/src/components/PageHeader/PageHeader.tsx @@ -32,6 +32,12 @@ export const PageHeaderSubtitle: React.FC = ({ children }) => { return

{children}

} +export const PageHeaderText: React.FC = ({ children }) => { + const styles = useStyles() + + return

{children}

+} + const useStyles = makeStyles((theme) => ({ root: { display: "flex", @@ -58,6 +64,15 @@ const useStyles = makeStyles((theme) => ({ marginTop: theme.spacing(1), }, + text: { + fontSize: theme.spacing(2), + color: theme.palette.text.secondary, + fontWeight: 400, + display: "block", + margin: 0, + marginTop: theme.spacing(1), + }, + actions: { marginLeft: "auto", }, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index a173570e19..962583b4a4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -4,14 +4,13 @@ import * as API from "../../api/api" import { Language as FooterLanguage } from "../../components/FormFooter/FormFooter" import { MockTemplate, MockWorkspace } from "../../testHelpers/entities" import { renderWithAuth } from "../../testHelpers/renderHelpers" -import { Language as FormLanguage } from "../../util/formUtils" import CreateWorkspacePage from "./CreateWorkspacePage" import { Language } from "./CreateWorkspacePageView" const renderCreateWorkspacePage = () => { return renderWithAuth(, { - route: "/workspaces/new?template=" + MockTemplate.name, - path: "/workspaces/new", + route: "/templates/" + MockTemplate.name + "/workspace", + path: "/templates/:template/workspace", }) } @@ -29,13 +28,6 @@ describe("CreateWorkspacePage", () => { expect(element).toBeDefined() }) - it("shows validation error message", async () => { - renderCreateWorkspacePage() - await fillForm({ name: "$$$" }) - const errorMessage = await screen.findByText(FormLanguage.nameInvalidChars(Language.nameLabel)) - expect(errorMessage).toBeDefined() - }) - it("succeeds", async () => { renderCreateWorkspacePage() // You have to spy the method before it is used. diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index fec4684d47..d534c0bbc9 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -1,8 +1,7 @@ import { useMachine } from "@xstate/react" import { FC } from "react" import { Helmet } from "react-helmet" -import { useNavigate, useSearchParams } from "react-router-dom" -import { Template } from "../../api/typesGenerated" +import { useNavigate, useParams } from "react-router-dom" import { useOrganizationId } from "../../hooks/useOrganizationId" import { pageTitle } from "../../util/page" import { createWorkspaceMachine } from "../../xServices/createWorkspace/createWorkspaceXService" @@ -10,11 +9,11 @@ import { CreateWorkspacePageView } from "./CreateWorkspacePageView" const CreateWorkspacePage: FC = () => { const organizationId = useOrganizationId() - const [searchParams] = useSearchParams() - const preSelectedTemplateName = searchParams.get("template") + const { template } = useParams() + const templateName = template ? template : "" const navigate = useNavigate() const [createWorkspaceState, send] = useMachine(createWorkspaceMachine, { - context: { organizationId, preSelectedTemplateName }, + context: { organizationId, templateName }, actions: { onCreateWorkspace: (_, event) => { navigate(`/@${event.data.owner_name}/${event.data.name}`) @@ -31,11 +30,12 @@ const CreateWorkspacePage: FC = () => { loadingTemplates={createWorkspaceState.matches("gettingTemplates")} loadingTemplateSchema={createWorkspaceState.matches("gettingTemplateSchema")} creatingWorkspace={createWorkspaceState.matches("creatingWorkspace")} + templateName={createWorkspaceState.context.templateName} templates={createWorkspaceState.context.templates} selectedTemplate={createWorkspaceState.context.selectedTemplate} templateSchema={createWorkspaceState.context.templateSchema} onCancel={() => { - navigate(preSelectedTemplateName ? "/templates" : "/workspaces") + navigate("/templates") }} onSubmit={(request) => { send({ @@ -43,12 +43,6 @@ const CreateWorkspacePage: FC = () => { request, }) }} - onSelectTemplate={(template: Template) => { - send({ - type: "SELECT_TEMPLATE", - template, - }) - }} /> ) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index e134d0e940..8a97781aff 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -33,11 +33,6 @@ export default { const Template: Story = (args) => -export const NoTemplates = Template.bind({}) -NoTemplates.args = { - templates: [], -} - export const NoParameters = Template.bind({}) NoParameters.args = { templates: [MockTemplate], diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index ebb398f86b..38fb1ee416 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -1,15 +1,9 @@ -import Link from "@material-ui/core/Link" -import MenuItem from "@material-ui/core/MenuItem" import { makeStyles } from "@material-ui/core/styles" -import TextField, { TextFieldProps } from "@material-ui/core/TextField" -import OpenInNewIcon from "@material-ui/icons/OpenInNew" +import TextField from "@material-ui/core/TextField" import { FormikContextType, useFormik } from "formik" import { FC, useState } from "react" -import { Link as RouterLink } from "react-router-dom" import * as Yup from "yup" import * as TypesGen from "../../api/typesGenerated" -import { CodeExample } from "../../components/CodeExample/CodeExample" -import { EmptyState } from "../../components/EmptyState/EmptyState" import { FormFooter } from "../../components/FormFooter/FormFooter" import { FullPageForm } from "../../components/FullPageForm/FullPageForm" import { Loader } from "../../components/Loader/Loader" @@ -20,29 +14,18 @@ import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formU export const Language = { templateLabel: "Template", nameLabel: "Name", - emptyMessage: "Let's create your first template", - emptyDescription: ( - <> - To create a workspace you need to have a template. You can{" "} - - create one from scratch - {" "} - or use a built-in template by typing the following Coder CLI command: - - ), - templateLink: "Read more about this template", } export interface CreateWorkspacePageViewProps { loadingTemplates: boolean loadingTemplateSchema: boolean creatingWorkspace: boolean + templateName: string templates?: TypesGen.Template[] selectedTemplate?: TypesGen.Template templateSchema?: TypesGen.ParameterSchema[] onCancel: () => void onSubmit: (req: TypesGen.CreateWorkspaceRequest) => void - onSelectTemplate: (template: TypesGen.Template) => void } export const validationSchema = Yup.object({ @@ -51,7 +34,8 @@ export const validationSchema = Yup.object({ export const CreateWorkspacePageView: FC = (props) => { const [parameterValues, setParameterValues] = useState>({}) - const styles = useStyles() + useStyles() + const form: FormikContextType = useFormik({ initialValues: { name: "", @@ -84,75 +68,20 @@ export const CreateWorkspacePageView: FC = (props) }, }) const getFieldHelpers = getFormHelpers(form) - const selectedTemplate = - props.templates && - form.values.template_id && - props.templates.find((template) => template.id === form.values.template_id) - - const handleTemplateChange: TextFieldProps["onChange"] = (event) => { - if (!props.templates) { - throw new Error("Templates are not loaded") - } - - const templateId = event.target.value - const selectedTemplate = props.templates.find((template) => template.id === templateId) - - if (!selectedTemplate) { - throw new Error(`Template ${templateId} not found`) - } - - form.setFieldValue("template_id", selectedTemplate.id) - props.onSelectTemplate(selectedTemplate) - } return (
- {props.loadingTemplates && } - - {props.templates && props.templates.length === 0 && ( - - } - /> - )} - {props.templates && props.templates.length > 0 && ( - - {Language.templateLink} - - ) - } - > - {props.templates.map((template) => ( - - {template.name} - - ))} - - )} + + {props.loadingTemplateSchema && } {props.selectedTemplate && props.templateSchema && ( <> = ({ template, activeTe + } diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 6e942c9f39..a27cf2cacf 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -22,7 +22,7 @@ import { HelpTooltipTitle, } from "../../components/HelpTooltip/HelpTooltip" import { Margins } from "../../components/Margins/Margins" -import { PageHeader, PageHeaderTitle } from "../../components/PageHeader/PageHeader" +import { PageHeader, PageHeaderText, PageHeaderTitle } from "../../components/PageHeader/PageHeader" import { Stack } from "../../components/Stack/Stack" import { TableLoader } from "../../components/TableLoader/TableLoader" @@ -84,6 +84,9 @@ export const TemplatesPageView: FC = (props) => { + {props.templates && props.templates.length > 0 && ( + Choose a template to create a new workspace. + )} diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 3d189179a4..d212fbff22 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -32,7 +32,7 @@ import { HelpTooltipTitle, } from "../../components/HelpTooltip/HelpTooltip" import { Margins } from "../../components/Margins/Margins" -import { PageHeader, PageHeaderTitle } from "../../components/PageHeader/PageHeader" +import { PageHeader, PageHeaderText, PageHeaderTitle } from "../../components/PageHeader/PageHeader" import { Stack } from "../../components/Stack/Stack" import { TableLoader } from "../../components/TableLoader/TableLoader" import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" @@ -41,7 +41,7 @@ import { getDisplayStatus, workspaceFilterQuery } from "../../util/workspace" dayjs.extend(relativeTime) export const Language = { - createWorkspaceButton: "Create workspace", + createFromTemplateButton: "Create from template", emptyCreateWorkspaceMessage: "Create your first workspace", emptyCreateWorkspaceDescription: "Start editing your source code and building your software", emptyResultsMessage: "No results matched your search", @@ -132,11 +132,13 @@ export const WorkspacesPageView: FC = ({ loading, works - - + + Create a new workspace from a{" "} + + Template + + . + } > @@ -213,7 +215,7 @@ export const WorkspacesPageView: FC = ({ loading, works description={Language.emptyCreateWorkspaceDescription} cta={ - + } /> diff --git a/site/src/xServices/createWorkspace/createWorkspaceXService.ts b/site/src/xServices/createWorkspace/createWorkspaceXService.ts index 297819d1b2..cefbd179c6 100644 --- a/site/src/xServices/createWorkspace/createWorkspaceXService.ts +++ b/site/src/xServices/createWorkspace/createWorkspaceXService.ts @@ -4,26 +4,18 @@ import { CreateWorkspaceRequest, ParameterSchema, Template, Workspace } from ".. type CreateWorkspaceContext = { organizationId: string + templateName: string templates?: Template[] selectedTemplate?: Template templateSchema?: ParameterSchema[] createWorkspaceRequest?: CreateWorkspaceRequest createdWorkspace?: Workspace - // This is useful when the user wants to create a workspace from the template - // page having it pre selected. It is string or null because of the - // useSearchQuery - preSelectedTemplateName: string | null } -type CreateWorkspaceEvent = - | { - type: "SELECT_TEMPLATE" - template: Template - } - | { - type: "CREATE_WORKSPACE" - request: CreateWorkspaceRequest - } +type CreateWorkspaceEvent = { + type: "CREATE_WORKSPACE" + request: CreateWorkspaceRequest +} export const createWorkspaceMachine = createMachine( { @@ -45,12 +37,6 @@ export const createWorkspaceMachine = createMachine( }, }, tsTypes: {} as import("./createWorkspaceXService.typegen").Typegen0, - on: { - SELECT_TEMPLATE: { - actions: ["assignSelectedTemplate"], - target: "gettingTemplateSchema", - }, - }, states: { gettingTemplates: { invoke: { @@ -58,17 +44,11 @@ export const createWorkspaceMachine = createMachine( onDone: [ { actions: ["assignTemplates"], - target: "waitingForTemplateGetCreated", cond: "areTemplatesEmpty", }, { - actions: ["assignTemplates", "assignPreSelectedTemplate"], + actions: ["assignTemplates", "assignSelectedTemplate"], target: "gettingTemplateSchema", - cond: "hasValidPreSelectedTemplate", - }, - { - actions: ["assignTemplates"], - target: "selectingTemplate", }, ], onError: { @@ -76,33 +56,6 @@ export const createWorkspaceMachine = createMachine( }, }, }, - waitingForTemplateGetCreated: { - initial: "refreshingTemplates", - states: { - refreshingTemplates: { - invoke: { - src: "getTemplates", - onDone: [ - { target: "waiting", cond: "areTemplatesEmpty" }, - { target: "#createWorkspaceState.selectingTemplate", actions: ["assignTemplates"] }, - ], - }, - }, - waiting: { - after: { - 2_000: "refreshingTemplates", - }, - }, - }, - }, - selectingTemplate: { - on: { - SELECT_TEMPLATE: { - actions: ["assignSelectedTemplate"], - target: "gettingTemplateSchema", - }, - }, - }, gettingTemplateSchema: { invoke: { src: "getTemplateSchema", @@ -164,13 +117,6 @@ export const createWorkspaceMachine = createMachine( }, }, guards: { - hasValidPreSelectedTemplate: (ctx, event) => { - if (!ctx.preSelectedTemplateName) { - return false - } - const template = event.data.find((template) => template.name === ctx.preSelectedTemplateName) - return !!template - }, areTemplatesEmpty: (_, event) => event.data.length === 0, }, actions: { @@ -178,7 +124,10 @@ export const createWorkspaceMachine = createMachine( templates: (_, event) => event.data, }), assignSelectedTemplate: assign({ - selectedTemplate: (_, event) => event.template, + selectedTemplate: (ctx, event) => { + const templates = event.data.filter((template) => template.name === ctx.templateName) + return templates.length ? templates[0] : undefined + }, }), assignTemplateSchema: assign({ // Only show parameters that are allowed to be overridden. @@ -188,17 +137,6 @@ export const createWorkspaceMachine = createMachine( assignCreateWorkspaceRequest: assign({ createWorkspaceRequest: (_, event) => event.request, }), - assignPreSelectedTemplate: assign({ - selectedTemplate: (ctx, event) => { - const selectedTemplate = event.data.find((template) => template.name === ctx.preSelectedTemplateName) - // The proper validation happens on hasValidPreSelectedTemplate - if (!selectedTemplate) { - throw new Error("Invalid template selected") - } - - return selectedTemplate - }, - }), }, }, )