From 623de5a5a6a3d4c8968959467b67421d8a04cf44 Mon Sep 17 00:00:00 2001 From: nanto_vi Date: Mon, 13 Jan 2025 02:09:05 +0900 Subject: [PATCH 1/3] fix: Don't reset form fields if reset event is cancelled. --- .../@react-aria/utils/src/useFormReset.ts | 4 +- .../utils/test/useFormReset.test.tsx | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 packages/@react-aria/utils/test/useFormReset.test.tsx diff --git a/packages/@react-aria/utils/src/useFormReset.ts b/packages/@react-aria/utils/src/useFormReset.ts index 3105cd27406..5bbd78e29fb 100644 --- a/packages/@react-aria/utils/src/useFormReset.ts +++ b/packages/@react-aria/utils/src/useFormReset.ts @@ -20,8 +20,8 @@ export function useFormReset( onReset: (value: T) => void ) { let resetValue = useRef(initialValue); - let handleReset = useEffectEvent(() => { - if (onReset) { + let handleReset = useEffectEvent((e: Event) => { + if (onReset && !e.defaultPrevented) { onReset(resetValue.current); } }); diff --git a/packages/@react-aria/utils/test/useFormReset.test.tsx b/packages/@react-aria/utils/test/useFormReset.test.tsx new file mode 100644 index 00000000000..84c58e01c69 --- /dev/null +++ b/packages/@react-aria/utils/test/useFormReset.test.tsx @@ -0,0 +1,53 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {fireEvent, render} from '@react-spectrum/test-utils-internal'; +import React, {useRef} from 'react'; +import {useFormReset} from '../'; + +describe('useFormReset', () => { + it('should call onReset on reset', () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( +
+ + +
+ ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).toHaveBeenCalled(); + }); + + it('should not call onReset if reset is cancelled', () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.preventDefault()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).not.toHaveBeenCalled(); + }); +}); From 45e92cf1a4ff585774d2656cf6ff311202b31161 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 20 May 2025 12:10:44 +1000 Subject: [PATCH 2/3] tests, handle bubble preventdefault and stoppropagation --- .../@react-aria/utils/src/useFormReset.ts | 46 +++++++++++++++++-- .../utils/test/useFormReset.test.tsx | 38 ++++++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/utils/src/useFormReset.ts b/packages/@react-aria/utils/src/useFormReset.ts index 2bc0ddbffff..b4bf47c3a60 100644 --- a/packages/@react-aria/utils/src/useFormReset.ts +++ b/packages/@react-aria/utils/src/useFormReset.ts @@ -19,18 +19,56 @@ export function useFormReset( initialValue: T, onReset: (value: T) => void ): void { + let currEvent = useRef(null); let resetValue = useRef(initialValue); + let handleReset = useEffectEvent((e: Event) => { - if (onReset && !e.defaultPrevented) { + currEvent.current = null; + if ( + e.target === ref?.current?.form + && onReset + && !e.defaultPrevented + ) { onReset(resetValue.current); } }); + /** + * Because event.stopPropagation() does not preventDefault and because we attach directly to the form element unlike React + * we need to create a new event which lets us monitor stopPropagation and preventDefault. This allows us to call onReset + * as the browser would natively. + */ + let formListener = useEffectEvent((e: Event) => { + if (currEvent.current !== null || e.target !== ref?.current?.form) { + // This is the re-dispatched event. Or it's for a different form. + return; + } + let alreadyPrevented = e.defaultPrevented; + e.stopPropagation(); + e.preventDefault(); + let event = new Event('reset', {bubbles: true, cancelable: true}); + currEvent.current = event; + let originalStopPropagation = event.stopPropagation; + let stopPropagation = () => { + currEvent.current = null; + originalStopPropagation.call(event); + if (!alreadyPrevented && !event.defaultPrevented && onReset) { + onReset(resetValue.current); + } + }; + event.stopPropagation = stopPropagation; + + e.target?.dispatchEvent(event); + }); + useEffect(() => { let form = ref?.current?.form; - form?.addEventListener('reset', handleReset); + let document = form?.ownerDocument; + document?.addEventListener('reset', formListener, true); + document?.addEventListener('reset', handleReset); return () => { - form?.removeEventListener('reset', handleReset); + document?.removeEventListener('reset', formListener, true); + document?.removeEventListener('reset', handleReset); }; - }, [ref, handleReset]); + }, [ref, handleReset, formListener]); } diff --git a/packages/@react-aria/utils/test/useFormReset.test.tsx b/packages/@react-aria/utils/test/useFormReset.test.tsx index 84c58e01c69..49c1151c45a 100644 --- a/packages/@react-aria/utils/test/useFormReset.test.tsx +++ b/packages/@react-aria/utils/test/useFormReset.test.tsx @@ -33,7 +33,43 @@ describe('useFormReset', () => { expect(onReset).toHaveBeenCalled(); }); - it('should not call onReset if reset is cancelled', () => { + it('should call onReset on reset even if event is stopped', () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.stopPropagation()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).toHaveBeenCalled(); + }); + + it('should not call onReset if reset is cancelled', async () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.preventDefault()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).not.toHaveBeenCalled(); + }); + + it('should not call onReset if reset is cancelled in capture phase', async () => { const onReset = jest.fn(); const Form = () => { const ref = useRef(null); From 0d17cc6bae71bd461f01b7ac73627cca70fa094b Mon Sep 17 00:00:00 2001 From: nanto_vi Date: Fri, 23 May 2025 02:46:44 +0900 Subject: [PATCH 3/3] fix: Allow multiple form elements with useFormReset --- .../@react-aria/utils/src/useFormReset.ts | 51 ++++++++----------- .../utils/test/useFormReset.test.tsx | 46 +++++++++++++++++ 2 files changed, 66 insertions(+), 31 deletions(-) diff --git a/packages/@react-aria/utils/src/useFormReset.ts b/packages/@react-aria/utils/src/useFormReset.ts index b4bf47c3a60..1b34a79fa41 100644 --- a/packages/@react-aria/utils/src/useFormReset.ts +++ b/packages/@react-aria/utils/src/useFormReset.ts @@ -14,61 +14,50 @@ import {RefObject} from '@react-types/shared'; import {useEffect, useRef} from 'react'; import {useEffectEvent} from './useEffectEvent'; +type ResetEvent = Event & { + reactAriaReDispatched?: boolean, + reactAriaShouldReset?: boolean +}; + export function useFormReset( ref: RefObject | undefined, initialValue: T, onReset: (value: T) => void ): void { - let currEvent = useRef(null); let resetValue = useRef(initialValue); - let handleReset = useEffectEvent((e: Event) => { - currEvent.current = null; - if ( - e.target === ref?.current?.form - && onReset - && !e.defaultPrevented - ) { - onReset(resetValue.current); - } - }); - /** * Because event.stopPropagation() does not preventDefault and because we attach directly to the form element unlike React * we need to create a new event which lets us monitor stopPropagation and preventDefault. This allows us to call onReset * as the browser would natively. */ - let formListener = useEffectEvent((e: Event) => { - if (currEvent.current !== null || e.target !== ref?.current?.form) { + let formListener = useEffectEvent((e: ResetEvent) => { + if (e.reactAriaReDispatched || e.target !== ref?.current?.form) { // This is the re-dispatched event. Or it's for a different form. return; } - let alreadyPrevented = e.defaultPrevented; - e.stopPropagation(); - e.preventDefault(); - let event = new Event('reset', {bubbles: true, cancelable: true}); - currEvent.current = event; - let originalStopPropagation = event.stopPropagation; - let stopPropagation = () => { - currEvent.current = null; - originalStopPropagation.call(event); - if (!alreadyPrevented && !event.defaultPrevented && onReset) { - onReset(resetValue.current); + if (e.reactAriaShouldReset === undefined) { + let event: ResetEvent = new Event('reset', {bubbles: true, cancelable: true}); + event.reactAriaReDispatched = true; + if (e.defaultPrevented) { + event.preventDefault(); } - }; - event.stopPropagation = stopPropagation; + e.stopPropagation(); + e.preventDefault(); - e.target?.dispatchEvent(event); + e.reactAriaShouldReset = e.target?.dispatchEvent(event) ?? false; + }; + if (onReset && e.reactAriaShouldReset) { + onReset(resetValue.current); + } }); useEffect(() => { let form = ref?.current?.form; let document = form?.ownerDocument; document?.addEventListener('reset', formListener, true); - document?.addEventListener('reset', handleReset); return () => { document?.removeEventListener('reset', formListener, true); - document?.removeEventListener('reset', handleReset); }; - }, [ref, handleReset, formListener]); + }, [ref, formListener]); } diff --git a/packages/@react-aria/utils/test/useFormReset.test.tsx b/packages/@react-aria/utils/test/useFormReset.test.tsx index 49c1151c45a..4aec20aeca6 100644 --- a/packages/@react-aria/utils/test/useFormReset.test.tsx +++ b/packages/@react-aria/utils/test/useFormReset.test.tsx @@ -51,6 +51,29 @@ describe('useFormReset', () => { expect(onReset).toHaveBeenCalled(); }); + it('should call every onReset on reset', () => { + const onReset1 = jest.fn(); + const onReset2 = jest.fn(); + const Form = () => { + const ref1 = useRef(null); + useFormReset(ref1, '', onReset1); + const ref2 = useRef(null); + useFormReset(ref2, '', onReset2); + return ( + + + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset1).toHaveBeenCalled(); + expect(onReset2).toHaveBeenCalled(); + }); + it('should not call onReset if reset is cancelled', async () => { const onReset = jest.fn(); const Form = () => { @@ -86,4 +109,27 @@ describe('useFormReset', () => { fireEvent.click(button); expect(onReset).not.toHaveBeenCalled(); }); + + it('should not call any onReset if reset is cancelled', () => { + const onReset1 = jest.fn(); + const onReset2 = jest.fn(); + const Form = () => { + const ref1 = useRef(null); + useFormReset(ref1, '', onReset1); + const ref2 = useRef(null); + useFormReset(ref2, '', onReset2); + return ( + e.preventDefault()}> + + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset1).not.toHaveBeenCalled(); + expect(onReset2).not.toHaveBeenCalled(); + }); });