3
\$\begingroup\$

Whenever a button is clicked, the following need to be performed:

  1. All siblings of the clicked button get toggled
  2. All edit_button get toggled, except for the edit_button within the same span as the clicked button (note: the excluded edit_button can be the same as the clicked button)

The JavaScript is working in the way I want it to, but I am curious if there's another better way to achieve the second requirement.

Note: save_button and cancel_button are hidden when first loaded.

$(function() {
  $('.edit_button, .save_button, .cancel_button').click(function() {
    $(this).toggle();
    $(this).siblings('button').toggle();
    $('div#personal_info').find('button.edit_button').not($(this).parent().find('button.edit_button')).toggle();
  });
});
<div id="personal_info">
  <div id="section_1">
    <span><button class="edit_button">Edit</button><button class="cancel_button">Cancel</button><button class="save_button">Save Changes</button></span>
  </div>
  <div id="section_2">
    <span><button class="edit_button">Edit</button><button class="cancel_button">Cancel</button><button class="save_button">Save Changes</button></span>
  </div>
  <div id="section_3">
    <span><button class="edit_button">Edit</button><button class="cancel_button">Cancel</button><button class="save_button">Save Changes</button></span>
  </div>
</div>

\$\endgroup\$
2
\$\begingroup\$

The selector for all edit_buttons can be expressed as a single selector $('div#personal_info button.edit_button').

I am tempted to rewrite the code to use closure scope to cache the selector values e.g.

// Iterate using each to define cached values for selectors within a 
// closure scope 
$('.edit_button, .save_button, .cancel_button').each(function() {
  var button = $(this);
  // select all elements that need be toggled on click
  var toggled = $([
    button,
    button.siblings('button'),
    $('div#personal_info button.edit_button').
       not(button.parent().find('button.edit_button'))
  ]);
  // toggle each element separetely on click as the first element in a selector
  // defines the target visiblity of a no parameter .toggle()
  button.click(function() {
    toggled.each(
      function() {
        $(this).toggle();
      });
    });
});

The benefit of it is that less traversing is needed. Unfortunately the toggle method didn't work quite as I imagined it would when there are both hidden and shown elements in the selector when using jQuery 1.6.2.

The above code is available through jsFiddle

From a maintenance/cleanliness point of view I'm a bit wary how this piece of code merges the concerns of

  1. Hiding all edit actions
  2. Showing all edit actions
  3. Displaying save and cancel actions for a specific edit action
  4. Hiding save and cancel actions for a specific edit action

The result of all these goals leads to astonishment that is higher than the least possible. I don't really know what a good name for that function would be?

Also how and where are the event handlers for onclick events of .save_button and .cancel_button defined?

\$\endgroup\$
  • \$\begingroup\$ Aleksi - thx for spending your time to show and explain this alternative solution. I like the way you have outlined its pros and cons. I think I will stick with my solution plus the minor change mentioned in your first sentence. But your solution has definitely opened up my eyes to a different approach. Cheers! \$\endgroup\$ – tamakisquare Jul 26 '11 at 17:02
  • \$\begingroup\$ Your solution helped me to learn the use of .each and to get to know jsFiddle (I have been looking for a tool like this). \$\endgroup\$ – tamakisquare Jul 26 '11 at 17:05
  • \$\begingroup\$ The sweetest words I've heard in all of stackexchange network. Thank you! \$\endgroup\$ – Aleksi Yrttiaho Jul 26 '11 at 19:52

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.