Skip to content
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

fix: disable form submission for isPending #7498

Merged
merged 10 commits into from
Dec 19, 2024
Merged
59 changes: 39 additions & 20 deletions packages/react-aria-components/src/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
import {createHideableComponent} from '@react-aria/collections';
import {filterDOMProps} from '@react-aria/utils';
import {ProgressBarContext} from './ProgressBar';
import React, {createContext, ForwardedRef, useEffect, useRef} from 'react';
import React, {createContext, DOMAttributes, ForwardedRef, useEffect, useMemo, useRef} from 'react';

export interface ButtonRenderProps {
/**
Expand Down Expand Up @@ -156,27 +156,46 @@ export const Button = /*#__PURE__*/ createHideableComponent(function Button(prop
}
wasPending.current = isPending;
}, [isPending, isFocused, ariaLabelledby, buttonId]);
let pendingButtonProps: DOMAttributes<HTMLButtonElement> = useMemo(() => isPending ? {
onKeyDown: (e) => {
if ((e.key === 'Enter' || e.key === ' ') && e.currentTarget instanceof HTMLButtonElement) {
e.preventDefault();
}
},
onClick: (e) => {
if (e.currentTarget instanceof HTMLButtonElement) {
e.preventDefault();
}
},
type: buttonProps.type === 'submit' ? 'button' : buttonProps.type
} : {}, [isPending]);

// When the button is in a pending state, we want to stop implicit form submission (ie. when the user presses enter on a text input).
// We do this by rendering a hidden submit button that is disabled BEFORE the actual submit button as a form's default button is the first submit button
snowystinger marked this conversation as resolved.
Show resolved Hide resolved
// in tree order.
// https://www.w3.org/TR/2018/SPSD-html5-20180327/forms.html#implicit-submission
return (
<button
{...filterDOMProps(props, {propNames: additionalButtonHTMLAttributes})}
{...mergeProps(buttonProps, focusProps, hoverProps)}
{...renderProps}
id={buttonId}
ref={ref}
aria-labelledby={ariaLabelledby}
slot={props.slot || undefined}
aria-disabled={isPending ? 'true' : buttonProps['aria-disabled']}
data-disabled={props.isDisabled || undefined}
data-pressed={renderValues.isPressed || undefined}
data-hovered={isHovered || undefined}
data-focused={isFocused || undefined}
data-pending={isPending || undefined}
data-focus-visible={isFocusVisible || undefined}>
<ProgressBarContext.Provider value={{id: progressId}}>
{renderProps.children}
</ProgressBarContext.Provider>
</button>
<>
<button
{...filterDOMProps(props, {propNames: additionalButtonHTMLAttributes})}
{...mergeProps(buttonProps, pendingButtonProps, focusProps, hoverProps)}
{...renderProps}
id={buttonId}
ref={ref}
aria-labelledby={ariaLabelledby}
slot={props.slot || undefined}
aria-disabled={isPending ? 'true' : buttonProps['aria-disabled']}
data-disabled={props.isDisabled || undefined}
data-pressed={renderValues.isPressed || undefined}
data-hovered={isHovered || undefined}
data-focused={isFocused || undefined}
data-pending={isPending || undefined}
data-focus-visible={isFocusVisible || undefined}>
<ProgressBarContext.Provider value={{id: progressId}}>
{renderProps.children}
</ProgressBarContext.Provider>
</button>
</>
);
});

Expand Down
149 changes: 148 additions & 1 deletion packages/react-aria-components/test/Button.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
* governing permissions and limitations under the License.
*/

import {act, fireEvent, pointerMap, render} from '@react-spectrum/test-utils-internal';
import {Button, ButtonContext, ProgressBar, Text} from '../';
import {fireEvent, pointerMap, render} from '@react-spectrum/test-utils-internal';
import React, {useState} from 'react';
import userEvent from '@testing-library/user-event';

Expand All @@ -21,6 +21,10 @@ describe('Button', () => {
user = userEvent.setup({delay: null, pointerMap});
jest.useFakeTimers();
});
afterEach(() => {
// clear any live announcers from pending buttons
act(() => jest.runAllTimers());
});

