- Notifications
You must be signed in to change notification settings - Fork920
feat(site): support match option for auto create workspace flow#13836
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
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.
Looks good! Mainly had a question for my own understanding of the E2E tests
Uh oh!
There was an error while loading.Please reload this page.
| const richParameters: RichParameter[] = [ | ||
| { ...emptyParameter, name: "repo", type: "string" }, | ||
| ]; | ||
| const template = await createTemplate( |
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 haven't worked with the E2E stuff much yet, so just to make sure I'm understanding: these testsaren't isolated, right? That's how the second test is able to have a workspace already exist, even though it didn't explicitly make two of them?
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.
They are not isolated, but I ensure that a previous workspace with the right match exists in line 39.
| }); | ||
| onCreateWorkspace(newWorkspace); | ||
| } catch (err) { | ||
| searchParams.delete("mode"); | ||
| setSearchParams(searchParams); | ||
| setMode("form"); |
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'm wondering if it might still be good to update the search params on top of the state change, but good catch with this
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've thought about this and I don't think it makes much of a difference in the user experience, based on what I can see. Since no other part of the component uses it, I just removed it and decided to set the mode only in the state.
6bf7e5a intomainUh oh!
There was an error while loading.Please reload this page.
This feature will support having a
matchproperty incoder.yamlfor the Coder Chrome plugin.Related tohttps://github.com/coder/chrome-coder/issues/1