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

feat(core): add field listeners #1032

Merged
merged 15 commits into from
Nov 25, 2024
Merged

Conversation

harry-whorlow
Copy link
Contributor

@harry-whorlow harry-whorlow commented Nov 22, 2024

Background

From discussion #709 and PR #801

A continuation of the PR #801 that been sitting stale since July, as per the previous pull request:

Sometimes we may want to attach our own side effects to the field when some event happens. 
For example, resetting another field if some field value has changed.

Attaching these 'listeners' inside the validator does not work as intended, because validator 
does not guarantee if it was triggered by the exact event. 
(eg. all validators run when form submits)

It's of course possible to implement, but it's not very clean and the concerns become scattered.

The PR was given the go ahead by @crutchcorn here.

This is something we would really like to have, as I'm currently experiencing a minor blocker with this functionality missing, and I would prefer to have a clear api as opposed to using the current validators onChange workaround.


Continuation of work

  • rebased to current main
  • removed onHandleChange listener & tests
  • add onBlur listener & tests
  • add onSubmit tests (need guidance on functionality)

Guidance wanted

Specifically referring to the onSubmit comment made here

Is there a method that I've missed for an "on submit" callback that I can use to set the listeners value inside the FiledApi when the form is submitted? or is this something I need to create and pass down from the form to the field? Just a hint in the correct direction would be really appreciated.

Feel free to fire off any questions, I'll do my best to get back to you in a timely manner.

Looking forward to hearing from the maintainers 🤟

Functionality Tests Docs
[x] onChange [x] onChange [ ] doc structure
[x] onBlur [x] onBlur [ ] React
[x] onMount [x] onMount [ ] Vue
[x] onSubmit [x] onSubmit [ ] Anngular

@harry-whorlow
Copy link
Contributor Author

Moved the onSubmit test from FieldApi.spec.ts -> FormApi.spec.ts

Copy link

nx-cloud bot commented Nov 22, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit dba1652. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Copy link

pkg-pr-new bot commented Nov 22, 2024

Open in Stackblitz

More templates

@tanstack/angular-form

npm i https://pkg.pr.new/@tanstack/angular-form@1032

@tanstack/react-form

npm i https://pkg.pr.new/@tanstack/react-form@1032

@tanstack/solid-form

npm i https://pkg.pr.new/@tanstack/solid-form@1032

@tanstack/valibot-form-adapter

npm i https://pkg.pr.new/@tanstack/valibot-form-adapter@1032

@tanstack/vue-form

npm i https://pkg.pr.new/@tanstack/vue-form@1032

@tanstack/yup-form-adapter

npm i https://pkg.pr.new/@tanstack/yup-form-adapter@1032

@tanstack/zod-form-adapter

npm i https://pkg.pr.new/@tanstack/zod-form-adapter@1032

@tanstack/form-core

npm i https://pkg.pr.new/@tanstack/form-core@1032

@tanstack/lit-form

npm i https://pkg.pr.new/@tanstack/lit-form@1032

commit: dba1652

Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.06%. Comparing base (c8e9887) to head (e07a05b).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1032      +/-   ##
==========================================
+ Coverage   85.70%   86.06%   +0.36%     
==========================================
  Files          28       28              
  Lines        1098     1105       +7     
  Branches      275      275              
==========================================
+ Hits          941      951      +10     
+ Misses        144      141       -3     
  Partials       13       13              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Member

@Balastrong Balastrong left a comment

Choose a reason for hiding this comment

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

Fantastic PR, very well done! :D

I think you can tick the onSubmit box and go ahead with the docs.

packages/form-core/src/FieldApi.ts Outdated Show resolved Hide resolved
packages/form-core/src/FormApi.ts Outdated Show resolved Hide resolved
@harry-whorlow
Copy link
Contributor Author

harry-whorlow commented Nov 22, 2024

With regards to your comment, I would say yes unless you have a leaning in a different direction... But, I'm completely open to suggestions.

@harry-whorlow
Copy link
Contributor Author

@Balastrong hot off the press with a first draft for the docs, let me know what you think.

Once I get the thumbs up I'll create the Vue and Angular docs. Feel free to offer improvements and tweaks 🤟

packages/form-core/src/FormApi.ts Outdated Show resolved Hide resolved
packages/form-core/tests/FormApi.spec.ts Outdated Show resolved Hide resolved
packages/form-core/src/FieldApi.ts Outdated Show resolved Hide resolved
docs/framework/react/guides/listeners.md Outdated Show resolved Hide resolved
docs/framework/react/guides/listeners.md Outdated Show resolved Hide resolved
docs/framework/react/guides/basic-concepts.md Show resolved Hide resolved
@harry-whorlow
Copy link
Contributor Author

@Balastrong, let me know what you think! if everything looks good I'll finish the Vue and Angular when I get back tonight. Enjoy the weekend!🤟

@ha1fstack
Copy link
Contributor

Thank you for following up the original PR 👏

Copy link
Member

@Balastrong Balastrong left a comment

Choose a reason for hiding this comment

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

Sorry I'm looking at all the tiny details now, but everything else looks really great :)

Please go ahead with the docs for the other frameworks and I think we're good to go!

docs/framework/react/guides/basic-concepts.md Outdated Show resolved Hide resolved
docs/framework/react/guides/listeners.md Outdated Show resolved Hide resolved
@harry-whorlow
Copy link
Contributor Author

@Balastrong no please, nitpick away... I'll be the poor sod using it 😂

@harry-whorlow
Copy link
Contributor Author

harry-whorlow commented Nov 24, 2024

@Balastrong would it be okay to close out this pull request, and leave the angular docs up to the community... It's been a few years since I did angular and most of the documentation is missing from tanstack/form, so its not like I have a reference to compare against.

I'll give it a go in the mean time, but if I can't get it, I'll create an issue for it once this is merged. 🤟

[edit] ah, I gave it a go... please double check it though as I'm not the Angular savant

Copy link
Member

@Balastrong Balastrong left a comment

Choose a reason for hiding this comment

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

Angular was missing an explicit Input definition to expose listeners, I added it now.
Also on the docs I realized we don't have yet the ability to reset a single field, so you either form.reset({...form.state.value, field: ''}) or form.setFieldValue('field', '').

Everything else looks good, thank you so much for the PR and all the fixes!

@Balastrong Balastrong merged commit 6968cfd into TanStack:main Nov 25, 2024
5 checks passed
@harry-whorlow
Copy link
Contributor Author

Thanks man! it's been a pleasure working with you.

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.

3 participants