-
-
Notifications
You must be signed in to change notification settings - Fork 479
feat(solid-form): add createFormHook
for solid-js.
#1597
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
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit c1f9dd8
☁️ Nx Cloud last updated this comment at |
Will start the work on docs after getting the green light on the PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1597 +/- ##
==========================================
+ Coverage 89.24% 93.50% +4.26%
==========================================
Files 31 3 -28
Lines 1432 77 -1355
Branches 366 4 -362
==========================================
- Hits 1278 72 -1206
+ Misses 137 5 -132
+ Partials 17 0 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The feature's already in the works over at #1453 . Would you mind taking a look at it and giving feedback? |
Looks like it has the same changes I've made, I've just addressed the comments there and used the types that were defined in the React implementation, I guess. We can change the target branch from main to that PR if we want to track it there. |
I'm using this PR in my project and fixing problems for now. |
afe9475
to
ebd3774
Compare
This fixes and issue where some parts of the form, like submitting didn't work.
@MAST1999 you are AWESOME. Thank you for taking this on! What can we do to support? |
Hey I think it's ready, I'm using it in my hobby project and it's working. But could always use a review or more testing. |
Awesome! In that case do you wanna start writing some docs for it? Even if it's just copy+pasted from the React one with Solid.js syntax? |
Sounds good Will get started! |
@MAST1999 I saw that you added docs, are we ready for final review and/or merge or are we waiting on something else? :) |
It's ready for final review. I wanted to add examples as well but I noticed that we use released versions for examples, So I'll open another PR with examples after release. |
Oh, actually, that's a great callout. Can we build out a We don't need to use the released version, Completely safe. |
Sorry for the continual last-minute requests 😅 |
Don't worry about it. This is what I wanted to do anyway 😁 |
OK, found and interesting problem that I hadn't run into. It happens when I try using the store on the I've added a comment on the Looked around, but I couldn't figure out where the problem is exactly. If you access the fields itself for value and errors it works correctly now, if you try using the |
I was starting a new project and decided to use solid, and I found out that
createHookForm
was missing so here I am 😄I don't have too much experience with solid, but I think this looks good.