nkbt / react-copy-to-clipboard

Copy-to-clipboard React component
MIT License
2.34k stars 125 forks source link

Add click event to onCopy #191

Closed benbynum closed 1 year ago

benbynum commented 1 year ago

Return the mouse event as a third parameter in the onCopy method. The particular use case is to allow event interception and stop propagation where needed. This avoids having to wrap the parent in an element with an onClick which violates a11y principles of only having click behavior on interactive elements.

Corresponding pull request for type defs: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/64963

nkbt commented 1 year ago

Add onClick event to the element. It is already bypassed.

 // Bypass onClick if it was present
    if (elem && elem.props && typeof elem.props.onClick === 'function') {
      elem.props.onClick(event);
    }
benbynum commented 1 year ago

Add onClick event to the element. It is already bypassed.

 // Bypass onClick if it was present
    if (elem && elem.props && typeof elem.props.onClick === 'function') {
      elem.props.onClick(event);
    }

This still doesn't address potential a11y issues since it will only check for an onClick in the direct child, not any further descendants. Take my specific use case as an example:

<CopyToClipboard>
    <Tooltip>
        <IconButton>
            <Icon />
        </IconButton>
    </Tooltip>
<CopyToClipboard>

The onClick only gets triggered if attached to the Tooltip, not the IconButton.

Returning the click event from onCopy would be an exceedingly low-risk and low-effort change while providing all parents with direct access to the event data.

nkbt commented 1 year ago

Tooltip can wrap Copy, so only IconButton is a child. Then there are no problems

Please, do not wrap non-clickable elements, this makes no practical sense