facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.3k stars 46.96k forks source link

Bug: click event is attached to createPortal #19611

Open 615349 opened 4 years ago

615349 commented 4 years ago

React version: 16.13.1

Steps To Reproduce

use createPortal will have click event attached to the DOM element automatically

import { createPortal } from "react-dom";

const Modal = ({ children, onClose, open }) =>
  open &&
  createPortal(
    <div />,
    document.getElementById("modal")
  );

this snippet will add click event attached to the element with id = modal please open chrome dev tool to check

Link to code example: https://codesandbox.io/s/vibrant-kowalevski-7ginb?file=/src/App.js

The current behavior

click event is added on the element

The expected behavior

click event should not be added

EDIT


createPortal(
    <h1>This is heading</h1>,
    document.getElementById("modal")
  );

The header is read by jaws (not sure about nvda) as this is heading, heading level one, clickable

eps1lon commented 4 years ago

click event should not be added

Is this about nvda announcing elements as clickable even if they aren't actually? I think adding a bit more context why the click listener should not be added would help the maintainers understanding the issue.

gaearon commented 4 years ago

FWIW this is intentional to prevent some iOS issue if I remember it right.

koba04 commented 4 years ago

https://github.com/facebook/react/pull/11927 seems to be the reason.

615349 commented 4 years ago

Is this about nvda announcing elements as clickable even if they aren't actually? I think adding a bit more context why the click listener should not be added would help the maintainers understanding the issue.

thanks @eps1lon, it is jaws not nvda, but I think NVDA would have same behaviour.
so for example

createPortal(
    <h1>This is heading</h1>,
    document.getElementById("modal")
  );

this is read as this is heading, heading level one, clickable

please advise

eps1lon commented 4 years ago

I tested it with latest nvda and didn't have any problems. IMO we should report this to the screen readers and ask for guidance on how to avoid the announcement. I understand that they want to patch up unintentionally inaccessible pages but in this case it is intended and jaws hurts a11y.

We should have a way to tell assistive technology that the click handler is added to leverage event bubbling for event delegation and not to execute user-affecting code directly.

jjenzz commented 3 years ago

I've just stumbled across this in NVDA version 2020.1 and latest NVDA version 2021.2.

https://user-images.githubusercontent.com/175330/140970574-4853e879-96d2-4bd7-a521-8db0679da1b3.mp4

https://codesandbox.io/s/xenodochial-bash-pnjwv?file=/src/App.js