Add a copy button to code samples #231
Conversation
|
The docsbuild CI seems broken, could it be related to this change: python/docsbuild-scripts@2113fd7 ? |
|
Yep, fixed here in #232. I've updated the branch from |
There was a problem hiding this comment.
Thanks for the PR!
It's working well on a quick test on Chrome, Safari, Firefox and Opera on macOS, and Chrome on Android.
For now it's text but maybe we could use an icon instead?
An advantage of text over icons is it's more accessible and less open to misinterpretation and different cultural associations.
However, GitHub and Discourse use a double-square icon, so this may be familiar:
a = 1
b = 2
assert a < b
https://discuss.python.org/t/pep-781-make-type-checking-a-built-in-constant/85728/1
And when clicked, GitHub shows a tick and "Copied", Discourse shows "copied!". They also use aria-labels for accessibility.
Discord is similar and shows a tick when clicked:
But I'm fine with this text-only version, and an icon could also be a followup if we want that.
I would suggest to use
navigator.clipboardwhich has been supported by all major browsers for >5 years at this point.
Yes, that feels long enough, it has ~95% support and we don't need to support copy for older browsers. But what happens on a browser without support? As long as we don't break things completely.
| clearTimeout(timeout) | ||
| const buttonEl = event.currentTarget | ||
| const codeEl = buttonEl.nextElementSibling | ||
| navigator.clipboard.writeText(getCopyableText(codeEl)) |
There was a problem hiding this comment.
My IDE gives a warning that the returned promise is ignored:
Should we have a .then() and only do "Copied!" in there?
https://web.dev/patterns/clipboard/copy-text
And maybe also log any error, in case it happens?
There was a problem hiding this comment.
Thanks for testing it!
They also use aria-labels for accessibility.
I think that won't be necessary in our case? We're using a regular button with type="button" and text content.
But what happens on a browser without support?
It's gonna raise inside the click handler which shouldn't break anything else but I'll add some proper feature detection.
Should we have a .then() and only do "Copied!" in there?
Yep, that sounds like a safer option!
|
Looks like there's an issue with the docs workflow: |
|
Also happening on Not a new failure, but it only started failing the build recently! Here's the previous Anyway, don't worry about it for this PR :) |
Alright, just wanted to point it out in case you haven't noticed (unlikely, I know 😄 ) Anyway, I updated the PR based on your feedback - I added proper error handling and feature detection for |
I kept the style similar to the previous button. For now it's text but maybe we could use an icon instead?
Note that unlike the previous button, I added the copy button to all code samples, not only the interactive ones.
I'd like to know what you think!
Before:
After:
Single line:
copybtn.webm
📚 Documentation preview 📚: https://python-docs-theme-previews--231.org.readthedocs.build/