Make clicks on theme settings preview select palette fields to edit

tagging

Good idea, would make a nice last-minute feature if it's accepted.

+      // Make clicks on HTML preview elements with classnames
+      // "hotspot-palette-[key]" select the palette input text field.

Can we get a more understandable comment? I'd have trouble figuring out what this means. Maybe:

"Make clicks on elements in the preview box select the input text field that controls that element." or something? I don't see a need to mention the classname here, but that might just be me.

I don't think this is backportable, unfortunately. Moving to 8.x.

Not if I have anything to do with it!

Re-rolled against latest HEAD

#13: 828578-13-hotspots_for_color_module_preview.patch queued for re-testing.

+++ b/core/modules/color/color.js
@@ -235,6 +235,15 @@ Drupal.behaviors.color = {
+      $('#preview .hotspot-palette-' + this.key)
+      .click( function(event) {

I'd rather this be on one line if possible. Also remove the extra space between "( function".

+++ b/core/modules/color/color.js
@@ -235,6 +235,15 @@ Drupal.behaviors.color = {
+        event.stopPropagation();

Any reason we can't just "return false;" here?

Also, if we're going to do this. We should probably also get some CSS styling to outline the element when you hover over it and also change the cursor to a pointer to imply that it is clickable. Bumping this to normal since I actually think this would be a nice feature to have.

It should be either

$('#preview .hotspot-palette-' + this.key)
  .click(function (event) {

or

$('#preview .hotspot-palette-' + this.key).click(function (event) {

with the space after function, and if its split, indented 2 spaces.

We never return false; anymore, always explicitly use event.preventDefault(); and event.stopPropagation(); explicitly, because they are not always both needed.

Fair enough.

Rerolled with the fix suggested by @tim.plunkett in #16

+++ b/core/modules/color/color.js
@@ -235,6 +235,14 @@ Drupal.behaviors.color = {
+      $('#preview .hotspot-palette-' + this.key).click(function (event) {

Actually, per jQuery Coding Standards: Event Delegation this should probably be:

$('#preview').on('click', '.hotspot-palette-' + this.key, function (event) {

I attach patch to fix #19
In the same time I think that support for bartik preview.html should be added to this patch.

The patch in #20 worked for me.

I am also attaching a patch that makes the Bartik theme's colored theme preview clickable too. But I would leave it to those more involved in this issue to take a call on if we should include support for Bartik and other theme's preview.html. I think we definitely should. Otherwise this nifty little UX addition would not be of much help.

If this goes in, the documentation for using colors module should also be updated.

This issue introduces a new feature, so per https://www.drupal.org/core/beta-changes, we should postpone it to 8.1.x or later.

Color has been removed from core, #3270899: Remove Color module from core.