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

copy button #177

Merged
merged 2 commits into from
Nov 15, 2023
Merged

copy button #177

merged 2 commits into from
Nov 15, 2023

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Nov 15, 2023

  • client.js only
  • visual feedback with css
  • uses navigator.clipboard.writeText
  • adds the copy button on all PRE > CODE (or even PRE > SPAN) elements
  • pass data-copy=off to disable

supersedes #138

For an example of a reactive block with a copy button, you can type this in your page.md:

<pre><code>const a = ${now};</code></pre>

to consider:

  • should we put this outside of client.js, so as to make it optional?
  • can we add an option in code block syntax (#28) to say copy=no (it should set data-copy=off on the <pre>)? (this could be added later if we want, the js code already backs off if data-copy is not undefined).

closes #17

- client.js only
- visual feedback with css
- uses navigator.clipboard.writeText
- adds the copy button on all PRE > CODE (or even PRE > SPAN) elements
- pass data-copy=off to disable

supersedes #138
@Fil Fil requested a review from trebor November 15, 2023 13:42
@mbostock
Copy link
Member

I’m also a fan of this inline approach. Thanks @Fil and @trebor.

(And eventually we should have bundling for the client #61 so we can start to organize the client code as it grows… though hopefully it continues to stay pretty small! And maybe we could use lazy loading for enhancements like this if we ever feel we need to reduce the amount of code in the initial load.)

Copy link
Contributor

@trebor trebor left a comment

Choose a reason for hiding this comment

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

oh, this is much cleaner! i will close out #138. thank you @Fil!

@trebor
Copy link
Contributor

trebor commented Nov 15, 2023

should we put this outside of client.js, so as to make it optional?

is the concern bloat in client.js, or the ability to disable the functionality?

@Fil Fil enabled auto-merge (squash) November 15, 2023 17:27
@Fil
Copy link
Contributor Author

Fil commented Nov 15, 2023

Yes, if someone disables the feature project-wide, it would be nice not to bundle this part of client.js; but it's small anyways.

@Fil Fil merged commit aca0666 into main Nov 15, 2023
1 check passed
@Fil Fil deleted the fil/copy-button branch November 15, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy buttons for code blocks
3 participants