From 0e38c686c69dd7c8c5cb4a9b3615b395b02ae973 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Fri, 3 Jun 2022 07:34:00 -0400 Subject: [PATCH] fix: Support the Clipboard API in modern browsers (#20058) * fix: Support the Clipboard API in modern browsers * fix tests * PR comment * Improvements --- .../src/components/CopyToClipboard/index.jsx | 6 +- .../ShareMenuItems/ShareMenuItems.test.tsx | 10 +- .../components/menu/ShareMenuItems/index.tsx | 3 +- .../CopyToClipboardButton.test.tsx | 22 +++- .../DataTablesPane/DataTablesPane.test.tsx | 6 +- .../useExploreAdditionalActionsMenu/index.jsx | 3 +- superset-frontend/src/utils/common.js | 8 +- superset-frontend/src/utils/copy.ts | 103 ++++++++++++------ .../SyntaxHighlighterCopy/index.tsx | 2 +- .../CRUD/data/savedquery/SavedQueryList.tsx | 6 +- superset-frontend/src/views/CRUD/hooks.ts | 6 +- 11 files changed, 118 insertions(+), 57 deletions(-) diff --git a/superset-frontend/src/components/CopyToClipboard/index.jsx b/superset-frontend/src/components/CopyToClipboard/index.jsx index 2047cf4b0f..95b6cdfdc0 100644 --- a/superset-frontend/src/components/CopyToClipboard/index.jsx +++ b/superset-frontend/src/components/CopyToClipboard/index.jsx @@ -57,10 +57,10 @@ class CopyToClipboard extends React.Component { onClick() { if (this.props.getText) { this.props.getText(d => { - this.copyToClipboard(d); + this.copyToClipboard(Promise.resolve(d)); }); } else { - this.copyToClipboard(this.props.text); + this.copyToClipboard(Promise.resolve(this.props.text)); } } @@ -72,7 +72,7 @@ class CopyToClipboard extends React.Component { } copyToClipboard(textToCopy) { - copyTextToClipboard(textToCopy) + copyTextToClipboard(() => textToCopy) .then(() => { this.props.addSuccessToast(t('Copied to clipboard!')); }) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index 579f9d4b69..498009224a 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -102,9 +102,10 @@ test('Click on "Copy dashboard URL" and succeed', async () => { userEvent.click(screen.getByRole('button', { name: 'Copy dashboard URL' })); - await waitFor(() => { + await waitFor(async () => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); + const value = await spy.mock.calls[0][0](); + expect(value).toBe('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(1); expect(props.addSuccessToast).toBeCalledWith('Copied to clipboard!'); expect(props.addDangerToast).toBeCalledTimes(0); @@ -128,9 +129,10 @@ test('Click on "Copy dashboard URL" and fail', async () => { userEvent.click(screen.getByRole('button', { name: 'Copy dashboard URL' })); - await waitFor(() => { + await waitFor(async () => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); + const value = await spy.mock.calls[0][0](); + expect(value).toBe('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(0); expect(props.addDangerToast).toBeCalledTimes(1); expect(props.addDangerToast).toBeCalledWith( diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 92e5665aa0..f9016e5263 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -64,8 +64,7 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { async function onCopyLink() { try { - const url = await generateUrl(); - await copyTextToClipboard(url); + await copyTextToClipboard(generateUrl); addSuccessToast(t('Copied to clipboard!')); } catch (error) { logging.error(error); diff --git a/superset-frontend/src/explore/components/DataTableControl/CopyToClipboardButton.test.tsx b/superset-frontend/src/explore/components/DataTableControl/CopyToClipboardButton.test.tsx index 2ce91590b9..a158bfd7ed 100644 --- a/superset-frontend/src/explore/components/DataTableControl/CopyToClipboardButton.test.tsx +++ b/superset-frontend/src/explore/components/DataTableControl/CopyToClipboardButton.test.tsx @@ -18,7 +18,7 @@ */ import userEvent from '@testing-library/user-event'; import React from 'react'; -import { render, screen } from 'spec/helpers/testing-library'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; import { CopyToClipboardButton } from '.'; test('Render a button', () => { @@ -28,14 +28,26 @@ test('Render a button', () => { expect(screen.getByRole('button')).toBeInTheDocument(); }); -test('Should copy to clipboard', () => { - document.execCommand = jest.fn(); +test('Should copy to clipboard', async () => { + const callback = jest.fn(); + document.execCommand = callback; + + const originalClipboard = { ...global.navigator.clipboard }; + // @ts-ignore + global.navigator.clipboard = { write: callback, writeText: callback }; render(, { useRedux: true, }); - expect(document.execCommand).toHaveBeenCalledTimes(0); + expect(callback).toHaveBeenCalledTimes(0); userEvent.click(screen.getByRole('button')); - expect(document.execCommand).toHaveBeenCalledWith('copy'); + + await waitFor(() => { + expect(callback).toHaveBeenCalled(); + }); + + jest.resetAllMocks(); + // @ts-ignore + global.navigator.clipboard = originalClipboard; }); diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx index 76d4b6951d..57d599ee82 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx @@ -175,9 +175,9 @@ describe('DataTablesPane', () => { expect(await screen.findByText('1 row')).toBeVisible(); userEvent.click(screen.getByLabelText('Copy')); - expect(copyToClipboardSpy).toHaveBeenCalledWith( - '2009-01-01 00:00:00\tAction\n', - ); + expect(copyToClipboardSpy).toHaveBeenCalledTimes(1); + const value = await copyToClipboardSpy.mock.calls[0][0](); + expect(value).toBe('2009-01-01 00:00:00\tAction\n'); copyToClipboardSpy.mockRestore(); fetchMock.restore(); }); diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx index 6523473267..bebe5be269 100644 --- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx +++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx @@ -168,8 +168,7 @@ export const useExploreAdditionalActionsMenu = ( if (!latestQueryFormData) { throw new Error(); } - const url = await getChartPermalink(latestQueryFormData); - await copyTextToClipboard(url); + await copyTextToClipboard(() => getChartPermalink(latestQueryFormData)); addSuccessToast(t('Copied to clipboard!')); } catch (error) { addDangerToast(t('Sorry, something went wrong. Try again later.')); diff --git a/superset-frontend/src/utils/common.js b/superset-frontend/src/utils/common.js index 4efdb205e5..603ec7c549 100644 --- a/superset-frontend/src/utils/common.js +++ b/superset-frontend/src/utils/common.js @@ -94,7 +94,7 @@ export function prepareCopyToClipboardTabularData(data, columns) { for (let i = 0; i < data.length; i += 1) { const row = {}; for (let j = 0; j < columns.length; j += 1) { - // JavaScript does not mantain the order of a mixed set of keys (i.e integers and strings) + // JavaScript does not maintain the order of a mixed set of keys (i.e integers and strings) // the below function orders the keys based on the column names. const key = columns[j].name || columns[j]; if (data[i][key]) { @@ -145,4 +145,10 @@ export const detectOS = () => { return 'Unknown OS'; }; +export const isSafari = () => { + const { userAgent } = navigator; + + return userAgent && /^((?!chrome|android).)*safari/i.test(userAgent); +}; + export const isNullish = value => value === null || value === undefined; diff --git a/superset-frontend/src/utils/copy.ts b/superset-frontend/src/utils/copy.ts index 7db289c040..0980f2ab17 100644 --- a/superset-frontend/src/utils/copy.ts +++ b/superset-frontend/src/utils/copy.ts @@ -17,40 +17,79 @@ * under the License. */ -const copyTextToClipboard = async (text: string) => - new Promise((resolve, reject) => { - const selection: Selection | null = document.getSelection(); - if (selection) { - selection.removeAllRanges(); - const range = document.createRange(); - const span = document.createElement('span'); - span.textContent = text; - span.style.position = 'fixed'; - span.style.top = '0'; - span.style.clip = 'rect(0, 0, 0, 0)'; - span.style.whiteSpace = 'pre'; +import { isSafari } from './common'; - document.body.appendChild(span); - range.selectNode(span); - selection.addRange(range); - - try { - if (!document.execCommand('copy')) { - reject(); - } - } catch (err) { - reject(); - } - - document.body.removeChild(span); - if (selection.removeRange) { - selection.removeRange(range); - } else { - selection.removeAllRanges(); - } +// Use the new Clipboard API if the browser supports it +const copyTextWithClipboardApi = async (getText: () => Promise) => { + // Safari (WebKit) does not support delayed generation of clipboard. + // This means that writing to the clipboard, from the moment the user + // interacts with the app, must be instantaneous. + // However, neither writeText nor write accepts a Promise, so + // we need to create a ClipboardItem that accepts said Promise to + // delay the text generation, as needed. + // Source: https://bugs.webkit.org/show_bug.cgi?id=222262P + if (isSafari()) { + try { + const clipboardItem = new ClipboardItem({ + 'text/plain': getText(), + }); + await navigator.clipboard.write([clipboardItem]); + } catch { + // Fallback to default clipboard API implementation + const text = await getText(); + await navigator.clipboard.writeText(text); } + } else { + // For Blink, the above method won't work, but we can use the + // default (intended) API, since the delayed generation of the + // clipboard is now supported. + // Source: https://bugs.chromium.org/p/chromium/issues/detail?id=1014310 + const text = await getText(); + await navigator.clipboard.writeText(text); + } +}; - resolve(); - }); +const copyTextToClipboard = (getText: () => Promise) => + copyTextWithClipboardApi(getText) + // If the Clipboard API is not supported, fallback to the older method. + .catch(() => + getText().then( + text => + new Promise((resolve, reject) => { + const selection: Selection | null = document.getSelection(); + if (selection) { + selection.removeAllRanges(); + const range = document.createRange(); + const span = document.createElement('span'); + span.textContent = text; + span.style.position = 'fixed'; + span.style.top = '0'; + span.style.clip = 'rect(0, 0, 0, 0)'; + span.style.whiteSpace = 'pre'; + + document.body.appendChild(span); + range.selectNode(span); + selection.addRange(range); + + try { + if (!document.execCommand('copy')) { + reject(); + } + } catch (err) { + reject(); + } + + document.body.removeChild(span); + if (selection.removeRange) { + selection.removeRange(range); + } else { + selection.removeAllRanges(); + } + } + + resolve(); + }), + ), + ); export default copyTextToClipboard; diff --git a/superset-frontend/src/views/CRUD/data/components/SyntaxHighlighterCopy/index.tsx b/superset-frontend/src/views/CRUD/data/components/SyntaxHighlighterCopy/index.tsx index 73a0f8e9cc..8e96b95f0d 100644 --- a/superset-frontend/src/views/CRUD/data/components/SyntaxHighlighterCopy/index.tsx +++ b/superset-frontend/src/views/CRUD/data/components/SyntaxHighlighterCopy/index.tsx @@ -65,7 +65,7 @@ export default function SyntaxHighlighterCopy({ language: 'sql' | 'markdown' | 'html' | 'json'; }) { function copyToClipboard(textToCopy: string) { - copyTextToClipboard(textToCopy) + copyTextToClipboard(() => Promise.resolve(textToCopy)) .then(() => { if (addSuccessToast) { addSuccessToast(t('SQL Copied!')); diff --git a/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx b/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx index df3f16a858..d2dc6aff9c 100644 --- a/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx +++ b/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx @@ -210,8 +210,10 @@ function SavedQueryList({ const copyQueryLink = useCallback( (id: number) => { - copyTextToClipboard( - `${window.location.origin}/superset/sqllab?savedQueryId=${id}`, + copyTextToClipboard(() => + Promise.resolve( + `${window.location.origin}/superset/sqllab?savedQueryId=${id}`, + ), ) .then(() => { addSuccessToast(t('Link Copied!')); diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 18349b305c..ed49da1e8c 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -611,8 +611,10 @@ export const copyQueryLink = ( addDangerToast: (arg0: string) => void, addSuccessToast: (arg0: string) => void, ) => { - copyTextToClipboard( - `${window.location.origin}/superset/sqllab?savedQueryId=${id}`, + copyTextToClipboard(() => + Promise.resolve( + `${window.location.origin}/superset/sqllab?savedQueryId=${id}`, + ), ) .then(() => { addSuccessToast(t('Link Copied!'));