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

Field groups #4554

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Field groups #4554

wants to merge 18 commits into from

Conversation

zaalbarxx
Copy link
Contributor

@zaalbarxx zaalbarxx commented Nov 21, 2023

🔎 Overview
As detailed in #4181, it is extended version of PR #4553. This one, besides of exposing additional fields also adds new FieldGroup component with tests and documentation (documentation could not be verified since on my machine the documentation throws some errors
image

🤓 Code snippets/examples (if applicable)
https://stackblitz.com/edit/vue-3-veevalidate-form-validation-example-1dlyi1?file=src%2Fvee-validate.esm.js,src%2Fmain.js,package.json,src%2FApp.vue

Here is the code snippet with vee-validate.esm.js built from the branch which shows how it actually works.

Issues affected
closes #4181

Also for some reason the builds are failing, I could get some assist here. But first of all, and most important question is if we will process with this PR ? :)

…iple nested fields, it allows to check if set of fields is dirty/touched/valid etc.
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (a50956c) 89.81% compared to head (03c3601) 89.99%.

Files Patch % Lines
packages/vee-validate/src/useFieldGroup.ts 92.22% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4554      +/-   ##
==========================================
+ Coverage   89.81%   89.99%   +0.17%     
==========================================
  Files          93       95       +2     
  Lines        7651     7855     +204     
  Branches     1345     1367      +22     
==========================================
+ Hits         6872     7069     +197     
- Misses        772      779       +7     
  Partials        7        7              

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

@zaalbarxx
Copy link
Contributor Author

After reverting docs package.json to state before recent version bumps I was able to run the documentation locally. Made some changes and labeled the FieldGroup component as v4.7.

downgraded `remark-gfm` to lower version due to issue with building docs
@zaalbarxx
Copy link
Contributor Author

Also downgraded remark-gfm to 3.x since I found that it's the culprit of failing docs build.

Copy link
Owner

@logaretm logaretm left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, I will need some time to think about this because I don't think such an API is warranted especially since we already encourage the composition API more than the components API which might deprecated in the future.

feat: added `checkChildFieldGroups` property to `FieldGroup`
fix: added types to `useFieldGroup`
docs: added `useFieldGroup` documentation
@zaalbarxx
Copy link
Contributor Author

@logaretm Extended the functionality by adding useFieldGroup composable. Could also come handy. Cheers.

@logaretm
Copy link
Owner

Thanks, I will take a look when I can (around the weekend)

@zaalbarxx
Copy link
Contributor Author

zaalbarxx commented Dec 18, 2023

Any update on this ? Is it possible it will make it into the package ? Not sure if I should take my time to resolve conflicts and stuff :)

# Conflicts:
#	docs/package.json
#	packages/vee-validate/src/Field.ts
#	packages/vee-validate/src/useField.ts
#	pnpm-lock.yaml
@logaretm
Copy link
Owner

Hey @zaalbarxx, Unfortunately I have not had the time to check this out yet. This is a significant feature work and I would like to make sure we aggressively scope how this feature works because it caused problems before in the past:

So give me sometime to think about this some more, and sorry about the delay, I appreciate the time you've put into this.

@YoshiYo
Copy link

YoshiYo commented Feb 21, 2024

Any news on this? We would like to use groups too 🤞🏻

@zaalbarxx
Copy link
Contributor Author

It does work on production for us, for over 3 months, if that matters anyhow. If you'd like to use it too @YoshiYo I released "patched" packages at

"@zaalbarxx/vee-validate": "4.12.3",
"@zaalbarxx/vee-validate-i18n": "4.12.3",
"@zaalbarxx/vee-validate-rules": "4.12.3",

I had to release all of the packages since they depend on each other (like rules depend on validate), so patched versions also have their dependencies changed. The con is that you'd have to change all references in imports from vee-validate to @zaalbarxx/vee-validate, so that's propably a bit of an overkill. Anyway, it works for us and I'm also waiting for it to go through.

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.

Aggregated form validation
3 participants