-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add copy code to clipboard button for rustdoc
#119929
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @fmease (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, I've only skimmed your PR so far and have a couple of comments. Could you please squash the 8 commits into one (or at least fewer) and ./x fmt
your changes (that's why CI has failed).
@@ -1734,6 +1734,37 @@ href="https://doc.rust-lang.org/${channel}/rustdoc/read-documentation/search.htm | |||
resizer.addEventListener("pointerdown", initResize, false); | |||
}()); | |||
|
|||
// This section handles the copy buttons that appears in top right corner of the code blocks | |||
(function() { | |||
let preElements = document.querySelectorAll("pre:has(code)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:has
is not supported in Firefox Mobile: https://caniuse.com/css-has
let preElements = document.querySelectorAll("pre:has(code)"); | ||
preElements.forEach(function(pre, index) { | ||
let resetTimeout = null; | ||
let copyBtn = pre.querySelector("button[class='copy-code']"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let copyBtn = pre.querySelector("button[class='copy-code']"); | |
let copyBtn = pre.querySelector("button.copy-code"); |
let copyBtn = pre.querySelector("button[class='copy-code']"); | ||
let icon = copyBtn.innerHTML; | ||
pre.addEventListener("mouseenter", function() { | ||
copyBtn.style.opacity = "1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to express this in pure CSS with:
pre:hover > button.copy-code { opacity: 1; }
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged. You can start a rebase with the following commands:
The following commits are merge commits: |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@fmease I messed up some things, Im closing this PR and I'll make a new one. Sorry .. 🥲 |
All good, now worries! You can write |
@grey4owl had you opened a new MR about this? Thank u! |
A few things to take into account for this feature:
|
I implemented the feature. Just needs some UI tests and then I'll send a PR. |
Forgot to mention but PR is open: #125779 |
A very simple button that was missing in the upper right corner to copy the code blocks to the clipboard. It is hidden by default, but when you hover over the code block (
<pre>
) element then it appears.. It also works on mobile devices and it will appears after you tap on the code block.It looks like this: