fix: persist synthetic event

[dan-lee]

What kind of change does this PR introduce?

• bugfix
• feature
• refactoring / style
• build / chore
• documentation

Did you add tests for your changes?

• Yes, my code is well tested
• Not relevant
• Don't know how

If relevant, did you update the documentation?

• Yes, I've updated the documentation
• Not relevant

Summary

Since v4.3 the event passed to the handler is not reusable anymore, because some work is done with it before. To prevent this evt.persist() is called.

Fixes #646

Does this PR introduce a breaking change?
no

Other information
I don't really know how to test this.

[codecov]

Codecov Report

Merging #657 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
+ Coverage   99.04%   99.05%   +<.01%     
==========================================
  Files           3        3              
  Lines         209      211       +2     
  Branches       60       60              
==========================================
+ Hits          207      209       +2     
  Misses          2        2

Impacted Files	Coverage Δ
src/index.js	99.43% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ad99a7...eab68eb. Read the comment docs.

[tomjn]

I tried something similar to fix that issue, it didn't work but I think the problem was I didn't do it in all the places you did :)

[tomjn]

On further review, is it not needed in onDragStart, onDragOver, onDragLeave, and onClick? Also I would push the persist call further up, ideally with the preventDefault and stop propagation calls

[dan-lee]

On further review, is it not needed in onDragStart, onDragOver, onDragLeave, and onClick

Oh so the only place should be onDrop then, I guess? I just adjusted all the places where getDataTransferItems 'manipulated' the event.

Also I would push the persist call further up, ideally with the preventDefault and stop propagation calls

Fair point! Will change it

[rxmarbles]

LGTM ! 👍

[okonet]

@dan-lee can you add tests for this?

[dan-lee]

@okonet I really would love to! But this behaviour doesn't seem to be reflected in the unit tests. The event is always populated with dataTransfer, which in practice it shouldn't. I tried a lot of things, but couldn't find a way to break it.

[okonet]

Hmm, I guess we can merge this and add tests later then.

_{Sent with GitHawk}