Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

fix: Copy address to clipboard on mobile devices #220

Closed
wants to merge 2 commits into from
Closed

fix: Copy address to clipboard on mobile devices #220

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f74d51
fix: Copy address to clipboard on mobile devices
Mar 12, 2021
f7fc71a
fix: Change function name
Mar 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/widgets/WalletModal/CopyToClipboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,33 @@ const Tooltip = styled.div<{ isTooltipDisplayed: boolean }>`
const CopyToClipboard: React.FC<Props> = ({ toCopy, children, ...props }) => {
const [isTooltipDisplayed, setIsTooltipDisplayed] = useState(false);

const copyToClipboard = (content: string) => {
const el = document.createElement("textarea");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nitpick - but we could perhaps use input instead of textarea. Even though in 99% of the cases it is too quick to notice anything - in case where user has super slow computer they might notice something in theory and input just takes less space on the screen.

el.value = content;
document.body.appendChild(el);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going to comment that we should specify el.style.display = "none" or el.style.visibility = "hidden" just to be safe. But - apparently el.select() doesn't work if the element is not actually visible on the screen.

Thankfully this happens quick enough so it is completely not noticeable for user.

el.select();
document.execCommand("copy");
document.body.removeChild(el);
};

function displayTooltip() {
setIsTooltipDisplayed(true);
setTimeout(() => {
setIsTooltipDisplayed(false);
}, 1000);
}

return (
<StyleButton
small
bold
onClick={() => {
if (navigator.clipboard) {
if (document.queryCommandSupported("copy")) {
copyToClipboard(toCopy);
displayTooltip();
} else if (navigator.clipboard) {
navigator.clipboard.writeText(toCopy);
setIsTooltipDisplayed(true);
setTimeout(() => {
setIsTooltipDisplayed(false);
}, 1000);
displayTooltip();
Comment on lines +54 to +59
Copy link

@ArtemKolichenkov ArtemKolichenkov Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This should be the fallback only for WebViews, iFrames and weird browsers, not the primary way to copy the address, not to mention it is (getting) deprecated so it's a bit icky to prioritize it.
    navigator.clipboard check should be first. And since technically Trust Wallet browser has navigator.clipboard defined, it fails because its a WebView where navigator.permissions is undefined (thus not having permission to copy), so we should check for that too.
  2. I know that it "works" as it is right now, but technically navigator.clipboard.writeText(toCopy) returns a promise, so we'd better handle it appropriately. Otherwise uses would see "Copied!" tooltip even if it didn't actually copied anything. Would be also nice to somehow handle error in .catch() but I guess (a) unless user uses extremely excotic browser it shouldn't happen (b) showing user "sorry we couldn't copy the address, browser is poop" is questionable 😄

EDIT: re-posted the comment to address multiple lines

Suggested change
if (document.queryCommandSupported("copy")) {
copyToClipboard(toCopy);
displayTooltip();
} else if (navigator.clipboard) {
navigator.clipboard.writeText(toCopy);
setIsTooltipDisplayed(true);
setTimeout(() => {
setIsTooltipDisplayed(false);
}, 1000);
displayTooltip();
if (navigator.clipboard && navigator.permissions) {
navigator.clipboard.writeText(toCopy).then(() => displayTooltip());
} else if (document.queryCommandSupported("copy")) {
copyToClipboard(toCopy);
displayTooltip();
}

}
}}
{...props}
Expand Down