3
\$\begingroup\$

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

Image here

Demo

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

<table>
<tr>
    <td id="first"><img src="#" /><br />Inserer un produit</td>
    <td><img src="#" /><br /></td>
    <td id="second"><img src="#" /><br />Inserer un nv crédit</td>
</tr>
<tr>
    <td id="third">Afficher les produits</td>
    <td id="fourth"><a href="#">Afficher les crédits courante</a></td>
</tr>

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");
});
\$\endgroup\$
2
\$\begingroup\$

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

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

So your HTML part becomes:

<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" ...

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();
}
\$\endgroup\$
  • \$\begingroup\$ You can sensibly save a line by chaining .fadeToggle()to the end of $element = $('#Element' + $(this).data('id')). \$\endgroup\$ – Roamer-1888 Feb 28 '16 at 11:28
  • \$\begingroup\$ @Roamer-1888 You're "technically" right. But I guess it'd degrade the effect desired by the OP, beginning to show the newly displayed element while the previous one has not been hidden yet. In the other hand, maybe it'd be totally transparent: only testing can make certain. \$\endgroup\$ – cFreed Feb 28 '16 at 12:06
  • \$\begingroup\$ I've done similar many times and it makes no difference to the human eye. .fadeToggle()(or any other jQuery animation) initiates the animation but 100% of it is effected in a series of asynchronous steps, each in its own event thread after the initiating event thread has completed. \$\endgroup\$ – Roamer-1888 Feb 28 '16 at 15:14
  • \$\begingroup\$ @Roamer-1888 Ok, I trust you and edit my answer. \$\endgroup\$ – cFreed Feb 28 '16 at 15:50
  • \$\begingroup\$ Trust is a wonderful thing ;-) Here's something more substantial. \$\endgroup\$ – Roamer-1888 Feb 28 '16 at 16:48

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.