Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Support the Clipboard API in modern browsers #20058

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions superset-frontend/src/components/CopyToClipboard/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand All @@ -72,7 +72,7 @@ class CopyToClipboard extends React.Component {
}

copyToClipboard(textToCopy) {
copyTextToClipboard(textToCopy)
copyTextToClipboard(() => textToCopy)
.then(() => {
this.props.addSuccessToast(t('Copied to clipboard!'));
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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;
});
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.'));
Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/utils/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -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;
105 changes: 72 additions & 33 deletions superset-frontend/src/utils/copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

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

nit: it could clean up the code by breaking the old logic into a separate function

.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;
Original file line number Diff line number Diff line change
Expand Up @@ -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!'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!'));
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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!'));
Expand Down