Remove disabled states on form buttons #1298
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| <Button | ||
| type="submit" | ||
| size="sm" | ||
| disabled={!!submitDisabled} |
There was a problem hiding this comment.
The button is still clickable while the request is going through, which means you can submit multiple times. Rather than changing Button internally so that loading also causes it to be disabled, I think I prefer keeping the disabled prop as the only one that can actually disable the button and changing the call to be something like this:
| disabled={!!submitDisabled} | |
| disabled={!!submitDisabled || loading || isSubmitting} |
There was a problem hiding this comment.
Thanks for catching that, I'd left out some logic. We have to supply loading with the proper state in order to provide the visual indication that it is indeed loading. Given we need to supply that state and it seems like invalid behavior to have a button both loading and clickable then it seems reasonable to expect the loading prop would disable the button.
If we don't then we must always pass the loading state twice in the same component usage. Not doing so would result in a bug.
There was a problem hiding this comment.
Yeah, that's true. We'd never want a button clickable while loading. Fine with me.
just-be-dev
force-pushed
the
better-disabled-behaviors
branch
from
2d4e461 to
51ca636
Compare
We discussed this in chat a few days ago. Even though the accessibility of disabled buttons was improved in #1292, it's typically accepted that form submit buttons shouldn't be disabled, but should just trigger validation instead. This PR takes that action.
Changes
loadingas a prop which is used to prevent double submitssubmitDisabledis updated to take a string such that if we do need to disable a submit button, we can ensure a reason is provided