1

I have a table like this:

<table>
  <tr>
    <td><input type="button" title="Toggle" value="Show" id="btn534534" class="toggle" />    </td>
    <td>some data about ID 534534</td>
  </tr>
  <tr id="row534534" class="hiddenRow">
    <td colspan="2">some hidden data about ID 534534 that must show/hide with toggle</td>
  </tr>
  <tr>
    <td><input type="button" title="Toggle" value="Show" id="btn2423434" class="toggle" /></td>
    <td>some data about ID 2423434</td>
  </tr>
  <tr id="row2423434" class="hiddenRow">
    <td colspan="2">some hidden data about ID 2423434that must show/hide with toggle</td>
  </tr>
  <!-- many more rows here -->
</table>

There are MANY "sets" of rows like the above. One row of the table is visible for each "set" of rows. The second row with class "hiddenRow" is hidden on page load by JQuery like this:

$("tr.hiddenRow").hide();

Now I want each button to toggle visibility of the row immediately after it. Note that the INPUT in the visible row, and the TR of the hidden row share a unique ID. I want to have the button onclick call a JQuery function and pass the ID so that JQuery knows which row to toggle.

Any ideas? I can't find any examples of this online.

Blackcoil
  • 175
  • 3
  • 18

4 Answers4

3

This should do the trick:

$(document).ready(function() {
    $(".toggle").click(function() {
        $(this).closest('tr').next('tr.hiddenRow').toggle(); 
    });
});

http://jsfiddle.net/DU8rd/1 (Thanks Šime Vidas)

Note: This doesn't pass the ID as you indicated, but it does create the behavior you described.

Community
  • 1
  • 1
Nathan Taylor
  • 24,423
  • 19
  • 99
  • 156
  • 1
    `closest('tr')` is more appropriate since you don't need all the parents but just the TR element – Šime Vidas Jan 18 '11 at 17:59
  • @ŠimeVidas Can you make a fiddle (fork mine) demonstrating this? It didn't work when I tried it. – Nathan Taylor Jan 18 '11 at 18:10
  • Careful of tagless class selectors - they're expensive in CPU cycles. Take a look at how JQuery implements them behind the scenes in IE... it's not pretty. – Chris Moschini Jan 18 '11 at 18:11
  • Nathan's code is working perfectly. But it would be nice to know how to do the ID-parameter version for future reference. – Blackcoil Jan 18 '11 at 18:21
1

I prefer @Nathan's solution, but if the DOM relationship would become more complicated, you would do it like so:

$('#tableID input.toggle').click(function() {
    $('#row' + this.id.replace('btn', '')).toggle();
});

Live demo: http://jsfiddle.net/pHKat/1/


Update: The same thing as above, but using event delegation:

$('#tableID').click(function(e) {
    if (!$(e.target).hasClass('toggle')) { return; }
    $('#row' + e.target.id.replace('btn', '')).toggle();
});

This code has one additional line of code, but performance-wise, this code is better, since only one event handler is bound (to the table element). For comparison, my original solution (above) binds one event handler per each button.

