1
\$\begingroup\$

I wrote JS to clone image and attached this action to button.
It works but I'm not sure that this better way to do it.

My questions:

  • is it correct to use querySelector to return elements, or is it better to add additional classes or id for buttons and pictures?
  • use cloneNode is optimal or better use other ways in this case?

(function() {
  'use strict';
  document.querySelector('button[action="button"]').addEventListener('click', function() {
    let image = document.querySelector('img[alt="Dog"]');
    let cln = image.cloneNode(true);
    document.getElementById('image-section').appendChild(cln);
  });
})();
<body>
  <div id="overview">
     <section class="section--center">
       <button action="button">Yeah, I want more dogs!</button>
     </section>
     <section class="section--center" id='image-section'>
        <img src="https://www.purina.com/sites/g/files/auxxlc196/files/styles/kraken_generic_max_width_480/public/HOUND_Beagle-%2813inch%29.jpg?itok=lN915WHC" alt="Dog" style="width: 150px">
     </section>
  </div>
</body>

\$\endgroup\$
  • \$\begingroup\$ Just a minor nitpick: <button> elements do not have an action attribute in standard HTML. What you're probably looking for is name. Cf. developer.mozilla.org/en-US/docs/Web/HTML/Element/button \$\endgroup\$ – Máté Safranka Jun 29 '18 at 13:41
  • \$\begingroup\$ While I agree that the action attribute should not be used, I believe he is looking for type, not name. If you do not declare a button type as "button" inside a form, it will automatically try to make it a submit button. If you would like to use a custom attribute, use the data- prefix, i.e. data-action = "button" \$\endgroup\$ – Tom Jenkins Jul 5 '18 at 16:45
1
\$\begingroup\$

Note: you're cloning a DOM element not an actual image..

About .querySelector

The great thing about .querySelector is that you can use any DOMString (CSS string selectors) to select the elements. However in your case you're using the action attribute on the button. So essentially you're already naming your buttons (here there's only one). You can actually get away with not naming them at all. If you have multiple buttons you could simply select them with .querySelectorAll which returns a NodeList that you can either:

  • directly iterate over with the built-in forEach

    document.querySelectorAll('button').forEach(button => {
      // do something with `button`
    });
    
  • or first convert the NodeList into an Array. You can use the ES6 spread syntax to destructure the iterable NodeList object into an actual object of type Array. This way you can then apply methods from the Array class, for example: .forEach, .map, .reduce, .filter, etc... Here's an example:

    [...document.querySelectorAll('button').forEach(button => {
      // do something with `button`
    }
    

I personally never go with .getElementById or .getElementbyClassName and always use .querySelector and .querySelectorAll.


About .nodeNode

If you're going to duplicate a DOM element on the page then I would say nodeList is the way to go. There are alternatives but they are slower.

  • copy the OuterHTML of the image (the actual image tag) and add it directly to the innerHTML of #image-section with .innerHTML +=. This is by far the worst idea as it forces the browser to re render the DOM when the property is changed. You can read more on this other Stack Overflow post.

  • copy the OuterHTML of the image, create a new element we will use as a wrapper for our duplicate then add the image element as the wrapper's child and append the wrapper's first child (the image) to #image-section.

    Here is how it can be implemented:

    const duplicate = () => {
      const image = document.querySelector('img[alt="Dog"]');
      const copy = document.createElement('div');
      copy.innerHTML = image.outerHTML;
      document.querySelector('#image-section').appendChild(copy.firstChild);
    }
    

I have tested both version with a timer to make sure, here is the code. Results are below.

const timeit = fn => {
    const start= new Date();
    let i = 1000;
    
    while(i--) fn();
    return new Date()-start;
}

console.log('clone: ', timeit(clone), 'ms')
console.log('duplicate: ', timeit(duplicate), 'ms')
<body>
  <div id="overview">
     <section class="section--center">
       <button action="button">Yeah, I want more dogs!</button>
     </section>
     <section class="section--center" id='image-section'>
        <img src="https://www.purina.com/sites/g/files/auxxlc196/files/styles/kraken_generic_max_width_480/public/HOUND_Beagle-%2813inch%29.jpg?itok=lN915WHC" alt="Dog" style="width: 150px">
     </section>
  </div>
</body>

<script>
const clone = () => {
  const image = document.querySelector('img[alt="Dog"]');
  const cln = image.cloneNode(true);
  document.querySelector('#image-section').appendChild(cln);
}


const duplicate = () => {
  const image = document.querySelector('img[alt="Dog"]');
  const copy = document.createElement('div');
  copy.innerHTML = image.outerHTML;
  document.querySelector('#image-section').appendChild(copy.firstChild);
}
</script>

Results are here, time to render 1000 dogs (in ms):

-----------------Chrome:----------------
clone:  50 | 30 | 44 | 38 | 50 | 37 | 42
copy:   86 | 50 | 55 | 50 | 57 | 45 | 56

-----------------Firefox:---------------
clone:  42 | 46 | 44 | 66 | 34 | 53 | 50
copy:   59 | 49 | 50 | 97 | 55 | 55 | 58

You should stick with nodeClone(true).

For further reference on this read: Deep cloning vs setting of innerHTML: what's faster?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.