Some thoughts in reviewing both your javascript code and your HTML on Plunker.
Code Style
Don't use spaces between HTML tag properties and their values. It makes your markup hard to read.
Example:
<div id = "footab" class = "module">
Should be:
<div id="footab" class="module">
Think of the spaces as the ONLY separator between each element property, since in HTML you don't have the luxury of commas. The only usage for spaces in element definition is for this purpose (excepting spaces inside property string values, of course).
Lots of unnecessary vertical white space both in your HTML and javascript code.
Code Content
I question introducing your own custom custom HTML tag properties. This is what data attributes are meant to do. You could change this:
<div id = "foo-btn" tabid = "footab" class = "btn btn-underline btn-selected">FOO</div>
To:
<div id="foo-btn" data-tab-target-id="footab" class="btn btn-underline btn-selected">FOO</div>
So when trying to determine target ID in javascript. You go from this:
$('#'+$(child)[0].attributes['tabid'].nodeValue).hide();
To this:
var target = this.data('tabTargetId');
$('#'+target).hide();
Much cleaner. Also note I used this here as...
You are introducing a scope confusion problem by using child instead of just using this.
This:
$(tg).children().each(function(i, child){
Should just be:
$(tg).children().each(function(){
Since you are not really using the index value anyway, there is no reason to specify parameters for the callback function. You would then use this or $(this) (if you need jQuery wrapper on current element) from within the callback.
Attach listeners at a higher level (delegate event handling). There is no reason to attach a click handler to every tab in a loop like this. Instead consider using on() with a selector (ideally you could add a class to tab group elements to make this selector simple). So this:
$(".tab-group").each(function(){
$(this).children().each(function(){
Can simply become:
$(".tab-group").on('click', '.tab-option', function() {
You have removed two levels of nested loops in doing this.
Thinks about whether it truly makes sense to use <h1> tags for title for the tabbed content. Semantically, I would think you should be using <h2> here perhaps. This is more meaningful for things such as Google search indexing.
Putting it together, you may end up with code like this.
HTML:
<body>
<h1>Hello Plunker!</h1>
<div class="tab-group">
<div id="foo-btn" data-tab-target-id="footab" class="tab-option btn btn-underline btn-selected">FOO</div>
<div id="bar-btn" data-tab-target-id="bartab" class="tab-option btn btn-underline">BAR</div>
</div>
<div id="footab" class="module">
<h2>FOO</h2>
</div>
<div id = "bartab" class = "module">
<h2>BAR</h2>
</div>
<div class="tab-group">
<div data-tab-target-id="a" class="tab-option btn btn-underline btn-selected"> A</div>
<div data-tab-target-id="b" class="tab-option btn btn-underline"> B</div>
<div data-tab-target-id="c" class="tab-option btn btn-underline"> C</div>
</div>
<div id="a" class="module">
<h2>ALPHA</h2>
</div>
<div id="b" class="module">
<h2>BETA</h2>
</div>
<div id="c" class="module">
<h2>CHARLIE</h2>
</div>
</body>
Javascript:
$(document).ready(function(){
$(".tab-group").on('click', '.tab-option', function() {
var $selectedTab = $(this);
var selectedTargetId = $(selectedTab).data('tab-target-id');
var $siblingTabs = $selectedTab.siblings('.tab-option');
$siblingTabs
.removeClass('btn-selected')
.each(function() {
var target = $(this).data('tab-target-id');
$('#'+target).hide();
});
$selectedTab.addClass('btn-selected');
$('#'+selectedTargetId).show();
});
});
A few thoughts for further exploration:
- Something like this might be something you consider writing as a jQuery plug-in. In fact there are probably a large number of such plug-ins available around tabbed interfaces that you can either use or reference for inspiration.
- You could optimize performance here. This code (both your version and mine) would still do some unnecessary DOM traversal that could be designed away if you REALLY needed to squeeze performance out of this functionality. You could certainly better store references between tab group and target content elements such that you would not need to re-calculate these relationships every time this event handler fires. You might take a look at my answer on this code review - https://codereview.stackexchange.com/a/135858/23727 - which presented a more complex use case similar to yours in behavior (controls that show/hide elements). For this use case, I had suggested using a javascript "class" structure to:
- Support efficient storage of jQuery collection references
- Build and store a data structure for efficient mapping of control elements to the elements they control
- Execute various show/hide (filtering) operations.
