Fix copy to clipboard is available only for rust snippets #447
Conversation
|
Thanks for taking this up @Listwon 👍 ! There is just one caveat. The added clip button collides with already present buttons for rust code snippets. Most likely the code for rust playpens and other code blocks could use unification/refactoring. |
There was a problem hiding this comment.
Also I am not sure if we don't want the clipboard support to be opt-in (the clipboard buttons look a little intrusive on pages with a lot of single line non rust code blocks like http://localhost:3000/cli/build.html) what do you think @azerupi?
| @@ -234,7 +246,7 @@ $( document ).ready(function() { | |||
| var clipboardSnippets = new Clipboard('.clip-button', { | |||
| text: function(trigger) { | |||
| hideTooltip(trigger); | |||
| let playpen = $(trigger).parents(".playpen"); | |||
| let playpen = $(trigger).parents("pre"); | |||
There was a problem hiding this comment.
most likely we would no longer name this variable playpen
|
|
||
| $("pre code").each(function(i, block){ | ||
| var pre_block = $(this).parent(); | ||
| if( !pre_block.hasClass('playpen') ) { |
There was a problem hiding this comment.
I guess that we would prefer to have the button dock for each code/pre instead of specifically in the playgrounds. And only add additional buttons to (other than clipboard) to already existing button bar for rust specific snippets
There was a problem hiding this comment.
I'll wait for your comments then.
|
@budziq yeah, my bad, it needs fixing and refactoring. I need to study the code more deeply :) Thanks for quick testing and feedback! |
|
Hi @Listwon, did you ever get a chance to revisit this? It seems like a nice little feature to have. Especially for things like The Book, being able to copy an entire snippet at the click of a button would be quite convenient. |
|
Hi @Michael-F-Bryan, sorry I didn't revisit it earlier. I've just updated book.js to the newest version and fixed the bug found by @budziq with duplicating copy buttons. @budziq please test this again, I don't see other bugs for now. If I'm not mistaken, adding opt-in requires changes in the Rust code in which I'm not fluent yet. I'll try to find the time for some refactoring in this part, but for now it's working as I wanted ;) |
If you need anything from the Rust side of things, let me know. We just landed a PR (#457) which makes configuration a lot easier to work with, so that should greatly simplify the job of adding a new configuration item and making sure the renderer has access to it. |
|
I think this would also be a great feature to have, and in fact something I am writing right now would benefit from this. Are there any plans to get the ball rolling on this again, or is there anything I can do to help? |
|
@donald-pinckney, it looks like most of the work for this feature is done and as far as I can tell everything looks okay. I've got a couple spare hours, so I'm going to check out the changes locally and do a proper review now. If there aren't any issues then this PR will probably be good to merge. |
|
From what I can tell this looks good to me. Copying works, it pops up a little "Copied!" tooltip, and the clipboard icon no longer collides with the other buttons. |
|
Awesome, thanks! |
Fix copy to clipboard is available only for rust snippets
Fixes #432