Skip to content

feat: Initial Project Create Form ('/projects/create') #60

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

Merged
merged 16 commits into from
Jan 26, 2022

Conversation

bryphe-coder
Copy link
Contributor

@bryphe-coder bryphe-coder commented Jan 25, 2022

This is a super simple form for creating projects:

2022-01-25 12 58 21

TODO:

  • Wire up actual cancel behavior in project form
  • Wire up actual submit behavior in project form
    • POST to /projects/{organization} route
    • Handle invalidating /projects routes
  • Fix 'submit' button (use loading button)
  • Update gif with entire flow
  • Add smoke test for CreateProjectForm

@bryphe-coder bryphe-coder self-assigned this Jan 25, 2022
@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #60 (458703e) into main (bbd8b8f) will decrease coverage by 0.31%.
The diff coverage is 63.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
- Coverage   71.14%   70.82%   -0.32%     
==========================================
  Files          82       87       +5     
  Lines        3372     3455      +83     
  Branches       49       56       +7     
==========================================
+ Hits         2399     2447      +48     
- Misses        767      799      +32     
- Partials      206      209       +3     
Flag Coverage Δ
unittest-go-macos-latest 67.45% <ø> (+0.14%) ⬆️
unittest-go-ubuntu-latest 69.74% <ø> (ø)
unittest-go-windows-latest 67.19% <ø> (-0.15%) ⬇️
unittest-js 71.29% <63.95%> (-1.58%) ⬇️
Impacted Files Coverage Δ
site/pages/projects/create.tsx 0.00% <0.00%> (ø)
site/components/Form/FormSection.tsx 87.50% <87.50%> (ø)
site/components/Form/FormTitle.tsx 87.50% <87.50%> (ø)
site/components/Form/FormDropdownField.tsx 91.66% <91.66%> (ø)
site/forms/CreateProjectForm.tsx 95.45% <95.45%> (ø)
site/components/Form/index.ts 100.00% <100.00%> (ø)
site/test_helpers/index.tsx 100.00% <100.00%> (ø)
site/test_helpers/mocks.ts 100.00% <100.00%> (ø)
peer/conn.go 75.44% <0.00%> (-1.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbd8b8f...458703e. Read the comment docs.

@bryphe-coder bryphe-coder marked this pull request as ready for review January 25, 2022 21:01
})

const FormTextField = formTextFieldFactory<CreateProjectRequest>()
const FormDropdownField = formDropdownFieldFactory<CreateProjectRequest>()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do this factory pattern instead of using the component directly? I assume it's for the types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, exactly - sets things up so the compiler knows the formFieldName can only be a few possibilities:

image

And if I introduce a typo - or the schema of the types changes - VSCode lets me know:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also some more backstory here: https://github.com/coder/m/pull/7766 - it's a pattern @vapurrmaid introduced there that we've been leveraging for all our forms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formik out of the box is very loose with type safety - there's a lot of any's in their type definitions: https://github.com/jaredpalmer/formik/blob/e677bea8181f40e6762fc7e7fb009122384500c6/packages/formik/src/types.tsx#L6

Copy link
Contributor Author

@bryphe-coder bryphe-coder Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I regressed some code early on (one of my first PRs): https://github.com/coder/m/pull/9609- if that component had used this, it would've been caught at compile time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, one thing on our mind is how to accomplish this type-safety w/o a higher-order component - there may be a way to simplify this, esp with newer typescript versions

Copy link
Contributor Author

@bryphe-coder bryphe-coder Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... sorry for this rambling thread... I experimented and it seems we may be able to move away from the higher-order component today w/ our current TS version: #70 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged #70 and refactored so that FormDropdownField doesn't need a HoC either in 458703e ... and still get type safety!

@bryphe-coder bryphe-coder merged commit c7fb16e into main Jan 26, 2022
@bryphe-coder bryphe-coder deleted the bryphe/feat/project-create-form branch January 26, 2022 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants