Refactor: Javascript block

Question

Need some help refactoring this code:

$("span[rel=color_content]").ColorPicker({
  onChange: function (hsb, hex, rg) {
    $("span[rel=color_content]").css('background-color', '#' + hex);
  }
});
$("span[rel=color_link]").ColorPicker({
  onChange: function (hsb, hex, rg) {
    $("span[rel=color_link]").css('background-color', '#' + hex);
  }
});
$("span[rel=color_selected]").ColorPicker({
  onChange: function (hsb, hex, rg) {
    $("span[rel=color_selected]").css('background-color', '#' + hex);
  }
});
$("span[rel=color_page]").ColorPicker({
  onChange: function (hsb, hex, rg) {
    $("span[rel=color_page]").css('background-color', '#' + hex);
  }
});
$("span[rel=color_player]").ColorPicker({
  onChange: function (hsb, hex, rg) {
    $("span[rel=color_player]").css('background-color', '#' + hex);
  }
});

The only thing changing between each is the contents of the rel attribute.

Justin Niessner · Accepted Answer · 2011-01-10 18:49:27Z

6

// Newlines added for readability
$('span[rel=color_content],
   span[rel=color_link],
   span[rel=color_selected],
   span[rel=color_page],
   span[rel=color_player]').ColorPicker({
       onChange: function(hsb, hex, rg){
           $(this).css('background-color', '#' + hex);
       }
   });

I'm guessing there's only one instance of each span on the page. If not, you can modify this slightly to get something that behaves the same way:

// Newlines added for readability
$('span[rel=color_content],
   span[rel=color_link],
   span[rel=color_selected],
   span[rel=color_page],
   span[rel=color_player]').ColorPicker({
       onChange: function(hsb, hex, rg){
           $('span[rel='+$(this).attr('rel')+']')
               .css('background-color', '#' + hex);
       }
   });

answered Jan 10, 2011 at 18:49

Justin Niessner

246k40 gold badges416 silver badges546 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Šime Vidas Over a year ago

The OP code changes the background-color for the entire list of elements (the entire selector), not just the one that was changed

Justin Niessner Over a year ago

@Šime - Added a second part to the answer to address that (even though I don't think that is the intent).

Šime Vidas Over a year ago

1. Maybe $('span').filter('[rel=foo], [rel=bar], ...') 2. this.rel is safe, no need for attr()

Tiago Deliberali Santos · Accepted Answer · 2011-01-10 18:49:46Z

5

You could do something like:

function setColorPicker(css_selector) {
    $(css_selector).ColorPicker({
      onChange: function (hsb, hex, rg) {
         $(css_selector).css('background-color', '#' + hex);
      }
    });
}

answered Jan 10, 2011 at 18:49

Tiago Deliberali Santos

5043 silver badges9 bronze badges

Comments

jon_darkstar · Accepted Answer · 2011-01-10 18:51:31Z

0

for(i in ['content','link','selected','page','player'])
{
    $("span[rel=color_"+i+"]").ColorPicker({   
    onChange: function (hsb, hex, rg) {$("span[rel=color_"+i+"]").css('background-color', '#' + hex);   } }); 
}

answered Jan 10, 2011 at 18:51

jon_darkstar

16.9k7 gold badges35 silver badges38 bronze badges

Comments

Ken Redler · Accepted Answer · 2011-01-10 19:04:07Z

0

If you end up with a really long list of these to maintain, one way is to put all the changing information in one easily-maintainable variable:

var rels = ['color_content','color_link','color_selected','color_page'];

Then you could have:

// build the selectors according to your patterh
var selectors = $.map( rels, function(e,i){ 
  return 'span[rel=' + rels + ']'; 
}).join(',');

$(selectors).ColorPicker({
  onChange: function(hsb,hex,rg){
    $(this).css('background-color','#'+hex); 
});

answered Jan 10, 2011 at 19:04

Ken Redler

23.9k8 gold badges60 silver badges69 bronze badges

Comments

Šime Vidas · Accepted Answer · 2011-01-10 19:08:08Z

0

var arr = [
    'color_content',
    'color_link', 
    'color_selected', 
    'color_page', 
    'color_player'
];

function foo() {
    var i, selector;

    for (i = 0; i < arr.length; i++) {
        selector = 'span[rel=' + arr[i] + ']';

        $(selector).ColorPicker({
            onChange: function (hsb, hex, rg) {
                $(selector).css('background-color', '#' + hex);
            }
        });
    }
}

edited Jan 10, 2011 at 19:08

answered Jan 10, 2011 at 19:00

Šime Vidas

187k66 gold badges290 silver badges392 bronze badges

Comments

kander · Accepted Answer · 2011-01-10 18:50:31Z

-1

You can separate several CSS selectors using a comma:

$("span[rel=color_player], span[rel=color_page]").ColorPicker();

answered Jan 10, 2011 at 18:50

kander

4,3061 gold badge28 silver badges43 bronze badges

Collectives™ on Stack Overflow

Refactor: Javascript block

6 Answers 6

3 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

3 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related