Skip to content
This repository has been archived by the owner on Jun 5, 2023. It is now read-only.

Prevent Modal from re-registering & resetting focus when onRequestClose is mutated #118

Closed
diondiondion opened this issue Oct 29, 2020 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@diondiondion
Copy link
Member

When the close handler of the Modal component is recreated during a render while the modal is open, it will unregister & immediately re-register the modal. This will mutate the modalStack and cause the user's focus to be reset to whichever element caused the previous render.

This re-registering & focus reset shouldn't happen, as Modals should only unregister when they're actually closed. One way to prevent this would be to store the latest reference to the onRequestClose handler in a React ref inside of useModalManager. This ref can be mutated without causing a re-registration, and without risking a stale close handler (which would be the result of simply removing the prop from the effect's dependency array).

To recreate the bug:

const [someState, setSomeState] = React.useState('');
const [someOtherState, setSomeOtherState] = React.useState('');

<Modal
	name="modal"
	onRequestClose={() => {}}
	aria-labelledby="modal"
>
	<input type="text" value={someState} onChange={e => setSomeState(e.target.value)} />
	<input type="text" value={someOtherState} onChange={e => setSomeOtherState(e.target.value)} />
</Modal>

Typing into one of the inputs, then moving to another input, should get you into a "focus loop" where focus is continuously moved back and forth between both inputs as you type, as the modal's close handler is recreated on every state update.

The fix in this scenario is to either memoise the function passed to onRequestClose, or in this case to simply move it outside of the component definition.

@diondiondion diondiondion added the bug Something isn't working label Oct 29, 2020
@diondiondion diondiondion self-assigned this Mar 30, 2021
5app-Machine added a commit that referenced this issue Mar 30, 2021
## [13.1.1](v13.1.0...v13.1.1) (2021-03-30)

### Bug Fixes

* **Modal:** Fix unneeded Modal re-register, [#118](#118) ([2014106](2014106))
@zube zube bot removed the [zube]: Done label Jun 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant