React Code Snippet copy button not working

[ashharrison90]

What package(s) are you using?

• [ ] carbon-components
• [x] carbon-components-react

Detailed description

Describe in detail the issue you're having

• go to http://react.carbondesignsystem.com/?path=/story/codesnippet--single-line
• click the copy button
• paste it somewhere else

Is this issue related to a specific component?

• CodeSnippet, but may affect other components that are using the copy button

What did you expect to happen? What happened instead? What would you like to see changed?

• I expected it to copy the text snippet. It didn't.

What browser are you working in?

• Chrome Version 77.0.3865.120 (Official Build) (64-bit)
• MacOS Catalina

What version of the Carbon Design System are you using?

• Whatever's on your demo page

What offering/product do you work on? Any pressing ship or release dates we should be aware of?

[jendowns]

Hey @ashharrison90, the CodeSnippet takes an onClick prop that allows you to pass a click handler to the underlying CopyButton --

https://github.com/carbon-design-system/carbon/blob/0dd565e6d8c84b32cfd8ad069cf9d7be1edbe8b5/packages/react/src/components/CodeSnippet/CodeSnippet.js#L46-L50

By default in the CopyButton component, it is empty:

https://github.com/carbon-design-system/carbon/blob/0dd565e6d8c84b32cfd8ad069cf9d7be1edbe8b5/packages/react/src/components/CopyButton/CopyButton.js#L51

So it seems that the storybook behavior for the CopyButton in the CodeSnippet is default.

[ashharrison90]

Oh whoops! Missed that. 😂 What's the reasoning behind a consumer having to implement a copy function themselves rather than just putting it in this library? 🤔

Also if its optional, I think CodeSnippet should hide the copy button if no function is passed in.

[jendowns]

Yeah no worries! To answer your question about reasoning for not including it, I'm not sure. 😅 (@asudoh do you know?)

[asudoh]

IIRC we historically let application code to do the actual copying due to the nature of requiring 3rd party library (back then). This may have changed now and feel free to implement the copying if anybody finds a cross-browser/lightweight way to do that.

[stale[bot]]

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions.

[stale[bot]]

As there's been no activity since this issue was marked as stale, we are auto-closing it.

[jeff-arn]

@asudoh I'd be interested in making a PR to add default copy functionality. We can either implement the copy ourselves using document.exec() with the appropriate fallbacks, or we can use an existing npm module like copy-to-clipboard to handle that for us.

My thinking if this change was supported would be to provide a default behavior for onClick with the option of letting the consumer override the default behavior by providing their own onClick prop. That way we wouldn't be introducing a breaking change by changing the prop interface.

If you agree I can look into adding this.

[asudoh]

If you want to try, probably something like below:

const range = doc!.createRange();
range.selectNodeContents((The DOM element, e.g. <code>, that contains the code snippet));
selection!.addRange(range);
doc!.execCommand('copy');
selection!.removeAllRanges();

Or in case above approach fails:

const selection = doc!.defaultView!.getSelection();
selection!.removeAllRanges();
const code = doc!.createElement('code');
code.className = `${prefix}--visually-hidden`;
const pre = doc!.createElement('pre');
pre.textContent = (The DOM element that contains the code snippet).textContent;
code.appendChild(pre);
doc!.body.appendChild(code);
const range = doc!.createRange();
range.selectNodeContents(code);
selection!.addRange(range);
doc!.execCommand('copy');
doc!.body.removeChild(code);
selection!.removeAllRanges();

[guigueb]

@asudoh / @repjarms I thank you two for continuing to address this. We - IBM Cognos - have also ran into this issue and will benefit from the fix.

I would like to make a request. In @repjarms's comment on Jan 30 the it was mentioned to override the onClick - and I agree this is needed. But I think there's also a case for not overriding but to propagate the onClick.

Usecase (we have this situation):

• client calls CodeSnippet with an onClick
• client wants the copy-to-clipboard to happen and also receive an onClick event to do some other task

I'm not sure the best way to accomplish this. Maybe an additional prop to indicating if the copy-to-clipboard should happen (or not happen). const copyToClipboard = this.props.copyToClipboard ?? this.props.onClick ? false : true;