1

Started this thread yesterday, marked as solved! Thx.

Want to add some functionality to the BTNs below the IMGs: Clicking a BTN should a) do everything the solution already does and b) change the BTNs .text("some_text") and addClass ("some_css") to the BTN. Re-clicking the same button should reset itself. Clicking another BTN needs to reset everything as well. The idea below works, but I feel it should be solved way more elegantly.

$(document).ready(function() {

    $('.btn').on("click", function() {

        if ($(this).hasClass("playing")) {

            const btnIndex = $(this).data("index"); // index of the clicked button
            $('.img').each((_,img) => {
                const $img = $(img);
                const imgIndex =$img.data("index");
                const suffix = (btnIndex == imgIndex) ? "a" : "a"
                $img.attr("src", `image-${imgIndex}-${suffix}.gif`);
            });

            // resets ALL BTNs text
            $(".btn").text("Play Animation");
            // resets ALL BTNs css
            $(".btn").removeClass("btn_state_active");
            // resets ALL BTNs incl. THIS one to "not-playing"..
            $(".btn").removeClass("playing");

        } else {

            const btnIndex = $(this).data("index"); // index of the clicked button
            $('.img').each((_,img) => {
                const $img = $(img);
                const imgIndex =$img.data("index");
                const suffix = (btnIndex == imgIndex) ? "b" : "a"
                $img.attr("src", `image-${imgIndex}-${suffix}.gif`);
            });

            // resets ALL BTNs text
            $(".btn").text("Play Animation");
            // resets ALL BTNs css
            $(".btn").removeClass("btn_state_active");
            // resets ALL BTNs incl. THIS one to "not-playing"..
            $(".btn").removeClass("playing");

            // updates THIS BTNs text
            $(this).text("Stop Animation");
            // updates THIS BTNs css
            $(this).addClass("btn_state_active");
            // changes/updates THIS BTNs status
            $(this).addClass("playing");

        }

    });

});
plbr
  • 75
  • 1
  • 9

1 Answers1

1

The most important thing is DRY (Don't repeat yourself!). Which means in this case finding the "common" functionality and always doing that and then finding the specific functionality, and executing that when necessary

You always do this bit

const btnIndex = $(this).data("index"); // index of the clicked button
$('.img').each((_,img) => {
    const $img = $(img);
    const imgIndex =$img.data("index");
    const suffix = (btnIndex == imgIndex) ? "b" : "a"
    $img.attr("src", `image-${imgIndex}-${suffix}.gif`);
});

// resets ALL BTNs text
$(".btn").text("Play Animation");
// resets ALL BTNs css
$(".btn").removeClass("btn_state_active");
// resets ALL BTNs incl. THIS one to "not-playing"..
$(".btn").removeClass("playing");

And then this bit you only do if the current button doesnt have the class playing:

// updates THIS BTNs text
$(this).text("Stop Animation");
// updates THIS BTNs css
$(this).addClass("btn_state_active");
// changes/updates THIS BTNs status
$(this).addClass("playing");

So do exactly that!

$(document).ready(function() {

    $('.btn').on("click", function() {
        const $this = $(this); // DRY!!
        const btnIndex = $this.data("index"); // index of the clicked button
        $('.img').each((_,img) => {
            const $img = $(img);
            const imgIndex =$img.data("index");
            const suffix = (btnIndex == imgIndex) ? "b" : "a"
            $img.attr("src", `image-${imgIndex}-${suffix}.gif`);
        });

        // Do this before resetting all buttons
        const isPlaying = $this.hasClass("playing");

        // remember DRY!
        const $allBtns = $('.btn');
        // resets ALL BTNs text
        $allBtns.text("Play Animation");
        // resets ALL BTNs css
        $allBtns.removeClass("btn_state_active");
        // resets ALL BTNs incl. THIS one to "not-playing"..
        $allBtns.removeClass("playing");

        if (!isPlaying) {           

            // updates THIS BTNs text
            $this.text("Stop Animation");
            // updates THIS BTNs css
            $this.addClass("btn_state_active");
            // changes/updates THIS BTNs status
            $this.addClass("playing");

        }

    });

});

Live example below

$('.btn').on("click", function() {
  const $this = $(this); // DRY!!
  const btnIndex = $this.data("index"); // index of the clicked button
  $('.img').each((_, img) => {
    const $img = $(img);
    const imgIndex = $img.data("index");
    const suffix = (btnIndex == imgIndex) ? "b" : "a"
    $img.attr("src", `image-${imgIndex}-${suffix}.gif`);
  });

  // Do this before resetting all buttons
  const isPlaying = $this.hasClass("playing");

  // remember DRY!
  const $allBtns = $('.btn');
  // resets ALL BTNs text
  $allBtns.text("Play Animation");
  // resets ALL BTNs css
  $allBtns.removeClass("btn_state_active");
  // resets ALL BTNs incl. THIS one to "not-playing"..
  $allBtns.removeClass("playing");

  if (!isPlaying) {

    // updates THIS BTNs text
    $this.text("Stop Animation");
    // updates THIS BTNs css
    $this.addClass("btn_state_active");
    // changes/updates THIS BTNs status
    $this.addClass("playing");

  }

});
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<button class="btn" data-index="100">Play Animation</button>
<button class="btn" data-index="99">Play Animation</button>

<img class="img" data-index="100">
<img class="img" data-index="99">
<img class="img" data-index="98">
<img class="img" data-index="97">
<img class="img" data-index="96">
Jamiec
  • 133,658
  • 13
  • 134
  • 193
  • Thx. I feel like a complete beginner/learner which is great, but…  !isPlaying needs to also reset the image above to -a…? – plbr Sep 30 '21 at 12:13
  • @plbr sorry i dont understand the question. – Jamiec Sep 30 '21 at 12:30
  • Oh wait maybe I do - why did you change to `const suffix = (btnIndex == imgIndex) ? "a" : "a"`? I didnt notice that – Jamiec Sep 30 '21 at 12:30
  • I tried that because clicking the same BTN twice would reset the BTN itself, but not the IMGs… So I tried (redundantly) adding $('.img').each() to !isPlaying, but with "b" : "a"… – plbr Sep 30 '21 at 13:39