Plugins: Toolbar & Copy to Clipboard

[mAAdhaTTah]

I'm using Prism as the syntax highlighter for my WordPress plugin, WP-Gistpen. I've built a toolbar plugin that I'd like to contribute to Prism. I've got a first pass at extracting the plugin from its CommonJS origins,, but I need some feedback on how to change it to fit the Prism way of doing things, e.g. the HTML API. I also see some possible synergy with the Parse Settings plugin.

The Toolbar exposes two methods: a hook method, which is registered with Prism's complete hook, and a registerButton method, which takes a callback that creates the buttons. The callback will be passed the env and should return an a or span element, which other plugins can use to register themselves with the Toolbar to get their text or link to display.

I've also created a Clipboard plugin is an example of this callback, which creates an anchor tag that, when clicked, copies the code to the clipboard. I plan to extract that into a Prism plugin as well, but I'm not sure how Prism wants to handle dependencies like Clipboard.js, or how the download page will work with internal dependencies like this. In the interest of user friendliness, what's the best approach to take here?

Prior art:

• Copy code snippets #280
• Prism toolbar #286
• http://dev.misterphilip.com/prism/plugins/toolbar/
• https://github.com/PrismJS/prism/pull/142/files

Open Questions:

• Should the registerButton method be polymorphic?
• How can the CSS styles be improved?
• How should we handle 3rd party dependencies, like Clipboard.js?
• What should the HTML API be? (See: Add file-name info to code block #910)
• How do we deal with this plugin & filename plugin conflicts? (See: Add file-name info to code block #910, Extend / replace "show language" to be more generic  #914)

[mAAdhaTTah]

@LeaVerou @Golmote @zeitgeist87 I'm on vacation this coming week, so I should have some time to tweak this and extract the other plugin to your preferences, if you have guys have a chance to take a look and let me know what you think.

Thanks!

[zeitgeist87]

Hi @mAAdhaTTah,

I've tested your plugin and it works fine. I have a few comments on the plugin itself, but I can't really help you with your question about third-party dependencies.

• Why do you need to expose the hook method? Is there a need to execute the hook manually?
• You could additionally to registerButton provide a higher level method to create a button. Something like: createButton(text, onClick)
• I think the helloworld button should do something. Maybe something like alert('Hello World!');
• You could also create a useful demo button, that selects all of the code in the box: http://stackoverflow.com/questions/11128130/select-text-in-javascript
• The CSS styling looks a bit boring, but that is just my opinion

Thanks for contributing to Prism! Sorry for the late response.

[mAAdhaTTah]

Why do you need to expose the hook method? Is there a need to execute the hook manually?

It was mostly to expose the function in case a plugin/developer wanted to unhook and possibly run manually. Not sure if it's necessary, but figured the extra flexibility would be helpful down the line.

You could additionally to registerButton provide a higher level method to create a button. Something like: createButton(text, onClick)

This is actually a good idea. What about making the registerButton method polymorphic:

function registerButton(opts) {
  var callback;

  if (typeof opts === 'function') {
    callback = opts;
  } else {
    callback = function(env) {
      // create element from opts.text & optional opts.onClick
    };
  }
  callbacks.push(callback);
}

I would probably set this up to ensure the env is passed into the onClick method as well.

I think the helloworld button should do something. Maybe something like alert('Hello World!');

Can do. And if we like the polymorphic idea, we can use the helloworld button to demonstrate how to do it that way.

You could also create a useful demo button, that selects all of the code in the box: http://stackoverflow.com/questions/11128130/select-text-in-javascript

Interesting. Might be more useful as an example than the helloworld button. The Clipboard.js implementation uses "select-all" as a fallback when the actual copying fails (primarily in Safari, maybe older browsers).

The CSS styling looks a bit boring, but that is just my opinion

I mostly copied it from the original implementation with some tweaks (I preferred having each button appear visually distinct), but I am not really a designer, so if anyone has any improvements, I'm all ears.

Thanks for contributing to Prism! Sorry for the late response.

Not a problem! Figured it was worth following up, since you guys are pretty active. I've added checkboxes for the open questions remaining on this PR. If the polymorphic idea is acceptable, I should be able to make those changes tonight.

[zeitgeist87]

This is actually a good idea. What about making the registerButton method polymorphic:

I like that idea. It also allows later additions to the interface. Maybe someone would like to implement buttons with icons or something else...

[Golmote]

Regarding third party scripts, I think the plugin page should simply mention explicitly that script is required. And that's it. ^^ I think Prism should not distribute directly the third party script.

[mAAdhaTTah]

@Golmote Any issues/considerations for Node/Browserify users? I don't know if that matters on the Node side, but Browserify users, I believe, will have to explicitly assign the Clipboard var as a global. I'll have to take a closer look myself, but would a require check and call be acceptable? And would we be ok including it as a dependency in package.json?

[Golmote]

I really don't feel like including it as a dependency of Prism globally.
Don't know much about Browserify so I really can't tell.

[LeaVerou]

Yeah, I would be opposed to adding global dependencies to support a plugin.

However, I'm not sure about just mentioning that Clipboard.js is needed. Part of why Prism is so easy to use is that you download a copy and it just works. I wonder if we can use gulp to pull in the latest version from clipboard.js’ repo and bundle it with the plugin.
Alternatively, the plugin could load the CDN version of clipboard.js dynamically.

[mAAdhaTTah]

There is an optionalDependencies key, but it doesn't appear to function differently from regular dependencies. I'll investigate some options and report back.

[mAAdhaTTah]

So optionalDependencies would mean that the dependency gets installed when installed from npm, but it's not required, so if it's not found or something, the install would still work. I don't know if that's acceptable, but one option is to add Clipboard.js as an optionalDependency. Presumably, if you're installing from npm, you have a build process (likely Browserify/Webpack), which should work with the require check I added (though I still need to fully test that).

Alternatively:

I wonder if we can use gulp to pull in the latest version from clipboard.js’ repo and bundle it with the plugin.

This should be possible; maybe we could do something similar to the way the autoloader's language dependencies map is built. It would mean that the Clipboard.js library would be inlined into the Copy to Clipboard plugin.

Alternatively, the plugin could load the CDN version of clipboard.js dynamically.

The only issue with this is we can't be sure the CDN asset will be fully loaded before the highlighting starts, unless DOMContentLoaded will only fire after all assets (including async ones) are loaded.

The only other idea I had was to modify the downloads page and ensure Clipboard.js is loaded first there when selecting the Copy to Clipboard plugin, but I'm not sure how to accomplish that at first glance of the downloads.js file.

I've pushed a first pass at Copy to Clipboard plugin. The demo page has Clipboard.js loaded from the CDN. Definitely interested in your feedback.

[zeitgeist87]

I would also pass the this pointer like this:

opts.onClick.call(this, env, opts);

That way the callback function gets access to the element that was clicked.

[mAAdhaTTah]

I don't know how important it is, but I didn't want to change the this value of the passed in callback in case the developer is relying on it.

[zeitgeist87]

Well without it, the this pointer will point to the opts object and the developer has no access to the created DOM element. I think it is standard practice to set the this pointer to something sensible, like the source of the click event. In what specific use case would the developer want this to be the opts object?

[mAAdhaTTah]

I guess it was more about not messing with developer expectations, but I think you're right. Will update.

[zeitgeist87]

The design on the https://clipboardjs.com/ homepage looks nice. They have nice copy buttons for their source code samples. Nice fade-in and fade-out animations, nice icon, colored box-shadows and small transparent speech bubbles with status messages. We could use that design as an inspiration.

[mAAdhaTTah]

Added a hover animation and a drop shadown. Does that make it pop a little bit more?

Also, I added Clipboard.js to optionalDependencies, but if that's not something you want, I can remove that commit. Even with that added, the download page doesn't bundle with Clipboard.js. Should it?

[mAAdhaTTah]

Bump. Thoughts on handling the dependency?

[LeaVerou]

What about checking if Clipboard.js is loaded (e.g. via some global it defines) and if not, loading it dynamically from the CDN? That way we don't have the issues of bundling a possibly outdated version or the hassle of requiring users to load it themselves.

[Golmote]

We should probably use a <button type="button"> element here, instead of an <a>.

Or we could allow for both depending on whether there is an onClick property (button) or an href property (link).

[mAAdhaTTah]

Will do. I used the a b/c there would be fewer style overrides, as browsers tend to add a number of default styles to buttons, but it would be more semantically correct.

[Golmote]

I think there should be a way to reorder the labels/actions in the toolbar, especially since some of them may be added automatically by plugins, and so depend on the order in which the plugins executed.

There could be a way to add buttons with the HTML API by providing a data-label-onclick attribute that takes the name of a global function?

There should also be a way to add multiple labels, maybe with numbered attributes, like data-label-1, data-label-2, etc.

<script>
function version_info() { alert('Some info about the version'); };
</script>
<pre data-label-1="v2.8" data-label-1-onclick="version_info" data-label-2="Download lib" data-label-2-href="http://example.com/lib.js">

I'm still very bad at naming things, so there are really just suggestions...

[mAAdhaTTah]

That sounds interesting, but how do we coordinate between the HTML API & the JS API? Copy to Clipboard isn't using the HTML API; should it just go second, all of the HTML-based labels/buttons have been added?

Regarding the data-label-onclick, if you're going to have to write JavaScript anyway, I think I'd rather push the user to use the JS API than reference a global function. Providing a url via HTML attributes would be useful though.

[Golmote]

You're right the data-label-onclick is quite useless.

Regarding the coordination, I think there should be a way to position the buttons that were added through plugins. Maybe this could be achieved with other data attributes

[mAAdhaTTah]

I fixed the a -> button and added support for data-url to add a link instead of just a plain label. I was looking into managing multiple labels, and I'm beginning to feel that API might get ugly & unwieldy. I think if you need multiple labels/buttons, you should be doing them in JS.

The HTML API provides a simple API for adding a single label. If you need multiple labels, or onClick handlers, or anything else more complex, I think we should push users to use the JS API.

[Golmote]

I'm fine with it. If there is a real need for multiple labels through HTML API, it might be added later anyway.

What about the order, when using plugins? Can you think of a way to handle this using HTML?

[mAAdhaTTah]

We could ask plugins to register by name, then have users declare on a the html or body the order they should appear, e.g.:

<html data-toolbar-order="['label', 'clipboard', 'language']">

We can fall back to the order they were registered if the data attribute doesn't exist.

[Golmote]

It seems good. Though I don't think there's a need for the brackets and single quotes. A comma separated list of aliases should be enough.

Would it be possible to control the position of the label added through the data-label attribute this way? Maybe by giving it an arbitrary alias?

[mAAdhaTTah]

Would it be possible to control the position of the label added through the data-label attribute this way? Maybe by giving it an arbitrary alias?

In the above example, label was supposed to refer to the position of the data-label attribute. Maybe toolbar is more explicit?

[Golmote]

Oh no, sorry, "label" is fine! I missed it somehow.

The plugins should probably register their exact name for consistency, i.e. "show-language".

[LeaVerou]

Idea: What about enabling data-label to link to a <template> element containing arbitrary HTML? Then people can have custom buttons and everything in that, with their own event handlers.
Of course it shouldn't be the only way to specify a label, because having to include a separate <template> element if all you want is a textual label is kind of annoying.

[mAAdhaTTah]

Just updated to control the order via data-attribute.

@LeaVerou I'll take a look at that later today. Would be useful if someone wanted to include an icon or something of that nature. How would someone declare their event handlers in a <template>? Unless you just mean inline JS, e.g:

<template id="my-custom-button">
    <button onclick="alert('Hi')">Hello!</button>
</template>

[mAAdhaTTah]

If we wanted to go crazy w/ that idea, we could even implement a mini-DSL and allow people to slot in the code snippet's language and such into the label:

<template id="my-custom-button">
    <button onclick="alert('Hi')">Hello {{language}}!</button>
</template>

[LeaVerou]

How would someone declare their event handlers in a <template>?

Either inline or through event delegation.

[mAAdhaTTah]

@LeaVerou PR updated to include that suggestion.

I think that addresses all of the questions I had when I opened the PR originally. I know this is a big one, and I just want to say thank you for everyone's patience and feedback in working through this.

[mAAdhaTTah]

Will have some time this weekend to make edits as needed. Bumping this PR; maybe get a merge?

[Golmote]

Sorry for the late answer. I'm merging now.

[mAAdhaTTah]

No worries! Happy to contribute.