[Maybe by design] Button of type submit not disabled with isPending at true

[alexis-lanoix-sixthfin]

Provide a general summary of the issue here

Not sure if this is by design, but I thought that with a Button of type submit, having the isPending prop set to true would prevent the form from being submitted but this is not the case.

🤔 Expected Behavior?

With a Button component of type submit and isPending set to true, pressing the button should not submit the form.

😯 Current Behavior

With a Button component of type submit and isPending set to true, pressing the button is triggering a form submit.

💁 Possible Solution

If this is by design, perhaps it should be explicitly explained and emphasized in the documentation. Like this for example:

Button events are disabled while isPending is true (this does not include submit events, you should use isDisabled for that).

🔦 Context

No response

🖥️ Steps to Reproduce

Preview the console and spam click on the "Click me" button: https://codesandbox.io/p/sandbox/peaceful-water-mynct4?file=%2Fsrc%2FApp.js

Version

3.32.2

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows 10

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[reidbarber]

This does look like a bug. I think we're just setting aria-disabled="true" and removing any passed-in event handlers:

https://github.com/adobe/react-spectrum/blob/759ac41ae9683d053044c508d9ec0dccebd3b8dd/packages/%40react-spectrum/button/src/Button.tsx#L36-L50

but instead, we could initialize isDisabled as props.isDisabled || props.isPending here:

https://github.com/adobe/react-spectrum/blob/759ac41ae9683d053044c508d9ec0dccebd3b8dd/packages/%40react-spectrum/button/src/Button.tsx#L56-L66

This may have been intentionally avoided though to prevent disabling the button while it is keyboard focused.

Maybe we should instead add event handlers to the button element while in its pending state that prevent the default behavior.

[snowystinger]

This was intentional, setting isDisabled will prevent screen reader users from accessing the button at all, so they won't know there is some long running task they are waiting for, they'll just wonder where the submit button is. It would also lose focus as @reidbarber pointed out.

As @reidbarber suggested, we should probably preventDefault onClick for the pending button so that a form submit isn't called, this could happen even if the form was already submitted, resulting in a second submission. Is that what you're dealing with? Otherwise, if you're applying isPending due to something else, I would recommend not using it except when processing a direct user action on the button.

[alexis-lanoix-sixthfin]

This was intentional, setting isDisabled will prevent screen reader users from accessing the button at all, so they won't know there is some long running task they are waiting for, they'll just wonder where the submit button is. It would also lose focus as @reidbarber pointed out.

As @reidbarber suggested, we should probably preventDefault onClick for the pending button so that a form submit isn't called, this could happen even if the form was already submitted, resulting in a second submission. Is that what you're dealing with? Otherwise, if you're applying isPending due to something else, I would recommend not using it except when processing a direct user action on the button.

We are currently using isPending to disable the form while processing and sending a request on form submit. I think it would be nice to prevent a form submit while isPending is true like you are suggesting.