Skip to content
This repository has been archived by the owner on Jun 18, 2019. It is now read-only.

Quickfix: select copyEl before copying text #33

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Conversation

garrettr
Copy link
Contributor

@garrettr garrettr commented Jun 9, 2017

This is a quickfix for #29. I'm fine with merging this for now, but also think the technique of adding the share text as a hidden input element and using document.execCommand('copy') is a bit of a hack, and there might be a cleaner way to do this using Electron's Clipboard API.

@GabeIsman Did you try using Electron's Clipboard API at any point while implementing this feature? Wondering if there's some reason why it wouldn't work (obviously I am fairly novice when it comes to both Electron and React).

@GabeIsman
Copy link
Contributor

I can't remember exactly what the reasoning was, but I think the Electron API was still marked experimental at the time, and I was thinking that a pure web-technologies solution would be more stable. Although somehow it broke anyway. I don't see anything wrong with using the Clipboard API, except that the integration tests will need to be modified.

@garrettr
Copy link
Contributor Author

garrettr commented Jun 9, 2017

I can't remember exactly what the reasoning was, but I think the Electron API was still marked experimental at the time

Sounds reasonable. FWIW, some of the Electron Clipboard API methods are still marked experimental as of Electron 1.6.10, but the only one we need, writeText, is not one of them.

@garrettr garrettr merged commit 8920c4f into master Jun 13, 2017
@garrettr garrettr deleted the fix-copying-shares branch June 13, 2017 18:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants