Skip to content
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

refactor: Refactor FormTextField to not require a HoC #70

Merged
merged 2 commits into from
Jan 25, 2022

Conversation

bryphe-coder
Copy link
Contributor

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

Prompted from discussion here: https://github.com/coder/coder/pull/60/files#r792124373

Our current FormTextField implementation requires a higher-order component, which can be complicated to understand.

This experiments with moving it to not require being a HoC.

The only difference in usage is that sometimes, you need to provide the type like <FormTextField<FormValues> form={form} formFieldName="some-field-in-form" /> - but it doesn't require special construction.

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

codecov bot commented Jan 25, 2022

Codecov Report

Merging #70 (0f26a37) into main (9cf4f7c) will decrease coverage by 0.18%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   71.09%   70.90%   -0.19%     
==========================================
  Files          82       82              
  Lines        3376     3372       -4     
  Branches       49       49              
==========================================
- Hits         2400     2391       -9     
- Misses        770      774       +4     
- Partials      206      207       +1     
Flag Coverage Δ
unittest-go-macos-latest 67.12% <ø> (-0.33%) ⬇️
unittest-go-ubuntu-latest 69.63% <ø> (-0.11%) ⬇️
unittest-go-windows-latest 67.12% <ø> (-0.15%) ⬇️
unittest-js 72.86% <93.33%> (-0.24%) ⬇️
Impacted Files Coverage Δ
site/components/Form/FormTextField.tsx 93.75% <93.10%> (-0.54%) ⬇️
site/components/SignIn/SignInForm.tsx 100.00% <100.00%> (ø)
peer/conn.go 74.25% <0.00%> (-1.50%) ⬇️

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 9cf4f7c...0f26a37. Read the comment docs.

@bryphe-coder
Copy link
Contributor Author

Thanks for the idea @kylecarbs !

@bryphe-coder bryphe-coder merged commit bbd8b8f into main Jan 25, 2022
@bryphe-coder bryphe-coder deleted the bryphe/refactor/form-text-field-no-hoc branch January 25, 2022 22:00
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.

None yet

2 participants