You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/08/30 12:41:49 UTC
[superset] 11/13: fix: Support the Clipboard API in modern browsers (#20058)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e5de0909c39349b7d270dab36d9b9b7f5f3a075b
Author: Diego Medina <di...@gmail.com>
AuthorDate: Fri Jun 3 07:34:00 2022 -0400
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 +-
.../menu/ShareMenuItems/ShareMenuItems.test.tsx | 10 +-
.../components/menu/ShareMenuItems/index.tsx | 3 +-
.../CopyToClipboardButton.test.tsx | 22 ++++-
.../DataTablesPane/DataTablesPane.test.tsx | 9 +-
.../explore/components/ExploreActionButtons.tsx | 3 +-
superset-frontend/src/utils/common.js | 8 +-
superset-frontend/src/utils/copy.ts | 105 ++++++++++++++-------
.../components/SyntaxHighlighterCopy/index.tsx | 2 +-
.../views/CRUD/data/savedquery/SavedQueryList.tsx | 6 +-
superset-frontend/src/views/CRUD/hooks.ts | 6 +-
11 files changed, 121 insertions(+), 59 deletions(-)
diff --git a/superset-frontend/src/components/CopyToClipboard/index.jsx b/superset-frontend/src/components/CopyToClipboard/index.jsx
index 00a23b1662..73cdc2b9e4 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 b196100734..e8f608e6fe 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(<CopyToClipboardButton data={{ copy: 'data', data: 'copy' }} />, {
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 380285b811..7f4f8d11e0 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
@@ -144,8 +144,9 @@ test('Should copy data table content correctly', async () => {
expect(await screen.findByText('1 rows retrieved')).toBeVisible();
userEvent.click(screen.getByRole('button', { name: 'Copy' }));
- expect(copyToClipboardSpy).toHaveBeenCalledWith(
- '2009-01-01 00:00:00\tAction\n',
- );
- fetchMock.done();
+ 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/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx
index adb85e11d5..8931321918 100644
--- a/superset-frontend/src/explore/components/ExploreActionButtons.tsx
+++ b/superset-frontend/src/explore/components/ExploreActionButtons.tsx
@@ -109,8 +109,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
const doCopyLink = async () => {
try {
setCopyTooltip(t('Loading...'));
- const url = await getChartPermalink(latestQueryFormData);
- await copyTextToClipboard(url);
+ await copyTextToClipboard(() => getChartPermalink(latestQueryFormData));
setCopyTooltip(t('Copied to clipboard!'));
addSuccessToast(t('Copied to clipboard!'));
} catch (error) {
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<void>((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();
- }
+import { isSafari } from './common';
+
+// Use the new Clipboard API if the browser supports it
+const copyTextWithClipboardApi = async (getText: () => Promise<string>) => {
+ // 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);
+ }
+};
+
+const copyTextToClipboard = (getText: () => Promise<string>) =>
+ copyTextWithClipboardApi(getText)
+ // If the Clipboard API is not supported, fallback to the older method.
+ .catch(() =>
+ getText().then(
+ text =>
+ new Promise<void>((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();
- });
+ 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 b0ca13d96a..6a503ee494 100644
--- a/superset-frontend/src/views/CRUD/hooks.ts
+++ b/superset-frontend/src/views/CRUD/hooks.ts
@@ -595,8 +595,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!'));