Playing with variables - something's wrong

Nesting Event Listeners (declaring a listener within another) could be considered bad form, I would think. A concept to consider could be to separate the logic so that component functionality is not intertwined. Even though $spoilerSpan and $button are parts of the $spoiler component, they are distinct entities.

Having this in mind could lead us to create an individual click listener for each:

$button.click(function() {
  $(this).hide();
  $(this).prev().show();
});
$button.prev().click(function() {
   $(this).hide();
   $(this).next().show();
});

You will notice the code is little more spread out now. The benefits, however, are that not only is it easier to read and understand, but the Event Listeners of each object (or entity) are their own. They are not bound to one another this way. Sure, they are related and the Listeners do similar tasks, yet they are different and, more importantly, we have removed any scope confusion with nested use of $(this), and also they don't need each other to survive. The $spoiler component needs them but they don't need each other. This line of thinking is at the core of modular development and Object Oriented Programming.

Another concept that could be touched on here is DRY (Don't Repeat Yourself). You will notice that the click listeners are fairly similar to each other (which is likely one of the reasons you grouped them together initially). So with DRY in mind, when we repeat lines of code, especially multiple lines, it is time to think about if a function can be created to help with typing the same thing more than once. This can also help reduce typos as well.

So for this function we definately want to use $(this).hide() and we also want to show() a sibling. Since the value of $(this) can be different we need to have our function be able to take two arguments - an object to hide and an object to show. This could be written similar to the following:

  function toggleSpoiler($hide, $reveal) {
       $hide.hide();
       $reveal.show();
  }

Then we can refactor the click Listeners to include the function call:

  $button.click(function() {
    toggleSpoiler($(this), $(this).prev());
  });
  $button.prev().click(function() {
    toggleSpoiler($(this), $(this).next());
  });

So now our tidy, easy to read code could be similar to:

$(function() {

  var $spoilerSpan = $('.spoiler span');
  var $spoiler = $('.spoiler');
  var $button = $('<button>Reveal it!</button');

  function toggleSpoiler($hide, $reveal) {
       $hide.hide();
       $reveal.show();
  }

  $spoilerSpan.hide();
  $spoiler.append($button);

  $button.click(function() {
    toggleSpoiler($(this), $(this).prev());
  });
  $button.prev().click(function() {
    toggleSpoiler($(this), $(this).next());
  });

});

Workspace Snapshot

Hi Tomas, there is one subtle error which is causing only the last button to respond to the click event handler.

So you declared:

var $button = $("<button>Reveal it!</button>"); 

which you used to dynamically create a new button here:

$spoiler.append($button); 

However, to apply the click event to the buttons, you need to use a valid and appropriate JQuery element | CSS selector as per the code below:

var $button = $("<button>Reveal it!</button>");  
var $spoilerSpan = $(".spoiler span"); 
var $spoiler = $(".spoiler");
//var $button = $("Reveal it!");

$spoilerSpan.hide();
$spoiler.append($button);

$("button").click(function() {
  console.log("I have been clicked"); 
   $(this).prev().show().click(function() {
     $(this).hide();
     $(this).next().show();
  });
  $(this).hide(); 
});

It works correctly!

Cheers!

Hi guys, thank you so much for your help :-) I think I am gettint it. Will need a bit more time and a bit more practice, but your explanations helped a lot. Thanks again for you help and for your patience, I know I am not the best in written english communication :-) Tomas