copy-to-clipboard icon indicating copy to clipboard operation
copy-to-clipboard copied to clipboard

onClick() callback prevents text from being copied

Open MaffooBristol opened this issue 5 years ago • 7 comments

MacOS Mojave 10.14.6 Tested on both Chrome 81 and Safari 13 copy-to-clipboard 3.3.1

This seems like a pretty fundamentally critical bug.

When I use the function on a button's onClick event, it works fine. However when I specify an onCopy() too, it just doesn't copy anything. It doesn't matter what's inside the callback function, or whether it's an arrow function, or whether the function is defined outside of the handler, or anything else really.

Works:

<button
  onClick={() => {
    copy(Math.random(), {});
  }}
>
  Copy text
</button>

Doesn't work:

<button
  onClick={() => {
    copy(Math.random(), {
      onCopy: () => {},
    });
  }}
>
  Copy text
</button>

MaffooBristol avatar May 18 '20 09:05 MaffooBristol

Additionally, I think it would be better if the function returned a promise

MaffooBristol avatar May 18 '20 09:05 MaffooBristol

wonder whether that's because of preventDefault in onCopy branching logic https://github.com/sudodoki/copy-to-clipboard/blob/master/index.js#L67 I will try to look into it later, but first thought is that this is odd

sudodoki avatar May 18 '20 09:05 sudodoki

@MaffooBristol

Additionally, I think it would be better if the function returned a promise

we had callback support at some point, but that wasn't too useful given we are doing sync stuff all the time (prompt is blocking as well). Given a newer api that is promise based, we had some initial discussion on the level 'well, it would be cool to support it and the probably wrapping everying into promise interface', but no work was done as nobody had time for it

sudodoki avatar May 18 '20 09:05 sudodoki

Cheers for looking into it. For now I'll just make the assumption that the copying was successful but it's not ideal going forwards

And re: the promise, it's not a deal breaker, just a nice to have 😄

MaffooBristol avatar May 18 '20 09:05 MaffooBristol

Same problem with me! When I remove the e.preventDefault() it works!

ronaldgj avatar Jun 22 '20 16:06 ronaldgj

Same problem here, tested with Firefox Browser Developer Edition 79.0b9(x64) ! When I remove/comment the e.preventDefault() line from index.js#L67, it works as expected then. This seems to be a bug.

BTW, why do we need the e.preventDefault() line please ? Can this line be safely removed ? @sudodoki

mike652638 avatar Jul 19 '20 04:07 mike652638

if you have onCopy handler that might be super useful to change format / etc. you don't want default handler to actually fire. The argument passed to onCopy can be used to actually do setData(format, data). By default copyToClipboard is sync, so no need for 'callback'.

So basically, providing onCopy you are opting-out of default copying mechanism of your browser and do all the work. If you only need this to provide format, consider using format option

sudodoki avatar Jul 21 '20 15:07 sudodoki