wwnorton/design-system

Modal does not reliably steal focus [NDS-95]

ottworks opened this issue · 2 comments

Modal can get into a state where focus is outside of the modal. This happens reliably when triggering a modal on Dropdown change:

function ModalWithButton() {
    const [isOpen, setIsOpen] = React.useState(false);
    const open = () => setIsOpen(true);
    const close = () => setIsOpen(false);
    return (
        <>
            <Dropdown variant="solid" onChange={open}>Open the modal</Dropdown>
            <Modal
                title="Confirm the prompt"
                isOpen={isOpen}
                onRequestClose={close}
                actions={[
                    <Button variant="outline" color="base" onClick={close}>
                        Also confirm
                    </Button>,
                    <Button variant="solid" onClick={close}>
                        Confirm
                    </Button>,
                ]}
            >
                <p>
                    This is a demo modal.
                    Real modals should have useful information here.
                </p>
            </Modal>
        </>
    );
}

image

This can also happen with the default button activation using NVDA on Windows:

image

Firefox 89 on Windows 10.
NDS-95

sh0ji commented

Great find! Thanks so much, @ottworks.

When the Dropdown closes, it returns focus to its triggering element, which we don't currently have a way to disable. The result is that both the Dropdown and the Modal are trying to control focus in the same event loop and the Dropdown is winning. I'll bring this up with the team to see if we can implement a way to control focus management in Dropdown.

We'll need to research the NVDA bug a bit more. It does appear to be a bug, but it's hard to say if it's an NVDA one or a design system one. Space emulates mouse clicks by default on NVDA, so holding it down will continually trigger the click event. From what I can tell, holding down Space causes the virtual cursor to get stuck on the button eventually, possibly because it lags behind focus. You won't see this issue unless a virtual cursor is involved because without it, click is triggered on keyup of Space, meaning you can't continuously click with Space (this is by design—Enter is the continuous click key).

sh0ji commented

@ottworks, we finally started looking into a fix for this and discovered that it is possible to circumvent the default behavior to stop the Dropdown from focusing its button when it closes. This can be accomplished by controlling the isOpen state via the onRequestClose callback. The onRequestClose callback allows you to respond to events like Escape or clicking outside the dropdown, choosing whether you're going to close the dropdown's listbox or not.

We'll continue looking into making this more obvious or updating the API to be more clear about how focus is managed, but in the meantime here's your example using a managed isOpen state for the Dropdown:

const ModalWithButton = () => {
	const [isOpen, setIsOpen] = React.useState(false);
	// manage the isOpen state of the listbox as well
	const [isListboxOpen, setIsListboxOpen] = React.useState(false);
	const open = () => setIsOpen(true);
	const close = () => setIsOpen(false);
	return (
		<>
			<Dropdown
				label="Modal with button"
				onChange={open}
				// control by binding isOpen to your state and responding to the onRequestClose callback
				isOpen={isListboxOpen}
				onRequestClose={() => setIsListboxOpen(false)}
			>
				<Dropdown.Option>Open the modal</Dropdown.Option>
				<Dropdown.Option>Open the modal again</Dropdown.Option>
			</Dropdown>
			<Modal
				title="Confirm the prompt"
				isOpen={isOpen}
				onRequestClose={close}
				actions={[
					<Button variant="outline" color="base" onClick={close}>
						Also confirm
					</Button>,
					<Button variant="solid" onClick={close}>
						Confirm
					</Button>,
				]}
			>
				<p>
					This is a demo modal.
					Real modals should have useful information here.
				</p>
			</Modal>
		</>
	);
};