Added copy to clipboard button for console tab commands #2431
Conversation
|
This looks nice 🌻 I'd recommend installing git hooks (you can follow the last section of the readme) and rebasing the PR to have a bit more clear history. |
aadeina
force-pushed
the
docs-copy-to-clipboard
branch
from
ad9d2d6 to
24e3b21
Compare
aadeina
force-pushed
the
docs-copy-to-clipboard
branch
from
bb03309 to
07c155d
Compare
aadeina
force-pushed
the
docs-copy-to-clipboard
branch
from
c22de90 to
cd6cf4c
Compare
|
Hi @ulgens , |
adamzap
left a comment
adamzap
left a comment
There was a problem hiding this comment.
Here are some initial thoughts. Thanks for attempting this!
| }); | ||
|
|
||
| function on_success(el) { | ||
| function on_success() { |
There was a problem hiding this comment.
Why add an unused argument here and on line 145?
There was a problem hiding this comment.
Good catch! Just to clarify — I actually removed the unused el parameter here, since it wasn't being used (the functions access success_el via closure instead). Happy to keep it if you prefer consistency with other patterns in the codebase.
There was a problem hiding this comment.
Oh, thanks! Sorry for my misunderstanding here.
| // Console tabs: extract text excluding prompts (.gp) and output (.go) | ||
| const pre_el = console_section.querySelector('.highlight pre'); | ||
| text = ''; | ||
| if (pre_el) { |
There was a problem hiding this comment.
In what case is there not a <pre> element?
There was a problem hiding this comment.
You're right — since Pygments always generates the <pre> element, this check isn't strictly necessary.
I'll remove it to keep the code cleaner. Thanks for pointing this out!
| const pre_el = console_section.querySelector('.highlight pre'); | ||
| text = ''; | ||
| if (pre_el) { | ||
| pre_el.childNodes.forEach(function (node) { |
There was a problem hiding this comment.
Do you think it would be more elegant to grab the text content like the old code was doing and just left trim the prompt?
(...).textContent.trim().replace(/^\$ /, '')There was a problem hiding this comment.
Great question! I went with the node-based approach mainly because:
- It also handles the Windows tab (which uses
...>prompts) - It excludes command output marked with
.go(in case any examples show output)
That said, if the console blocks only ever show simple $ prompts without output, a regex would definitely be simpler. I'm happy to switch to:
text = pre_el.textContent.trim().replace(/^(\$ |\.\.\.>)/gm, '');There was a problem hiding this comment.
Let me know which approach you'd prefer!
aadeina
force-pushed
the
docs-copy-to-clipboard
branch
from
cd6cf4c to
b3cf678
Compare
|
Updated per feedback — removed the |
3 participants
Summary
Adds a copy-to-clipboard button to console tab code blocks (Unix / Windows installation commands).
Fixes: #1276, #2399
Builds on feedback from @sabderemane in PR #1434.
Changes
JavaScript
djangoproject/static/js/djangoproject.js.gp— e.g.$,...>).go)SCSS
djangoproject/scss/_console-tabs.scssposition: relativeto console tab sections.btn-clipboardstyling using absolute positioning (nofloat)Visual Comparison
Before
After
Video.Project.mp4