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(v2): CodeBlock copy button #1643

Merged
merged 2 commits into from
Jul 12, 2019
Merged

Conversation

bvego
Copy link
Contributor

@bvego bvego commented Jul 8, 2019

Motivation

Hi there. The motivation behind this was so many websites that don't have the copy button.
The design was pretty much eyeballed but is open for suggestions. I chose not to go with an icon here.

Attempts to close #1587

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  • bootstrap the dependencies with
  •  yarn
  • Start the project with
  •  yarn start
  • Navigate to any codeblock, i.e. http://localhost:3000/docs/installation
  • Hover the codeblock
  • Click the copy button

I've only wrapped the pre element with a div that acts as a wrapper and use it for anchoring the position of the copy button.

I've used the clipboard package for copying because I saw it elsewhere in the repo

Edit:
I also noticed that packages/docusaurus-theme-live-codeblock/src/theme/CodeBlock/styles.module.css and packages/docusaurus-theme-classic/src/theme/CodeBlock/styles.module.css are missing the copyright notice on the top.

Maybe it's indented but just wanted point it out if it slipped through.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 8, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 8, 2019

Deploy preview for docusaurus-2 ready!

Built with commit f0d052c

https://deploy-preview-1643--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 8, 2019

Deploy preview for docusaurus-preview ready!

Built with commit f0d052c

https://deploy-preview-1643--docusaurus-preview.netlify.com

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

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

@bvego
Copy link
Contributor Author

bvego commented Jul 9, 2019

Oh wow, what a blunder 😦

Pushed a fix, works now

@endiliey
Copy link
Contributor

endiliey commented Jul 9, 2019

cc @yangshun @wgao19 on UI feedback.

@endiliey endiliey requested review from yangshun and wgao19 July 10, 2019 15:11
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

The code for the component looks good to me. Thanks @bvego!

However, I wonder if there's a way we can avoid duplicating the buttons in theme-classic/CodeBlock and theme-live-codeblock. It's not really ideal to have so much duplication. WDYT @endiliey?

@endiliey
Copy link
Contributor

However, I wonder if there's a way we can avoid duplicating the buttons in theme-classic/CodeBlock and theme-live-codeblock. It's not really ideal to have so much duplication. WDYT @endilie

ideally if we want to avoid duplication we have to make one as another dependency. But i think its out of this PR scope.

@endiliey
Copy link
Contributor

lets merge this out first ?

@endiliey endiliey merged commit 7b7d1e6 into facebook:master Jul 12, 2019
@endiliey
Copy link
Contributor

Thanks a lot @bvego

you are awesome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copy code button for Docusaurus 2
5 participants