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

feat(CodeSnippet): allow copy button to be optional #6505

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Jul 20, 2020

Closes #6499

This PR allows the copy button to be optionally hidden from single and multiline code snippet. For inline code snippets, a span will be rendered instead of a button

Testing / Reviewing

Confirm the code snippet variants without copy buttons appear correct

@netlify
Copy link

netlify bot commented Jul 20, 2020

Deploy preview for carbon-elements ready!

Built with commit 2d57752

https://deploy-preview-6505--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jul 20, 2020

Deploy preview for carbon-components-react ready!

Built with commit 2d57752

https://deploy-preview-6505--carbon-components-react.netlify.app

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Functionality of showing and hiding the copy is working 👍🏻

When we hide the copy button for single and multi-line code snippets the padding on the left should change to allow more text to fill the empty space.

Spec:
Artboard

@emyarod emyarod force-pushed the 6499-code-snippet-hide-copy-button-option branch from bb079b3 to 4fbe6a2 Compare July 21, 2020 16:17
@emyarod emyarod requested a review from laurenmrice July 21, 2020 16:33
@emyarod emyarod force-pushed the 6499-code-snippet-hide-copy-button-option branch from 1a57c6e to 78b6fb5 Compare July 21, 2020 16:34
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Inline code snippet
-The inline code snippet increases in height when the no copy button prop is turned on.

Single line code snippet
-The overflow text is getting a hard cutoff instead of having the gradient when you horizontal scroll.

@emyarod
Copy link
Member Author

emyarod commented Jul 21, 2020

the inline code snippet height change should be resolved now

the gradient does not appear in Netlify because of CSS custom properties (related #4426) and this is not a regression in this PR. but I have implemented a workaround at the component level (#6410 for example) and I can resolve this issue for gradients in the other affected components in a separate PR

@emyarod emyarod requested a review from laurenmrice July 21, 2020 21:07
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

looks great! thank you 🙌🏻

Copy link
Member

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

@tw15egan
Copy link
Member

@emyarod it looks like a border: none was removed. Was that intentional? Don't see anything referring to the border so just wanted to make sure before I approve the visual diff

@emyarod
Copy link
Member Author

emyarod commented Jul 27, 2020

cc @laurenmrice to double check the code snippet borders (single and multiline since it looks like they are the only variants that import border styles and then override them)

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Actually yeah, now there is a border showing up on the single line code snippet. it needs to be removed.
Screen Shot 2020-07-28 at 10 34 02 AM

@emyarod
Copy link
Member Author

emyarod commented Jul 28, 2020

does it also not belong on the multiline snippet? if so we can change the default mixin styles since they're currently being overridden in both variants

@laurenmrice
Copy link
Member

laurenmrice commented Jul 28, 2020

It doesn't need to be used for any of the code snippet types. @emyarod

@emyarod emyarod force-pushed the 6499-code-snippet-hide-copy-button-option branch from db325d0 to 2d57752 Compare July 28, 2020 16:00
@emyarod emyarod requested a review from laurenmrice July 28, 2020 16:07
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

looks good! thank you 👍🏻

@kodiakhq kodiakhq bot merged commit ad4d9a2 into carbon-design-system:master Jul 28, 2020
@emyarod emyarod deleted the 6499-code-snippet-hide-copy-button-option branch July 28, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CodeSnippet: Ability to hide copy button
4 participants