Script Revision for event.target

Question

I'm kind of a newbie to js, could someone have a look at this code and tell me if it is correct and if it could be improved and how? it is working as intended, but honestly I don`t know if it is well written. Thanks in advance (:

           <script>
               $(document).ready(function() {

                   $("button").click(function(event) {

                       var a = (event.target).classList.item(0);
                       var b =  ".carousel" + "." + a ;

                       $(b).myfunction;

                   });
               });
           </script>

The reason for that script is that I have a Carousel (Bootsrap) inside a Slider ("JQuery lightSlider") inside a custom popup modal and I don`t want the imgs carousel to load at page start only when the carousel would be visible.

What the script does is to "store" the class name of the button element ("custom-n") clicked, then write .carousel.custom-x to be stored in the variable b, and then use the var b as a selector to call the function for that custom-n carousel that replaces data-src with src.

I hope this isn't confusing haha

This is a simplification of the html, the modal opens whenever a <button> is clicked. The are kind of a "thumbnail" for the main slider (light-slider), so: <button class="custom-1"> opens the modal and set the "light-slider" to slide n1 <button class="custom-2"> opens the modal and set the "light-slider" to slide n2 and so on.

The carousels inside the lightslider's slides they all have img with data-src and the Idea is to replace the data-src with src of the carousel that should be "related" to the custom


   <div class="light-slider-buttons">
       <button class="custom-1 myclass"></button>
       <button class="custom-2 myclass"></button>
       <button class="custom-3 myclass"></button>
   </div>
   

   <div class="modal-wrp">
      <div class="modal-inner">
        <ul id="light-slider">
          <li>
            <div class="slide-wrapper">
                      
              <div class="carousel custom-1">                                                    
                <div class="carousel-inner">
                  <div class="carousel-item active">
                  <img data-src="...">
                  </div>
                  <div class="carousel-item">
                  <img data-src="...">
                  </div>
                    <div class="carousel-item">
                  <img data-src="...">
                  </div>
                </div>                           
              </div>
                       
            </div>
          </li>
               
        </ul>

      </div>


   </div>

score 0 · Accepted Answer · answered Dec 24 '20 at 00:56

The code looks good.

I have two suggestions.

Firstly, you seem to be using class attribute to store data (custom-1, etc)

Why not set it to data-item-class attribute instead (any name starting with data- will do)?

   <div class="light-slider-buttons">
       <button class="myclass" data-item-class="custom-1"></button>
       <button class="myclass" data-item-class="custom-2"></button>
       <button class="myclass" data-item-class="custom-3"></button>
   </div>

The value can be read as event.target.dataset.itemClass (item-class becomes itemClass, see docs for more info).

The code for variable a will be replaced with:

var a = event.target.dataset.itemClass;

Secondly, setting click event handler for 'button' selector is too wide.

It will cause errors when new button element with another purpose gets added to your page in the future.

It is much safer to think of <div class="light-slider-buttons"> as a component wrapping your button elements and apply the click handler only to those.

$(".light-slider-buttons button").click(function(event) {
...
}

It does not change much but is important because if you add other buttons outside <div class="light-slider-buttons"> they will not get click event handler.

jQuery uses CSS selectors syntax, you can find more in docs.

Here's the recommended code for script tag

           <script>
               $(document).ready(function() {

                   $(".light-slider-buttons button").click(function(event) {

                       var a = event.target.dataset.itemClass;
                       var b =  ".carousel" + "." + a ;

                       $(b).myfunction;

                   });
               });
           </script>

P.S. jQuery programming style is quite imperative (specifying how page should update on every event).

There are other ways to build UI, for example in React programming is declarative (specifying how UI should look like based on given input/state)

Thanks a lot for your time @Vitalli, really appreciate it **Firstly** Awesome recomendation, works and looks great And I didn't use it before basically because I didn't know about that, I barely knew about data-src for imgs haha! **Secondly** yes I also know it was too global, I just post it like that in here to keep things "simple" but also thanks for the explanation, — Julio Pimentel, Dec 24 '20 at 02:14

Script Revision for event.target

1 Answers1