Clean up CSS/JS in color module

https://gist.github.com/3006391#L6038

The problem is it doesn't have any other usable selector besides the IDs. So we might have to dig into the JS.

This is FUBAR. I mean, it needs to be addressed in color.module, color.js and the themes implementing it (including the js in the themes) :|

So, what I actually did was add classes, leave the ids the way they were. Trying to remove the ids would force me to go through the js files and it's enough of a patch for this file.

This still applies cleanly. Color widget seems unaffected by this patch (as supposed).

+++ w/core/modules/color/color.js
@@ -21,7 +21,7 @@ Drupal.behaviors.color = {
-    $('<div id="placeholder"></div>').once('color').prependTo(form);
+    $('<div id="placeholder" class="placeholder"></div>').once('color').prependTo(form);

If we're going to replace IDs with classes (which we should btw +1), you might as well remove the IDs entirely from the JS and HTML.

@Mark Carver, agreed, but it's quite some effort to remove the id from js. I suggest it as a follow up patch.

#4: csslint-color-1662958.patch queued for re-testing.

Patch needs to be rerolled against the latest HEAD.

On a side note, we may want to also refactor this patch against our CSS coding standards while we're at it.

Working with this.

@iflista, marking as unassing given that there are no work for more than three weeks.

If you're interested on work on it, please reassign it.

Regards.

Working on this

Css classes renamed as suggested in css code styling.
This patch also fixes Bartik, which overrides the color template.

Some js selectors had to be rewritten for this patch to allow the new code styling.

Needs review

• Ran color.admin.css through CSS Lint and resolved all warnings except "Disallow too many floats" (address later)
• Updated Color module templates with class names instead of ID
• Updated javascript to call class instead of ID
• Does not use new CSS coding standards

Is the intention with CSS Lint to remove all errors, warnings, or both?

ZenDoodles asked me to upload this patch.

those files you requested.

Noticed in the previous patch that the id "placeholder" was removed. It looks to me like the placeholder div is generated via the color.js and that farbtastic depends on the id "placeholder". I ended up adding a class on that element to apply style and layout css instead of calling #placeholder.

run, bot!

+++ b/core/modules/color/color.js
@@ -39,8 +39,8 @@ Drupal.behaviors.color = {
+        $('.preview').once('color').append('<div class="gradient-' + i + '"></div>');
+        var gradient = $('.preview .gradient-' + i);

We should be following jQuery Coding Standards. I know this issue just said CSS, but when that changes, so does the markup/JS:

• Use context and proper event delegations.
• Use correct variable names for jQuery objects (ie: form => $form).

Did some cleanup for JS/jQuery coding standards for variable names & using the context passed to attach. (https://drupal.org/node/172169 as well as https://drupal.org/node/1720586).

I wasn't sure what was meant by "proper event delegations" at this time, so this will probably need another pass.

This work was started during the Drupalcamp ATL code sprint for http://drupalmentoring.org/task/4416

This patch needs to be rebased against the current HEAD; it no longer applies.

I'd rather not mess with JS here. Just make the bare minimum changes please. Anything else should be dealt with in #1751334: Selectors clean-up: color module (and there is already a patch fixing things to review).

Also don't forget the JavaScript tag on JS issues. Otherwise JS maintainers can't have a look at it and will reopen the issue and probably complain, thanks :D

Removed .js changes since they will be handled in another issue. Kept changes needed for using a class instead of the ID.

25: 1662958-25.patch queued for re-testing.

Re-roll.

1. +++ b/core/modules/color/color.js
   @@ -21,7 +21,7 @@
   -      $('<div id="placeholder"></div>').once('color').prependTo(form);
   +      $('<div id="placeholder class="color-picker""></div>').once('color').prependTo(form);
   
   +++ b/core/modules/color/css/color.admin.css
   @@ -27,56 +27,56 @@
   -  [dir="rtl"] #placeholder {
   +  [dir="rtl"] .color-picker {
   

   I think you forgot to remove the #id. Also, let's not change the name, just the specificity here.

2. +++ b/core/modules/color/preview.html
   @@ -1,7 +1,7 @@
   \ No newline at end of file
   

   There should be a single empty line at the end of the file (for git).

I kept the placeholder ID, per comment #18. (Something about Farbtastic.) The missing newline was in the existing code. Unassigning because I might not get to work on this for a while.

30: 1662958-30.patch queued for re-testing.

I noticed #2268955: Deprecate farbtastic library and remembered this issue, so that placeholder ID can probably be removed with a reroll.

Agreed. Let's postpone this until Farbtastic is removed. No sense in trying to clean up something that will end up just being removed anyway.

Just removing the needs reroll tag, since this is going to be postponed for quite some time... farbtastic isnt going to be removed until #1651344: Use color input type in the color.module gets in, and that's postponed until Color input type gets support on all major browsers....

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