squidfunk / mkdocs-material

Documentation that simply works
https://squidfunk.github.io/mkdocs-material/
MIT License
20.64k stars 3.51k forks source link

Override clipboard text for code blocks #6086

Closed ti-mo closed 1 year ago

ti-mo commented 1 year ago

Context

No response

Description

Our documentation contains shell snippets including a command's expected output, like:

image

One of our users complained about the clipboard button grabbing everything in the block, which is indeed rather annoying for something like shell-session. Even though the code highlighter correctly recognizes the prompt and the output, that doesn't translate to the behaviour of the clipboard button.

Related links

I ended up here (https://github.com/facelessuser/pymdown-extensions/issues/1802) in my search, but that only really affects text selection. It's still a nice UX improvement, though, and I'll follow the advice given there as well.

Use Cases

I started out trying to influence clipboard.js' behaviour by setting { .shell-session data-clipboard-target="foo !important" data-clipboard-text="go mod init ebpf-test && go mod tidy" } on the block (ending up on the div), but this only works when I remove the data-clipboard-target property from the button using my browser's devtools.

Since it can't easily be determined what the correct behaviour is in shell-session blocks containing multiple lines with commands, I figured it would be nice to be able to manually specify the snippet used by the clipboard button. XYProblem, but it looks like a relatively straightforward change from a distance.

It looks like setupClipboardJS() already takes data-clipboard-text into account, but data-clipboard-target seems to take precedence if both are specified on an element. Not sure why that's the case.

Visuals

No response

Before submitting

squidfunk commented 1 year ago

Thanks for suggesting. Could you please provide a self-contained minimal reproduction that allows us to run what you describe? We would like to understand if we can move properties around to allow for this use case, but for this, we need something we can run and experiment with.

squidfunk commented 1 year ago

Closing, as we didn't receive a minimal reproduction as asked. We can reopen this feature request once the reproduction was provided. The screenshot is not sufficient to understand and inspect the mechanics of what we would've need to change. We'd be happy to consider making it easier to provide your own clipboard text for code blocks, but for that, we need more information. Screenshots are just not runnable code 😉

ti-mo commented 1 year ago

@squidfunk Please reopen, didn't have more time to dedicate to this last week. Here's a repro: 9.4.3-clipboard-text-override.zip

Observation: without superfences, the hierarchy is as follows:

<pre>
  <button></button>
  <code data-clipboard-text="go mod init ebpf-test && go mod tidy"></code>
</pre>

This won't allow the user to override the property on the button, since the <code> is a sibling.

With superfences enabled, the attribute is defined on the surrounding <div> instead:

<div class="highlight" data-clipboard-text="go mod init ebpf-test && go mod tidy">
  <pre>
    <button></button>
    <code></code>
  </pre>
</div>

Note that the data-clipboard-text attr is ignored in both scenarios. Maybe this is something that can be addressed. It depends on a particular configuration, though, making this a little fragile.

Would it be an option to add an extra argument to the block header (fence?) instead? Something like:

    ```shell-session clipboard-text="foo"
    this should copy 'foo'


Thank you.
squidfunk commented 1 year ago

Thanks for providing the reproduction. We'll look into it.

squidfunk commented 1 year ago

However, note that it will be very cumbersome to provide clipboard text via attributes, because everything must fit into a single line without line breaks. I'm not sure this is viable or comfortable to use, so my current guess is: not feasible.

ti-mo commented 1 year ago

Our use case would be providing shorthands/one-liners for longer code blocks, so not being able to use line breaks is fine.

squidfunk commented 1 year ago

The thing is, I'm not sure it's a good user experience. The user clicks on the copy-to-clipboard icon and whats copied is not what's listed. It's counter-intuitive. I understand that there's need for not copying specific lines, but that's actually what code annotations are for. You can annotate each line with additional content without influencing what's copied.

ti-mo commented 1 year ago

Agreed on the premise of principle of least surprise, but that's not what our users are telling us.

If we could somehow exclude the % and output lines in the snippet in the screenshot, that would be ideal, it would make the output reproduce 1:1 in the user's terminal. (basically, make clipboard.js obey user-select: none;) Not sure if that's currently supported by the library, though.

squidfunk commented 1 year ago

Added support in a9da0b632. Note that this will probably always only work properly for single line statements, as you cannot supply multi-line clipboard text in the prologue of a Markdown code block. It might be possible to pull something of with custom blocks, or by using literal line feeds, but I'm not sure and it is currently not a priority for us.

Also note that we cannot call the attribute data-clipboard-text, because the whole code block will be copied on click, since Clipboard.js considers it to be a target. Thus, we named it data-copy. It now works with and without superfences by looking for the closest ancestor defining a data-copy attribute and falling back to the inner text.

squidfunk commented 1 year ago

Released as part of 9.4.4.

ti-mo commented 1 year ago

@squidfunk Thank you! Works like a charm!

squidfunk commented 1 year ago

We've decided to remove this feature from our documentation in ee0a3c6ed. There are severe problems with the implementation in Python Markdown, most prominently that the content needs to be escaped as HTML entities. After thorough discussion in https://github.com/Python-Markdown/markdown/issues/1390, it is not expected that this issue is solved any time soon.

The current implementation is too fragile to advertise as something that we feel comfortable maintaining and providing support for. We will not change the source code for now, meaning it will likely keep working, but we cannot promise anything. The general advise is not to use the data-copy attribute, because it might be removed in the future, e.g. when we're working on the copy-to-clipboard functionality again.

This is the very first time we're removing a feature that we tagged as experimental. Thanks for your understanding.

squidfunk commented 1 year ago

Addendum: if somebody comes up with another extension that allows to comfortably set data-copy, we might consider not removing it and adding it back. The blocks extension could be of help here, but I haven't had the time to look into it.