HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Fading content for each TD

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
contentfadingforeach

Problem

I have an HTML table and I use jQuery to fade some content for each TD:

Demo

My code works great, but I want to know if my solution can be better.


    Inserer un produit
    
    Inserer un nv crédit

    Afficher les produits
    Afficher les crédits courante


jQuery

$( "#first" ).click(function() {
if( $("#Element2" ).show || $("#Element3" ).show || $("#Element4" ).show){
    $("#Element2").hide();
    $("#Element3").hide();
    $("#Element4").hide();
}
$( "#Element1" ).fadeToggle("slow");
});
$( "#second" ).click(function() {
if( $("#Element1" ).show || $("#Element3" ).show || $("#Element4" ).show){
    $("#Element1").hide();
    $("#Element3").hide();
    $("#Element4").hide();
}
$( "#Element2" ).fadeToggle( "slow");
});
$( "#third" ).click(function() {
if( $("#Element1" ).show || $("#Element2" ).show || $("#Element4" ).show){
    $("#Element1").hide();
    $("#Element2").hide();
    $("#Element4").hide();
}
$( "#Element3" ).fadeToggle( "slow");
});
$( "#fourth" ).click(function() {
if( $("#Element1" ).show || $("#Element2" ).show || $("#Element3" ).show){
    $("#Element1").hide();
    $("#Element2").hide();
    $("#Element3").hide();
}
$( "#Element4" ).fadeToggle( "slow");
});

Solution

You can DRY your JS code, assuming you modify the HTML this way:

  • On each of the four selector `s, add a selector class and a data-id="N" attribute, where "N" refers to the #ElementN.



  • On each of the four selectable s, add a selectable class.



So your HTML part becomes:


    Inserer un produit
    
    Inserer un nv crédit

    Afficher les produits
    Afficher les crédits courants

<table id="Element1" class="selectable formInsertTab" ...


BTW, note that I affected
data-value in the order showed by your demo (1, 4, 2, 3), while the code posted in your question is 1, 2, 3, 4.

Then in the JS part you can:

  • Take advantage of the above HTML changes to bind click event only once for all selectors.



  • Hide selectable elements without previously checking if they show.



Warning, anyway your test was falsy:
$('#ElementN').show always returns true, since it checks if the show() method exists, not if the element is showing!

  • Add the improvement proposed by @Roamer-1888: chain fadeToggle() before hiding other elements.



BTW here is an interesting demo from him, which illustrates how you can have fine control on the
fadeToggle()` execution.

The resulting code is so much reduced:

$('.selector').click(function() {
  $element = $('#Element' + $(this).data('id')).fadeToggle('slow');
  $('.selectable').not($element).hide();
}

Code Snippets

<tr>
    <td class="selector" data-id="1"><img src="#" /><br />Inserer un produit</td>
    <td><img src="#" /><br /></td>
    <td class="selector" data-id="4"><img src="#" /><br />Inserer un nv crédit</td>
</tr>
<tr>
    <td class="selector" data-id="2">Afficher les produits</td>
    <td class="selector" data-id="3"><a href="#">Afficher les crédits courants</a></td>
</tr>
<!-- ElementN sample -->
<table id="Element1" class="selectable formInsertTab" ...
$('.selector').click(function() {
  $element = $('#Element' + $(this).data('id')).fadeToggle('slow');
  $('.selectable').not($element).hide();
}

Context

StackExchange Code Review Q#121321, answer score: 2

Revisions (0)

No revisions yet.