-
Notifications
You must be signed in to change notification settings - Fork 937
fix: create and read workspace page #1294
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
Changes from all commits
386a2d7
c4c4b3c
7e3db42
f46c970
74a5d1d
b0e4b5a
495aba6
14be28e
fc98515
c0ad87b
60532a8
cfac4c2
78f1708
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { screen } from "@testing-library/react" | ||
import React from "react" | ||
import { MockTemplate, MockWorkspace, renderWithAuth } from "../../testHelpers" | ||
import { WorkspacePage } from "./WorkspacePage" | ||
|
||
describe("Workspace Page", () => { | ||
it("shows a workspace", async () => { | ||
renderWithAuth(<WorkspacePage />, { route: `/workspaces/${MockWorkspace.id}`, path: "/workspaces/:workspace" }) | ||
const workspaceName = await screen.findByText(MockWorkspace.name) | ||
const templateName = await screen.findByText(MockTemplate.name) | ||
expect(workspaceName).toBeDefined() | ||
expect(templateName).toBeDefined() | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { useActor } from "@xstate/react" | ||
import React, { useContext, useEffect } from "react" | ||
import { useParams } from "react-router-dom" | ||
import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" | ||
import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" | ||
import { Margins } from "../../components/Margins/Margins" | ||
import { Stack } from "../../components/Stack/Stack" | ||
import { Workspace } from "../../components/Workspace/Workspace" | ||
import { firstOrItem } from "../../util/array" | ||
import { XServiceContext } from "../../xServices/StateContext" | ||
|
||
export const WorkspacePage: React.FC = () => { | ||
const { workspace: workspaceQueryParam } = useParams() | ||
const workspaceId = firstOrItem(workspaceQueryParam, null) | ||
|
||
const xServices = useContext(XServiceContext) | ||
const [workspaceState, workspaceSend] = useActor(xServices.workspaceXService) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, this uses a global XState service. Would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking it would be active for longer but maybe that's not actually desired since it's for a specific workspace. I can make it local for now and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neato neato There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kylecarbs Actually I think we'll want it to outlive the component because we're using separate pages for things like the create/edit form and the parts of the timeline. Does that sound right? |
||
const { workspace, template, organization, getWorkspaceError, getTemplateError, getOrganizationError } = | ||
workspaceState.context | ||
|
||
/** | ||
* Get workspace, template, and organization on mount and whenever workspaceId changes. | ||
* workspaceSend should not change. | ||
*/ | ||
useEffect(() => { | ||
workspaceId && workspaceSend({ type: "GET_WORKSPACE", workspaceId }) | ||
}, [workspaceId, workspaceSend]) | ||
|
||
if (workspaceState.matches("error")) { | ||
return <ErrorSummary error={getWorkspaceError || getTemplateError || getOrganizationError} /> | ||
} else if (!workspace || !template || !organization) { | ||
return <FullScreenLoader /> | ||
presleyp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
return ( | ||
<Margins> | ||
<Stack spacing={4}> | ||
<Workspace organization={organization} template={template} workspace={workspace} /> | ||
</Stack> | ||
</Margins> | ||
) | ||
} | ||
} |
This file was deleted.
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.
Should this be
organizationID
to match othercamelCase
fields in this type?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 left it snakecased because it gets piped directly out of and into api calls, but I agree the inconsistency isn't good. I guess I'll ask about this at FE V, we have some other spots like this.