ansaurus

Question

Answer 1

A:

It looks like the problem is this:

Your anchor tag (link) is surrounded by a div (the div itself doesn't take up any more space than the link inside it in this case). When the user clicks the link, its running the onclick of the link (and its passing the id of the link, which is not added as an attribute).

Also, you need to move your script tag into your header (by the way, its "head", not "header")

Gabriel McAdams 2009-12-22 22:40:33

Answer 2

+6 A:

<header> isn't an element, but <head> is.
the only legal children of <html> are <head> and <body>. The sample code has a <script> element that's a child of <html>
nav_color_swap sets the classname for navid, rather than navid_array[i], when navid != navid_array[i]. This is likely the source of your problem.
When that's fixed, the click handlers for the <a> will set the class name for all elements to "navCSS1", since the <a> don't have ID attributes.

Since at most two element classes will need to change with each click, I recommend keeping track of the current element rather than looping over all elements:

<style type="text/css">
.selected {
    background-image:url('news_selected.png');
}
.news, .news li {
    padding: 0;
    margin: 0;
    list-style-type: none;
}
...
</style>
<script type="text/javascript">
function createNavSelector(curr_nav, selectedClass, unselectedClass) {
    return function (nav) {
        curr_nav.className = unselectedClass;
        curr_nav = nav;
        curr_nav.className = selectedClass;
    }
};
</script>
</head>
<body>
...
<div class="container_news">
  <ul class="news">
    <li id="nav1" onclick="nav_select(this)">Blah 1</li>
    <li id="nav2" onclick="nav_select(this)">Blah 2</li>
    <li id="nav3" class="selected" onclick="nav_select(this)">Blah 3</li>
    <li id="nav4" onclick="nav_select(this)">Blah 4</li>
    <li id="nav5" onclick="nav_select(this)">Blah 5</li>
  </ul>
</div>
<script type="text/javascript">
var nav_select = createNavSelector(document.getElementById('nav3'), 'selected', '');

Since the #nav* appear to be a list of news items or a list of navigation items, I switched to using elements, since they carry more semantic information than <div>s. At this point, div-vs-ol/ul is largely a matter of personal preference.

I also renamed functions and element classes to reflect their purpose, rather than how they fulfill that purpose. That way, you can change their behavior without requiring a change in name.

Did you use the <a> to support old versions of IE? I wouldn't worry about anything older than IE 6.

As per No Refund, No Return's comment, here's some links to get you started on debugging JS using Firebug. Safari also has a good debugger, if that's your browser of choice.

outis 2009-12-22 22:42:21

gimme some free debugging, please.

No Refunds No Returns 2009-12-22 22:44:42

I changed the if and else clauses to do this instead:document.getElementByID(navid_array[i]).className = etc...It works! Thank you very much.

Napoli 2009-12-22 23:02:21

@NRNR: I know, I shouldn't just *give* Napoli the fish. I mostly said what's wrong with the original code because I was going to offer the more efficient, scalable alternative.

outis 2009-12-22 23:11:13

Answer 3

A:

A few things with your code:

Your 'head' tags should be <head> </head>, not <header> </header>
Your <script> tag should be inside the <head> element, and it's best practice to specify the language with: <script language="JavaScript" type="text/javascript"> ... </script>
Invoke your code in a function that is triggered at the onload event of the body to be sure the elements your javascript is referencing have actually been initialised on the page

BranTheMan 2009-12-22 22:43:30

No, there's no need to include deprecated-for-more-than-10-years-now "language" attribute.

kangax 2009-12-23 04:24:02

You mean there's no need to type those 22 chars every time I open a script tag any more? This calls for some champagne.

BranTheMan 2009-12-23 14:30:02

ansaurus

tags:

views:

answers:

Changing a <div> class with JavaScript

related questions