Creating JS function to operate on dynamically created variables

Question

I have this code which works, but it very elementary in my opinion:

    var color1Default = $('.layer1').css('fill');
    $('#colorbox1').css('background-color', color1Default);

    var color2Default = $('.layer2').css('fill');
    $('#colorbox2').css('background-color', color2Default);

    var color3Default = $('.layer3').css('fill');
    $('#colorbox3').css('background-color', color3Default);

I have a variable that is created based on an echoed PHP variable. This variable is an integer value. This value can be different with any given page load. It comes from a db that says how many elements are needed. The html elements are created with a loop in PHP. The class of layer1, layer2, etc are referencing svg paths.

This is how I tried to simply this code:

    var numberColors = <?php echo $numColors; ?>;
    var colorDefault[];
    for (var i = 1; i <= numberColors; i++) {
        colorDefault[i] = 'colorDefault' + i;
    }

    for (var i=1; i <= numberColors; i++){
        colorDefault[i] = $('.layer' + i).css('fill');
        $('#colorbox' + i).css('background-color', colorDefault[i]);
    }

Am I approaching this anywhere near the way I should? Can you please help me not only to make this work, but to understand how to handle this problem in the future with similar problems?

Additionally, the following code is also repeated 12 times changing the numbers by hard coding them. How can I recreate this function to operate dynamically as well?

    $('#color1').on('input', function(){
            var newValue = $('#color1').val();

            if (newValue == ''){
                $('.layer1').css('fill', color1Default);
                $('#colorbox1').css('background-color', color1Default);
            } else {
                $('.layer1').css('fill', newValue);
                $('#colorbox1').css('background-color', newValue);
            }
        });

Final Solution including the initial loading of each default color into each of the colorboxes:

    var numberColors = <?php echo $numColors; ?>;
    $( document ).ready(function() {

        const defaultColors = Array.from(
            { length: numberColors },
            (_, i) => $('.layer' + (i + 1)).css('fill')
        );

        const colorBoxes = $('.colorbox');
        const colorInputs = $('.colorinput');
        $(colorBoxes).each(function (index) {
            $(this).css('background-color', defaultColors[index]);
        });

        colorInputs.on('input', function() {
            const $this = $(this);
            const index = colorInputs.index($this);
            const newColor = $this.val() || defaultColors[index];
            $('.layer' + (index + 1)).css('fill', newColor);
            colorBoxes.eq(index).css('background-color', newColor);
        });
    });

Your second for loop is overwriting the values of the array created in the first for loop. Also you don't need two for loops. And you're missing an equals sign in your array declaration. — Steven B.
– Steven B., Commented Sep 8, 2018 at 4:51

DavidG · Accepted Answer · 2018-09-08 19:57:04Z

IDs should be used for elements that are unique and distinct, like a search box. Elements that are simply part of a collection, like your colorboxes, should not have separate IDs for each item in the collection. For a situation like that, use classes instead. In order to tie the functionality of a color input fields (currently your #color<num>s) to your color display boxes (currently your #colorbox<num>s), you can check the index of the changed field in the collection of input fields, and change the colorbox with the same index in the collection of colorboxes.

For example, you might use a class name of colorinput for the input fields, and a class name of colorbox for the colorboxes. On page load, make an array for default colors of each layer:

const defaultColors = Array.from(
  { length: numberColors },
  (_, i) => $('.layer' + (i + 1)).css('fill')
);

This means you have a single variable which you can access by index rather than using individual variable names for every item, which is repetitive and hard to manage.

The i + 1 is there because arrays and array indicies (like most things in programming) are zero-indexed, but your .layer<num>s start at 1, not at 0.

For the colorinputs, to repeat yourself less, put the new color (whether it's the value in the input field or the default color) into a variable initially, and then set each .css to that variable:

const colorBoxes = $('.colorbox');
const colorInputs = $('.colorinput');
colorInputs.on('input', function() {
  const $this = $(this);
  const index = colorInputs.index($this);
  const newColor = $this.val() || defaultColors[index];
  $('.layer' + (index + 1)).css('fill', newColor);
  colorBoxes.eq(index).css('background-color', newColor);
});

Again, the index + 1 in there is because collections are zero-indexed. If you can, you might consider using .layer<num> classes where num starts at 0 instead, which will simplify the code even more.

Works great! Thanks. I should be able to handle the initial loading of the colorboxes. I also edited your code to change colorboxes 'fill' property to 'background-color' since they are divs. Your code was very helpful and thanks for taking the time to explain it. I also learned about the eq(). Thanks so much!!!

Collectives™ on Stack Overflow

Creating JS function to operate on dynamically created variables

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related