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

feat: add copy button for markdown code blocks #2799

Merged
merged 26 commits into from
Apr 11, 2024

Conversation

chayandatta
Copy link
Contributor

@chayandatta chayandatta commented Feb 2, 2024

fixes: #2668

image
image

Thanks @pankgeorg for all the help

Copy link
Contributor

github-actions bot commented Feb 2, 2024

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/chayandatta/Pluto.jl", rev="cd/copy-button")
julia> using Pluto

@chayandatta chayandatta changed the title feat: add copy button feat: add copy button for markdown code blocks Feb 2, 2024
@fonsp
Copy link
Owner

fonsp commented Feb 6, 2024

Heyyy that's cool! Could you reupload the screenshots? The URLs are from a private repo

@fonsp
Copy link
Owner

fonsp commented Feb 6, 2024

I would like to also use this functionality in more places!

  • inside the Live Docs panel
  • in code blocks that are used inside of a PlutoUI.ExperimentalLayout.hbox

Do you think it is possible to move this functionality to the RawHTMLContainer component? You could take a look at where we call highlight (which uses highlight.js to syntax highlight code blocks). Maybe this can be done in the same place?

Als try to extract this functionality into a new function, just like highlight.

@chayandatta chayandatta marked this pull request as draft February 8, 2024 07:03
@chayandatta chayandatta marked this pull request as ready for review February 8, 2024 13:21
@chayandatta
Copy link
Contributor Author

chayandatta commented Feb 8, 2024

Hi, I've moved the code inside RawHTMLContainer and extracted the functionality into a new function.

You could take a look at where we call highlight (which uses highlight.js to syntax highlight code blocks). Maybe this can be done in the same place?

As far I can understand, where hightlight() is being called, It can be accessed only when the notebook is running, not on static mode.

and generateCopyCodeButton generates an element of Type 'ReactElement' and it is unable to prepend(container.prepend()) with the element container because it's type HTMLElement.

@fonsp
Copy link
Owner

fonsp commented Feb 8, 2024

Thanks for taking a look again!

This is not exactly what I meant. 🙈 Instead of using regex to find the code blocks, and then a preact element for the button, consider using container.querySelectorAll to find the code blocks, and document.createElement to create the button.

In CellOutput.js, add this block:

				// EXISTING:
                // Apply syntax highlighting
                try {
					... highlight ...
				} catch ... {}

				// NEW:
				// Find code blocks and add a copy button:
                try {
                    if (container.firstElementChild?.matches("div.markdown")) {
                        container.querySelectorAll("pre > code").forEach((code_element) => {
                            const pre = code_element.parentElement
                            add_code_copy_button(pre)
                        })
                    }
                } catch (err) {
                    console.warn("Adding markdown code copy button failed", err)
                }

Then the add_code_copy_button uses document.createElement to create the button, and appendChild to add it to the page.

@fonsp
Copy link
Owner

fonsp commented Feb 8, 2024

As far I can understand, where hightlight() is being called, It can be accessed only when the notebook is running, not on static mode.

It's true that highlight is not called when the notebook is in "safe mode". Let's think about that later!

frontend/components/CellOutput.js Outdated Show resolved Hide resolved
frontend/components/CellOutput.js Outdated Show resolved Hide resolved
@fonsp
Copy link
Owner

fonsp commented Feb 14, 2024

Nice! It was currently positioned relative to the cell, not relative to the <pre>. I fixed that, you can see it when you have multiple code blocks in one cell.

Can you make the button look a bit nicer in dark mode?

image

In general, this button should match the styling of all other buttons in the Pluto UI. Carefully take a look at their border, background, light/dark theme, opacity, visibility-on-hover behaviour (simulate a touch screen with dev tools), padding. You can probably get all this behaviour for free if you add selectors to our existing CSS styles.

We usually give buttons an image with the ::before { background-image: url trick. Again, take a look at the other buttons to see how this was done. (Maybe this is a tip in general for future PRs! Try to find existing functionality that achieves some of the same goals that you want, and try to reuse or copy their implementation.)

@chayandatta chayandatta marked this pull request as draft February 20, 2024 11:14
@fonsp fonsp added the frontend Concerning the HTML editor label Feb 26, 2024
@fonsp fonsp added the enhancement New feature or request label Feb 26, 2024
@chayandatta
Copy link
Contributor Author

Kapture.2024-03-13.at.17.08.33.mp4

@chayandatta chayandatta marked this pull request as ready for review March 13, 2024 11:41
@fonsp
Copy link
Owner

fonsp commented Apr 11, 2024

Thanks!

image

The button was positioned relative to the last line of code (with pre.appendChild(button) it ends up at the end of the child list, and top: -20px moves it up.) I fixed that by prepending the button to the start of the children of <pre>.

Now it's positioned as expected:

image

I also tested this on Firefox and Safari and it works great!

I removed the

.then(() => {
            console.log("Code copied to clipboard:\n" + txt)
        })
        .catch((error) => {
            console.error("Error copying code to clipboard:", error)
        })

Because the then won't be so useful anymore after this is merged, more useful during development, and the catch is actually done automatically! If you have a promise in an event handler that rejects, but there is no catch, Chrome will automatically log it to the console.

Thanks again!

@fonsp fonsp merged commit ad6306d into fonsp:main Apr 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frontend Concerning the HTML editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy-pasteable code blocks
3 participants