Skip to content

Instantly share code, notes, and snippets.

@minitech

minitech/index.html

Last active December 18, 2015 21:09
Show Gist options
  • Save minitech/5845169 to your computer and use it in GitHub Desktop.
Save minitech/5845169 to your computer and use it in GitHub Desktop.
Download ZIP
Raw
index.html
<!DOCTYPE html>
<html>
<head>
<title>Simple tabbed menu</title>
<link rel="stylesheet" type="text/css" href="tabs.css">
</head>
<body>
<h1>Simple tabbed menu</h1>
<div id="navigation">
<ul>
<li><a href="#tab1">Tab 1</a></li>
<li><a href="#tab2">Tab 2</a></li>
<li><a href="#tab3">Tab 3</a></li>
<li><a href="#tab4">Tab 4</a></li>
<li><a href="#tab5" class="selected">Tab 5</a></li>
</ul>
<div class="clear"></div>
</div>
<div id="tabs">
<div id="tab1">pannel doesn't always shows everythings</div>
<div id="tab2">this is not a favorit glass</div>
<div id="tab3">Oh my god we don't have such strong idea</div>
<div id="tab4">my favorit pannel is hear</div>
<div id="tab5" class="content">Do not talk too much</div>
</div>
<script type="text/javascript" src="tabs.js"></script>
</body>
</html>
Raw
tabs.css
body, html, div, ul, li, a {
margin: 0;
padding: 0;
}
body {
font-family: Arial, sans-serif;
font-size: 12px;
}
.clear {
clear: both;
}
a img {
border: none;
}
ul {
border-left: 1px solid #f5ab36;
display: table;
list-style: none;
position: relative;
top: 1px;
z-index: 2;
}
ul li {
float: left;
}
ul li a {
background: #ffd89b;
color: #222;
display: block;
padding: 6px 15px;
text-decoration: none;
border-right: 1px solid #f5ab36;
border-top: 1px solid #f5ab36;
}
ul li a.selected {
background: #fff;
border-bottom: 1px solid #fff;
color: #344385;
}
h1 {
color: #fff;
display: block;
margin: 0 auto;
padding: 20px 0;
width: 600px;
}
#logo {
margin: 0 auto;
padding:10px 0;
text-align: right;
width: 600px;
}
#navigation {
margin: 0 auto;
width: 602px;
}
#tabs > div {
background: #fff;
border: 1px solid #f5ab36;
display: none;
height: 200px;
margin: 0 auto;
text-align: center;
padding: 10px 0;
width: 600px;
z-index: 1;
}
#tabs > .content {
display: block;
}
Raw
tabs.js
'use strict';
var tabs = document.getElementById('tabs');
var navigation = document.getElementById('navigation');
navigation.addEventListener('click', function(e) {
if(e.target.nodeName === 'A') {
document.querySelector('#navigation .selected').classList.remove('selected');
e.target.classList.add('selected');
document.querySelector('#tabs .content').classList.remove('content');
document.querySelector(e.target.getAttribute('href')).classList.add('content');
}
e.preventDefault();
});
@mkmcdonald
Copy link

mkmcdonald commented Jun 27, 2013

Your code here is rife with needless complexity. There is zero need, insofar as I can discern, for querySelector here. You could just toggle the classes.

I would try some solution such as the following:

CSS:

.tabs .hidden_tab
{
    display: none;
}

JavaScript:

function isTab(
    target
)
{
    return hasClass(
        target,
        "tab"
    );
}

function toggleTab(
    target
)
{
    toggleClass(
        target,
        "hidden_tab"
    );
    return false;
}

function delegateBodyClicked(
    target
)
{
    var result = true;
    if (isTab(target)) {
        result = toggleTab(target);
    }
    return result;
}

function bodyClicked(
    evt
)
{
    return delegateBodyClicked(
        getEventTarget(evt)
    );
}

document.body.onclick = bodyClicked;

Note that such an approach does require a few abstractions. I use a very simple feature detection method for getEventTarget and my own library, Utils, for classes. Those aside, the browser support for which is excellent.

@minitech
Copy link
Author

minitech commented Jun 27, 2013

@mkmcdonald ­— I must confess that I find your code unreadable. There is no need, insofar as I discern, for such an excess of whitespace. Your rewriting is rife with needless indirection.

Seriously. There is so much whitespace in your code that I can’t follow its flow. I don’t care about its browser support unless that’s actually a problem. I’m not sure what your modified HTML looks like. I would normally try to figure that out but, to put perhaps too fine a point on it, I can’t read what you just wrote.

@minitech
Copy link
Author

minitech commented Jun 27, 2013

Okay, so I gave it a try. I think what you wrote, without Utils and general old browser support, boils down to this:

document.body.onclick = function(e) {
    if(e.target.classList.contains('tab')) {
        e.target.classList.toggle('hidden');
        return false;
    }
};

… and your advice boils down to “give each tab a class”. Salient advice, and duly noted.

@mkmcdonald
Copy link

mkmcdonald commented Jun 27, 2013

@minitech yes, that is the crux of the strategy. I try to avoid node selection as much as possible.

You are, however, one of the few people to complain about the way in which I write code. If you would like to avoid Twitter rants, please send me an email (the address can be found via my site). I would like to discuss your reciprocal critiques further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment