Skip to content

Commit 087f973

Browse files
authored
refactor(site): clean up clipboard functionality and define tests (#12296)
* refactor: clean up and update API for useClipboard * wip: commit current progress on useClipboard test * docs: clean up wording on showCopySuccess * chore: make sure tests can differentiate between HTTP/HTTPS * chore: add test ID to dummy input * wip: commit progress on useClipboard test * wip: commit more test progress * refactor: rewrite code for clarity * chore: finish clipboard tests * fix: prevent double-firing for button click aliases * refactor: clean up test setup * fix: rename incorrect test file * refactor: update code to display user errors * refactor: redesign useClipboard to be easier to test * refactor: clean up GlobalSnackbar * feat: add functionality for notifying user of errors (with tests) * refactor: clean up test code * refactor: centralize cleanup steps
1 parent e183843 commit 087f973

File tree

10 files changed

+425
-63
lines changed

10 files changed

+425
-63
lines changed

site/src/components/CodeExample/CodeExample.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ export const CodeExample: FC<CodeExampleProps> = ({
2323
}) => {
2424
const buttonRef = useRef<HTMLButtonElement>(null);
2525
const triggerButton = (event: KeyboardEvent | MouseEvent) => {
26-
if (event.target !== buttonRef.current) {
26+
const clickTriggeredOutsideButton =
27+
event.target instanceof HTMLElement &&
28+
!buttonRef.current?.contains(event.target);
29+
30+
if (clickTriggeredOutsideButton) {
2731
buttonRef.current?.click();
2832
}
2933
};

site/src/components/CopyButton/CopyButton.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ export const CopyButton = forwardRef<HTMLButtonElement, CopyButtonProps>(
3232
buttonStyles,
3333
tooltipTitle = Language.tooltipTitle,
3434
} = props;
35-
const { isCopied, copyToClipboard } = useClipboard(text);
35+
const { showCopiedSuccess, copyToClipboard } = useClipboard({
36+
textToCopy: text,
37+
});
3638

3739
return (
3840
<Tooltip title={tooltipTitle} placement="top">
@@ -45,7 +47,7 @@ export const CopyButton = forwardRef<HTMLButtonElement, CopyButtonProps>(
4547
variant="text"
4648
onClick={copyToClipboard}
4749
>
48-
{isCopied ? (
50+
{showCopiedSuccess ? (
4951
<Check css={styles.copyIcon} />
5052
) : (
5153
<FileCopyIcon css={styles.copyIcon} />

site/src/components/CopyableValue/CopyableValue.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ export const CopyableValue: FC<CopyableValueProps> = ({
1616
children,
1717
...attrs
1818
}) => {
19-
const { isCopied, copyToClipboard } = useClipboard(value);
19+
const { showCopiedSuccess, copyToClipboard } = useClipboard({
20+
textToCopy: value,
21+
});
2022
const clickableProps = useClickable<HTMLSpanElement>(copyToClipboard);
2123

2224
return (
2325
<Tooltip
24-
title={isCopied ? "Copied!" : "Click to copy"}
26+
title={showCopiedSuccess ? "Copied!" : "Click to copy"}
2527
placement={placement}
2628
PopperProps={PopperProps}
2729
>

site/src/components/GlobalSnackbar/GlobalSnackbar.tsx

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,37 +24,37 @@ const variantFromMsgType = (type: MsgType) => {
2424
};
2525

2626
export const GlobalSnackbar: FC = () => {
27-
const [open, setOpen] = useState<boolean>(false);
28-
const [notification, setNotification] = useState<NotificationMsg>();
29-
27+
const [notificationMsg, setNotificationMsg] = useState<NotificationMsg>();
3028
useCustomEvent<NotificationMsg>(SnackbarEventType, (event) => {
31-
setNotification(event.detail);
32-
setOpen(true);
29+
setNotificationMsg(event.detail);
3330
});
3431

35-
if (!notification) {
32+
const hasNotification = notificationMsg !== undefined;
33+
if (!hasNotification) {
3634
return null;
3735
}
3836

3937
return (
4038
<EnterpriseSnackbar
41-
key={notification.msg}
42-
open={open}
43-
variant={variantFromMsgType(notification.msgType)}
44-
onClose={() => setOpen(false)}
45-
autoHideDuration={notification.msgType === MsgType.Error ? 22000 : 6000}
39+
key={notificationMsg.msg}
40+
open={hasNotification}
41+
variant={variantFromMsgType(notificationMsg.msgType)}
42+
onClose={() => setNotificationMsg(undefined)}
43+
autoHideDuration={
44+
notificationMsg.msgType === MsgType.Error ? 22000 : 6000
45+
}
4646
anchorOrigin={{ vertical: "bottom", horizontal: "right" }}
4747
message={
4848
<div css={{ display: "flex" }}>
49-
{notification.msgType === MsgType.Error && (
49+
{notificationMsg.msgType === MsgType.Error && (
5050
<ErrorIcon css={styles.errorIcon} />
5151
)}
5252

5353
<div css={{ maxWidth: 670 }}>
54-
<span css={styles.messageTitle}>{notification.msg}</span>
54+
<span css={styles.messageTitle}>{notificationMsg.msg}</span>
5555

56-
{notification.additionalMsgs &&
57-
notification.additionalMsgs.map((msg, index) => (
56+
{notificationMsg.additionalMsgs &&
57+
notificationMsg.additionalMsgs.map((msg, index) => (
5858
<AdditionalMessageDisplay key={index} message={msg} />
5959
))}
6060
</div>
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* This test is for all useClipboard functionality, with the browser context
3+
* set to insecure (HTTP connections).
4+
*
5+
* See useClipboard.test-setup.ts for more info on why this file is set up the
6+
* way that it is.
7+
*/
8+
import { useClipboard } from "./useClipboard";
9+
import { scheduleClipboardTests } from "./useClipboard.test-setup";
10+
11+
describe(useClipboard.name, () => {
12+
describe("HTTP (non-secure) connections", () => {
13+
scheduleClipboardTests({ isHttps: false });
14+
});
15+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* This test is for all useClipboard functionality, with the browser context
3+
* set to secure (HTTPS connections).
4+
*
5+
* See useClipboard.test-setup.ts for more info on why this file is set up the
6+
* way that it is.
7+
*/
8+
import { useClipboard } from "./useClipboard";
9+
import { scheduleClipboardTests } from "./useClipboard.test-setup";
10+
11+
describe(useClipboard.name, () => {
12+
describe("HTTPS (secure/default) connections", () => {
13+
scheduleClipboardTests({ isHttps: true });
14+
});
15+
});
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
/**
2+
* @file This is a very weird test setup.
3+
*
4+
* There are two main things that it's fighting against to insure that the
5+
* clipboard functionality is working as expected:
6+
* 1. userEvent.setup's default global behavior
7+
* 2. The fact that we need to reuse the same set of test cases for two separate
8+
* contexts (secure and insecure), each with their own version of global
9+
* state.
10+
*
11+
* The goal of this file is to provide a shared set of test behavior that can
12+
* be imported into two separate test files (one for HTTP, one for HTTPS),
13+
* without any risk of global state conflicts.
14+
*
15+
* ---
16+
* For (1), normally you could call userEvent.setup to enable clipboard mocking,
17+
* but userEvent doesn't expose a teardown function. It also modifies the global
18+
* scope for the whole test file, so enabling just one userEvent session will
19+
* make a mock clipboard exist for all other tests, even though you didn't tell
20+
* them to set up a session. The mock also assumes that the clipboard API will
21+
* always be available, which is not true on HTTP-only connections
22+
*
23+
* Since these tests need to split hairs and differentiate between HTTP and
24+
* HTTPS connections, setting up a single userEvent is disastrous. It will make
25+
* all the tests pass, even if they shouldn't. Have to avoid that by creating a
26+
* custom clipboard mock.
27+
*
28+
* ---
29+
* For (2), we're fighting against Jest's default behavior, which is to treat
30+
* the test file as the main boundary for test environments, with each test case
31+
* able to run in parallel. That works if you have one single global state, but
32+
* we need two separate versions of the global state, while repeating the exact
33+
* same test cases for each one.
34+
*
35+
* If both tests were to be placed in the same file, Jest would not isolate them
36+
* and would let their setup steps interfere with each other. This leads to one
37+
* of two things:
38+
* 1. One of the global mocks overrides the other, making it so that one
39+
* connection type always fails
40+
* 2. The two just happen not to conflict each other, through some convoluted
41+
* order of operations involving closure, but you have no idea why the code
42+
* is working, and it's impossible to debug.
43+
*/
44+
import {
45+
type UseClipboardInput,
46+
type UseClipboardResult,
47+
useClipboard,
48+
} from "./useClipboard";
49+
import { act, renderHook } from "@testing-library/react";
50+
import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
51+
52+
const initialExecCommand = global.document.execCommand;
53+
beforeAll(() => {
54+
jest.useFakeTimers();
55+
});
56+
57+
afterAll(() => {
58+
jest.restoreAllMocks();
59+
jest.useRealTimers();
60+
global.document.execCommand = initialExecCommand;
61+
});
62+
63+
type MockClipboardEscapeHatches = Readonly<{
64+
getMockText: () => string;
65+
setMockText: (newText: string) => void;
66+
simulateFailure: boolean;
67+
setSimulateFailure: (failureMode: boolean) => void;
68+
}>;
69+
70+
type MockClipboard = Readonly<Clipboard & MockClipboardEscapeHatches>;
71+
function makeMockClipboard(isSecureContext: boolean): MockClipboard {
72+
let mockClipboardValue = "";
73+
let shouldFail = false;
74+
75+
return {
76+
get simulateFailure() {
77+
return shouldFail;
78+
},
79+
setSimulateFailure: (value) => {
80+
shouldFail = value;
81+
},
82+
83+
readText: async () => {
84+
if (shouldFail) {
85+
throw new Error("Clipboard deliberately failed");
86+
}
87+
88+
if (!isSecureContext) {
89+
throw new Error(
90+
"Trying to read from clipboard outside secure context!",
91+
);
92+
}
93+
94+
return mockClipboardValue;
95+
},
96+
writeText: async (newText) => {
97+
if (shouldFail) {
98+
throw new Error("Clipboard deliberately failed");
99+
}
100+
101+
if (!isSecureContext) {
102+
throw new Error("Trying to write to clipboard outside secure context!");
103+
}
104+
105+
mockClipboardValue = newText;
106+
},
107+
108+
getMockText: () => mockClipboardValue,
109+
setMockText: (newText) => {
110+
mockClipboardValue = newText;
111+
},
112+
113+
addEventListener: jest.fn(),
114+
removeEventListener: jest.fn(),
115+
dispatchEvent: jest.fn(),
116+
read: jest.fn(),
117+
write: jest.fn(),
118+
};
119+
}
120+
121+
function renderUseClipboard(inputs: UseClipboardInput) {
122+
return renderHook<UseClipboardResult, UseClipboardInput>(
123+
(props) => useClipboard(props),
124+
{
125+
initialProps: inputs,
126+
wrapper: ({ children }) => (
127+
<>
128+
<>{children}</>
129+
<GlobalSnackbar />
130+
</>
131+
),
132+
},
133+
);
134+
}
135+
136+
type ScheduleConfig = Readonly<{ isHttps: boolean }>;
137+
138+
export function scheduleClipboardTests({ isHttps }: ScheduleConfig) {
139+
const mockClipboardInstance = makeMockClipboard(isHttps);
140+
141+
const originalNavigator = window.navigator;
142+
beforeAll(() => {
143+
jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
144+
...originalNavigator,
145+
clipboard: mockClipboardInstance,
146+
}));
147+
148+
if (!isHttps) {
149+
// Not the biggest fan of exposing implementation details like this, but
150+
// making any kind of mock for execCommand is really gnarly in general
151+
global.document.execCommand = jest.fn(() => {
152+
if (mockClipboardInstance.simulateFailure) {
153+
return false;
154+
}
155+
156+
const dummyInput = document.querySelector("input[data-testid=dummy]");
157+
const inputIsFocused =
158+
dummyInput instanceof HTMLInputElement &&
159+
document.activeElement === dummyInput;
160+
161+
let copySuccessful = false;
162+
if (inputIsFocused) {
163+
mockClipboardInstance.setMockText(dummyInput.value);
164+
copySuccessful = true;
165+
}
166+
167+
return copySuccessful;
168+
});
169+
}
170+
});
171+
172+
afterEach(() => {
173+
mockClipboardInstance.setMockText("");
174+
mockClipboardInstance.setSimulateFailure(false);
175+
});
176+
177+
const assertClipboardTextUpdate = async (
178+
result: ReturnType<typeof renderUseClipboard>["result"],
179+
textToCheck: string,
180+
): Promise<void> => {
181+
await act(() => result.current.copyToClipboard());
182+
expect(result.current.showCopiedSuccess).toBe(true);
183+
184+
const clipboardText = mockClipboardInstance.getMockText();
185+
expect(clipboardText).toEqual(textToCheck);
186+
};
187+
188+
/**
189+
* Start of test cases
190+
*/
191+
it("Copies the current text to the user's clipboard", async () => {
192+
const textToCopy = "dogs";
193+
const { result } = renderUseClipboard({ textToCopy });
194+
await assertClipboardTextUpdate(result, textToCopy);
195+
});
196+
197+
it("Should indicate to components not to show successful copy after a set period of time", async () => {
198+
const textToCopy = "cats";
199+
const { result } = renderUseClipboard({ textToCopy });
200+
await assertClipboardTextUpdate(result, textToCopy);
201+
202+
setTimeout(() => {
203+
expect(result.current.showCopiedSuccess).toBe(false);
204+
}, 10_000);
205+
206+
await jest.runAllTimersAsync();
207+
});
208+
209+
it("Should notify the user of an error using the provided callback", async () => {
210+
const textToCopy = "birds";
211+
const onError = jest.fn();
212+
const { result } = renderUseClipboard({ textToCopy, onError });
213+
214+
mockClipboardInstance.setSimulateFailure(true);
215+
await act(() => result.current.copyToClipboard());
216+
expect(onError).toBeCalled();
217+
});
218+
}

0 commit comments

Comments
 (0)