feat(core): add useDocumentForm hook
#8557
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pedrobonamin
requested review from
cngonzalez
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
No changes to documentation |
|
Component Testing Report Updated Feb 21, 2025 10:27 AM (UTC) ❌ Failed Tests (6) -- expand for details
|
⚡️ Editor Performance ReportUpdated Fri, 21 Feb 2025 10:22:14 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
pedrobonamin
force-pushed
the
standalone-form-2
branch
from
f35716c to
193846b
Compare
pedrobonamin
force-pushed
the
standalone-form-2
branch
from
193846b to
5a4dff2
Compare
pedrobonamin
requested review from
a team and
bjoerge
and removed request for
a team
bjoerge
reviewed
Feb 13, 2025
| * | ||
| * Use this as a base point to create your own form. | ||
| */ | ||
| export function useCreateForm(options: CreateFormOptions): CreateFormValue { |
There was a problem hiding this comment.
Unsure about the naming here. Why "create" form? Could it be named "useDocumentForm" instead?
There was a problem hiding this comment.
Yeah, not married with the name.
I was thinking on create because it creates the handlers and states necessary for the <FormBuilder>
create ~ build.
We are exposing another hook that is useFormBuilder which gives you the values back from the formBuilder. Which when I read that name it gives me the idea that I can create a new form using the useFormBuilder, but not, it gives me the values from the provider.
Btw, the comment is incorrect, it should say:
Hook for creating a form state and combine it with the
FormProviderFormBuilder.
| documentId: string | ||
| documentType: string | ||
| }) { | ||
| const {setParams: setPaneParams} = usePaneRouter() |
There was a problem hiding this comment.
Are the passed params also coming from usePaneRouter() ? Wondering if setParams could then be passed in as well? Or if we could get params from the hook
There was a problem hiding this comment.
Makes sense, I'll pass it.
In the first implementation it was using them from the usePaneRouter but then it requires to do here the to use the useUnique which ends up creating a new state for the params, which we reuse in multiple places, so it was better to pass it.
| collapsedPaths, | ||
| schemaType, | ||
| value, | ||
| } = useCreateForm({documentId, documentType: 'tasks.task', initialValue}) |
There was a problem hiding this comment.
Very nice that you can now do this 🎉
There was a problem hiding this comment.
This could have save me a lot of work last year , hope it saves some time for us in the future
pedrobonamin
changed the title
useCreateForm hookuseDocumentForm hook
bjoerge
reviewed
Feb 18, 2025
| const handleChange = useCallback((event: PatchEvent) => patchRef.current(event), []) | ||
|
|
||
| useInsertionEffect(() => { | ||
| if (readOnly) { |
bjoerge
reviewed
Feb 18, 2025
| */ | ||
| getFormDocumentValue?: (value: SanityDocumentLike) => SanityDocumentLike | ||
| } | ||
| interface CreateFormValue { |
There was a problem hiding this comment.
Thanks for catching.
Why Props and not Value ?
bjoerge
reviewed
Feb 18, 2025
pedrobonamin
force-pushed
the
standalone-form-2
branch
from
dee23b7 to
52233ce
Compare
pedrobonamin
force-pushed
the
standalone-form-2
branch
from
dbc1f9e to
f391abb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements a new hook
useDocumentFormwhich handles the necessary logic to build a form and pass the values to the<FormProvider>It also simplifies the logic inside the
<DocumentPaneProvider>to make it easier to digest:useDocumentPaneInitialValueuseDocumentPaneInspectorFurther steps:
forcedVersioninDocumentPaneProviderinspectlegacy dialog into the new inspectors logic suggestionWhy we need this?
Currently if you want to render a form using the patch mechanism, have presence, validation and compare values you would need to reuse the
<DocumentPaneProvider>which bring with it some unnecessary things likehistory,actions,perspectiveetc, or you needed to write it in your own like in useTasksFormBuilder. This is not ideal and it takes a lot of effort from the developer.By implementing this hook you can now do use this minimal implementation to get all the necessary form values and handlers. (see example in code)
This should also be useful for:
<DocumentPaneProvider>from hereWhat to review
Please help me review in detail this PR as it touches a critical surface which handles a lot of logic, my recommendation is going commit per commit to see how the changes build on top of each other.
Is the naming of the new
useDocumentFormhook good?Testing
Manual testing has been done to the PR, focusing on history, versions and tasks which are not covered by e2e tests.
Existing tests should cover the rest of the functionality
Notes for release
adds
useDocumentFormhook