Šime Vidas
  • 182,163
  • 62
  • 281
  • 385
  • Two issues here: 1) Use 'tr.toggle' or it will be slow in IE for larger documents, 2) If the table is dynamic - if JS can add and remove row pairs - you'll want to use a .live('click', function... instead of .click(). – Chris Moschini Jan 18 '11 at 18:31
  • @Chris 1. Yes, a more specific selector is definitively better 2. Yes, for dynamic tables. I'll stick to my assumption that the table is static. (I try to avoid live when possible since it executes selector queries every time an click event (in this case) happens) – Šime Vidas Jan 18 '11 at 18:51
  • @Chris & @Šime - [`$('#tableID').delegate('input.toggle','click', fn)`](http://api.jquery.com/delegate) - the best of both worlds ;) – gnarf Jan 18 '11 at 20:44
  • @gnarf Very cool, had not come across .delegate(). The name is unfortunately confusing, but the functionality is a great performance win. http://api.jquery.com/delegate/ – Chris Moschini Jan 18 '11 at 20:47
  • @gnarf `$('#tableID').delegate('input.toggle','click', fn)` is equivalent to `$('#tableID').each(function() { $('input.toggle', this).live('click', fn); });` It uses live() internally. I believe, live() should be avoided if possible. – Šime Vidas Jan 18 '11 at 20:52
  • @Chris I'm not sure that it's a performance win. Can you explain? Or do you have a source? – Šime Vidas Jan 18 '11 at 20:55
  • You are correct about `delegate()` using `.live()`, either will bind the actual event handler to the `context`. To address performance, it is likely better to attach one event handler to the table that checks the parents of `event.target` than to immediately perform a `.each()` over 100 inputs binding 'click'. Using the context parameter ensures you only catch bubbling events under the table, so other clicks don't count - and the per click cost to check `$(event.target).closest(selector)` is a minimal (usually <1ms) per click performance loss to gain performance on the document load. YMMV – gnarf Jan 18 '11 at 21:15
  • @gnarf Yes, I use event delegation regularly. I updated my answer with an example of event delegation. – Šime Vidas Jan 18 '11 at 21:27
  • @Šime It really depends on what you're trying to optimize - event attachment or event firing speed. Agree that delegate attaches via .live(), and that executes a closest(selector) each firing. Reading source, I'm surprised .live()'s implementation doesn't cache matches - it might be a performance improvement for the .live implementation. Though I suppose you would need to cache both hits and misses. – Chris Moschini Jan 20 '11 at 16:48
  • Also - your update doesn't have the event obj as a parameter, so it's going to throw an Exception onclick. Stackoverflow doesn't do code formatting in comments, so I posted a correction and jsfiddle in my answer below. I think the fixed version of your update is the best answer. – Chris Moschini Jan 20 '11 at 17:03
  • @Chris live() does not cache matches, because then it would not be up to date. The purpose of live() is to always be up to date (hence the name). – Šime Vidas Jan 20 '11 at 17:17
  • @Šime This is way off topic now, but it would be interesting if you could optionally enable the live cache, and invalidate the cache if you move an item in the DOM (adding or deleting items would not impact the accuracy of the cache, since added items just get added, deleted items never trigger events). – Chris Moschini Jan 20 '11 at 19:15
  • @Chris You mean if you could set the `live()` handler and then immediately cache the matches, and then when you manipulate the DOM, you trigger a re-cache of the matches? Yea, that would be an interesting option `:)` – Šime Vidas Jan 20 '11 at 19:25
  • I'd rather cache on-demand. So say you do a .live('click' - someone clicks an element, that element is checked against the cache. Cache hit fails, selector run, result stored like cache[elem] = $.closest(selector, context) != null; - future calls can check against the cache and find it - cache[elem] returning true is a hit, cache[elem] === false is a negative, and cache[elem] returning undefined means uncached, run the selector. Only useful for expensive selectors though. – Chris Moschini Jan 21 '11 at 02:00
0

My first suggestion when you say:

"MANY ... hidden on page load" is that you're using Javascript to do CSS's job, and it's either already causing or going to cause you a performance problem. Define:

tr.hiddenRow {
    display: none;
}

in your stylesheet, and get rid of the on page load code.

I'd use the following to implement your button code:

$('input.toggle').live('click', function(ev) {
    var id = this.id.substr(3);
    var row = $('#row' + id);
    if (row.hasClass('hiddenRow'))
        row.removeClass('hiddenRow');
    else
        row.addClass('hiddenRow');
});

If row IDs aren't necessary for other functions, you could reduce the size of your HTML by getting rid of the ID on both the input and the tr, then implement toggle with DOM-walking:

$('input.toggle').live('click', function(ev) {
    var row = $(this).parents('tr').next('tr');
    if (row.hasClass('hiddenRow'))
        row.removeClass('hiddenRow');
    else
        row.addClass('hiddenRow');
});

Edit: Hadn't come across jsfiddle before - that's sort of fun. Here you go: http://jsfiddle.net/z6rAe/

Updating again to include my corrections to Šime's answer - why doesn't StackOverflow do code in comments?

$('#tableID').click(function(ev) {
    if (!$(ev.target).hasClass('toggle')) { return; }
    $(ev.target.id.replace('btn', '#row')).toggle();
});

http://jsfiddle.net/7Dy5s/3/

Chris Moschini
  • 36,764
  • 19
  • 160
  • 190
  • 1
    @Chris You can easily reduce those 6 lines of code to just one line: `$('#row' + this.id.substr(3)).toggleClass('hiddenRow');` 1. There is no need for those local variables since you're using them only once, 2. Instead of the if-else statement just use toggleClass() – Šime Vidas Jan 18 '11 at 18:17
  • @Šime Agreed, but it's much easier to test and teach when you break things down into smaller steps. Otherwise a newer coder can make a simple typo, end up with an error, and be left with this single massive complex operation and no clue where to start to fix it. – Chris Moschini Jan 18 '11 at 18:20
  • When I say MANY I mean a max of 100 hidden rows, so performance isn't a huge deal, but I would prefer the most elegant solution that I could reuse in another situation, say, where the data to be toggled happens to NOT be adjacent to the button, and where the row ID is critical. – Blackcoil Jan 18 '11 at 18:20
  • @Chris In cases when we have lot's of code, that probably is true. But in this case, the code is simple enough. I don't think that beginners would have problems with that line of code I posted above. – Šime Vidas Jan 18 '11 at 18:30
  • @Blackcoil - you have both solutions here. You'll want to keep the .hasClass() since you presumably want to toggle the message on the button as well. Take a look at the jsfiddle I posted as part of the answer. – Chris Moschini Jan 18 '11 at 18:32
  • @Chris That replace() is a nice idea `:)` – Šime Vidas Jan 20 '11 at 17:14
-2

use a custom method like this one:

    <script type="text/javascript">
        function viewToggle(id){
            $('#'+id).slideToggle(500);
        }
    </script>

And then have each button call the function above passing in the value of the id you would like to toggle the view of:

<table>
  <tr>
    <td><input type="button" onclick="viewToggle('row534534');" title="Toggle" value="Show" id="btn534534" class="toggle" />    </td>
    <td>some data about ID 534534</td>
  </tr>
  <tr id="row534534" class="hiddenRow">
    <td colspan="2">some hidden data about ID 534534 that must show/hide with toggle</td>
  </tr>
  <tr>
    <td><input type="button" onclick="viewToggle('row2423434');" title="Toggle" value="Show" id="btn2423434" class="toggle" /></td>
    <td>some data about ID 2423434</td>
  </tr>
  <tr id="row2423434" class="hiddenRow">
    <td colspan="2">some hidden data about ID 2423434that must show/hide with toggle</td>
  </tr>
  <!-- many more rows here -->
</table>
NerdFury
  • 18,876
  • 5
  • 38
  • 41
  • @Nerd I don't see why one would define a separate function, when the function can easily be passed as an argument into jQuery's click method. – Šime Vidas Jan 18 '11 at 18:21
  • I agree with previous commenter. – mekwall Jan 18 '11 at 18:25
  • @gnarf I get down voted for using onclick, but the original poster doesn't get down voted for using tables? If you're going to be an HTML snob, don't be selective. ;) In the code I took this from, I had used the function because I had several nested elements that I wanted to toggle views on, some were tables and others were divs nested in the table structure. This way I didn't need to write 2 selectors. And the whole report (html + js) was being output from a C# program... and most of all - old habits die hard. – NerdFury Jan 18 '11 at 18:50
  • This is tabular data, therefore I use a table. I'm not using a table for UI layout but for displaying tabular data. – Blackcoil Jan 18 '11 at 18:53
  • As a promise to the community, I will read up on unobtrusive javascript before responding to anymore jquery questions. – NerdFury Jan 18 '11 at 18:56
  • @Blackcoil - sorry, I wasn't really being serious. I was surprised that I was considered wrong enough to get 2 down votes, that's all. – NerdFury Jan 18 '11 at 18:57
  • @NerdFury - It's not a matter of HTML snob, its a matter of bad practices for jQuery users.. To quote bot-t in #jQuery: `?inline` - don't use inline event handlers; instead, use jQuery's `bind()` or `delegate()` method in an external JavaScript file. Inline handlers are harder to debug, aren't reusable, are harder to maintain, bloat your HTML, and violate the separation of content design principle. Also - jQuery can't normalize your event, leaving you unsure if you can reliably `stopPropagation()`, or access `e.which`. – gnarf Jan 18 '11 at 19:36
  • those all sound like good ideas. I'm trying to learn from the down vote. Google told me to read about unobtrusive javascript, and I will do so. – NerdFury Jan 18 '11 at 19:53
  • One more reason to avoid inline event handling: They often lead to memory leaks, which jQuery cleans up for you with any of its event binding methods. Look for SetupLeak() here: http://msdn.microsoft.com/en-us/library/bb250448(v=vs.85).aspx – Chris Moschini Jan 24 '11 at 00:22