it('should render a button with default class', () => {
let {getByRole} = render(<Button>Test</Button>);
Expand Down Expand Up @@ -197,4 +201,147 @@ describe('Button', () => {
let button = getByRole('button');
expect(button).not.toHaveAttribute('href');
});

it('should prevent explicit mouse form submission when isPending', async function () {
let onSubmitSpy = jest.fn(e => e.preventDefault());
function TestComponent() {
let [pending, setPending] = useState(false);
return (
<Button
type="submit"
onPress={() => {
// immediately setting pending to true will remove the click handler before the form is submitted
setTimeout(() => {
setPending(true);
}, 0);
}}
isPending={pending}>
{({isPending}) => (
<>
<Text style={{opacity: isPending ? '0' : undefined}}>Test</Text>
<ProgressBar
aria-label="loading"
style={{opacity: isPending ? undefined : '0'}}
isIndeterminate>
loading
</ProgressBar>
</>
)}
</Button>
);
}
let {getByRole} = render(
<form onSubmit={onSubmitSpy}>
<TestComponent />
</form>
);
let button = getByRole('button');
expect(button).not.toHaveAttribute('aria-disabled');

await user.click(button);
expect(onSubmitSpy).toHaveBeenCalled();
onSubmitSpy.mockClear();

// run timer to set pending
act(() => jest.runAllTimers());

await user.click(button);
expect(onSubmitSpy).not.toHaveBeenCalled();
});

it('should prevent explicit keyboard form submission when isPending', async function () {
let onSubmitSpy = jest.fn(e => e.preventDefault());
function TestComponent() {
let [pending, setPending] = useState(false);
return (
<Button
type="submit"
onPress={() => {
// immediately setting pending to true will remove the click handler before the form is submitted
setTimeout(() => {
setPending(true);
}, 0);
}}
isPending={pending}>
{({isPending}) => (
<>
<Text style={{opacity: isPending ? '0' : undefined}}>Test</Text>
<ProgressBar
aria-label="loading"
style={{opacity: isPending ? undefined : '0'}}
isIndeterminate>
loading
</ProgressBar>
</>
)}
</Button>
);
}
render(
<form onSubmit={onSubmitSpy}>
<TestComponent />
</form>
);
await user.tab();
await user.keyboard('{Enter}');
expect(onSubmitSpy).toHaveBeenCalled();
onSubmitSpy.mockClear();
act(() => jest.runAllTimers());

await user.keyboard('{Enter}');
expect(onSubmitSpy).not.toHaveBeenCalled();
});

// Note: two inputs are needed, otherwise https://www.w3.org/TR/2011/WD-html5-20110525/association-of-controls-and-forms.html#implicit-submission
// Implicit form submission can happen if there's only one.
it.only('should prevent implicit form submission when isPending', async function () {
let onSubmitSpy = jest.fn(e => e.preventDefault());
function TestComponent(props) {
let [pending, setPending] = useState(false);
return (
<form
onSubmit={(e) => {
// forms are submitted implicitly on keydown, so we need to wait to set pending until after to set pending
props.onSubmit(e);
}}
onKeyDown={(e) => {
if (e.key === 'Enter') {
// keyup could theoretically happen elsewhere if focus is moved during submission
document.body.addEventListener('keyup', () => {
setPending(true);
}, {capture: true, once: true});
}
}}>
<label htmlFor="foo">Test</label>
<input id="foo" type="text" />
<input id="bar" type="text" />
<Button
type="submit"
isPending={pending}>
{({isPending}) => (
<>
<Text style={{opacity: isPending ? '0' : undefined}}>Test</Text>
<ProgressBar
aria-label="loading"
style={{opacity: isPending ? undefined : '0'}}
isIndeterminate>
loading
</ProgressBar>
</>
)}
</Button>
</form>
);
}
render(
<TestComponent onSubmit={onSubmitSpy} />
);
await user.tab();
await user.keyboard('{Enter}');
expect(onSubmitSpy).toHaveBeenCalled();
onSubmitSpy.mockClear();

await user.keyboard('{Enter}');
expect(onSubmitSpy).not.toHaveBeenCalled();
});
});
Loading