-
Notifications
You must be signed in to change notification settings - Fork 8
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
Supported nested fields #1695
Supported nested fields #1695
Conversation
would this work correctly?
(maybe we should extend the demo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a really hard time reviewing this. Lot's of refactors, lot's of similar functions that are slightly different in detail. Also, most functions are rather hard to read.
Maybe adding more commits for smaller changes would have been preferable?
packages/admin/cms-admin/src/generator/future/utils/generateFieldList.ts
Outdated
Show resolved
Hide resolved
packages/admin/cms-admin/src/generator/future/utils/convertObjectToStructuredString.ts
Outdated
Show resolved
Hide resolved
packages/admin/cms-admin/src/generator/future/utils/generateFormValuesTypeDefinition.ts
Outdated
Show resolved
Hide resolved
packages/admin/cms-admin/src/generator/future/utils/generateFieldList.ts
Outdated
Show resolved
Hide resolved
I also underestimated this change. I thought simply improving the types, and handle the dot notation in the query would do the trick, but it needed changes to many places... and it seems it's not even ready now, as I found there are many issues with generated grid... |
I didn't test this level of nesting, but I think there should be no limit. I'll extend the demo to test this. |
I'm thinking about opening another pull-request for grid-support, but there are some functions required for both implementations. So even if it doubles the changes I think it's similar enough for the argument to keep it together. |
make it dependent on this one! |
I came up with some product-related entities containing double nesting and optional fields. should I add it to this PR or with a separate one? part of schema.gql
|
👍 please create a separate PR |
done: #1729 |
54cd1b1
to
f29211c
Compare
- `packageDimensions` was nullable, although the database fields aren't nullable (caused an DB error if null was saved) - Admin Generator (present and future) doesn't support nested fields (for future it is WIP, present will never get it) - Easy solution: remove it - #1729 will showcase nested fields and everything (as replacement for packageDimensions)
f29211c
to
357f7f5
Compare
a3cf65a
to
b672bf9
Compare
cae5610
to
1cc85e6
Compare
to simplify type supporting writing form-config
open points
UsableFields
(Supported nested fields #1695 (comment))objectSuffix
fromrecursiveStringify
maybe follow-up pull-request for checkable fieldset?-> conditional-fields will be added later