-
Notifications
You must be signed in to change notification settings - Fork 2
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: global forms provider #250
base: master
Are you sure you want to change the base?
Conversation
/** | ||
* Returns a combined boolean of all submitSuccessful states | ||
*/ |
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.
How are they combined?
export function useGlobalForms() { | ||
const { isSubmitting, reset, submit } = useContext(GlobalFormsContext); | ||
|
||
return { | ||
isSubmitting, | ||
reset, | ||
submit, | ||
}; | ||
} |
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.
Why is this function necessary or useful? It seems to me like the indirection is not needed.
import React, { useCallback, useContext, useMemo } from 'react'; | ||
|
||
export type FormInfo = { name: string; order: number }; | ||
type FormInfoMixed = string | FormInfo; |
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.
type FormInfoMixed = string | FormInfo; | |
type FormInfoObjectOrString = FormInfo | string; |
return form; | ||
} | ||
|
||
export function useGlobalForm(form: FormInfoMixed) { |
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.
When would I use an explicit order?
setSubmitSuccessful, | ||
} = useContext(GlobalFormsContext); | ||
|
||
const setResetCallbackWrapped = useCallback( |
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.
const setResetCallbackWrapped = useCallback( | |
const setResetCallbackMemoized = useCallback( |
It is not just wrapped in some arbitrary way.
import { useGlobalForms, useGlobalForm } from './GlobalFormsContext'; | ||
import { GlobalFormsProvider } from './GlobalFormsProvider'; | ||
|
||
function createOptions() { |
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.
Options for what? An explicit return type might help
expect(submitFirst).toHaveBeenCalledTimes(1); | ||
expect(submitError).toHaveBeenCalledTimes(1); | ||
expect(submitLast).toHaveBeenCalledTimes(0); |
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 think this does not prove the order was correct, I think you can swap the numbers around and still pass this test.
function findNextOrder( | ||
callbacks: Record<string, CallbackWithOrder> | undefined, | ||
): number { | ||
const orders = values(callbacks).map((callback) => callback.order); |
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.
const orders = values(callbacks).map((callback) => callback.order); | |
const orders = Object.values(callbacks).map((callback) => callback.order); |
|
||
const submit: GlobalFormsContextType['submit'] = useCallback( | ||
async (additionalCallback?: CallbackWithOrder) => { | ||
keys(submitCallbacks).forEach((formName) => { |
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.
keys(submitCallbacks).forEach((formName) => { | |
Object.keys(submitCallbacks).forEach((formName) => { |
keys(submitCallbacks).forEach((formName) => { | ||
submitSuccessfulList.current[formName] = false; | ||
}); | ||
const callbacks = values(submitCallbacks); |
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.
const callbacks = values(submitCallbacks); | |
const callbacks = Object.values(submitCallbacks); |
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.
and other uses
feat: global forms provider