onClick() callback prevents text from being copied

[MaffooBristol]

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]

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

[sudodoki]

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]

@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

[MaffooBristol]

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 😄

[ronaldgj]

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

[mike652638]

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

[sudodoki]

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