epicweb-dev / full-stack-foundations

Learn the foundational skills of building full stack web applications.
https://epicweb.dev/workshops/full-stack-foundations
Other
598 stars 155 forks source link

Shouldn't "useIsSubmitting" compare to idle? #41

Closed atreib closed 6 hours ago

atreib commented 1 year ago

So, I noticed something in the Scripting module. Let's have a look at the exercises/05.scripting/05.solution.pending/app/utils/misc.tsx file. We have the following implementation for the useIsSubmitting() utility:

export function useIsSubmitting({
    formAction,
    formMethod = 'POST',
}: {
    formAction?: string
    formMethod?: 'POST' | 'GET' | 'PUT' | 'PATCH' | 'DELETE'
} = {}) {
    const contextualFormAction = useFormAction()
    const navigation = useNavigation()
    return (
        navigation.state === 'submitting' &&
        navigation.formAction === (formAction ?? contextualFormAction) &&
        navigation.formMethod === formMethod
    )
}

Before using the utility, we see in the course that the .state property have 3 options: idle, submitting and loading. So, idle, well, it is idle, submitting for when the user is sending data, and loading for when remix is getting data.

As explained, to have a best UX in the StatusButton behavior, we do: navigation.state !== 'idle'. But, then, in the useIsSubmitting utility, we compare to submitting instead.

Is that right? Cheers

kentcdodds commented 1 year ago

Thanks for pointing this out!

The utility itself is actually still evolving. You can find the current form in the Epic Stack here: https://github.com/epicweb-dev/epic-stack/blob/bce5f01b21f2dcaeb6b6e6e8bddf5247f3a3ae66/app/utils/misc.tsx#L193-L223

It follows the pattern shown in the videos by default. I'm pretty happy with that incarnation and should probably bring it into the workshops.

Ultimately, what's most important in the workshops is that you understand the different states so you can decide which states to check for when you're showing pending states. In general, I agree that just checking for non-idle is the right way to go as far as when to show a pending state.

I'll close this issue once the utility is updated in all the workshops.