HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Expandable navigation menu

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
menunavigationexpandable

Problem

I am trying to step to the next level from student to amateur web developer, and I would like to make sure I am not making any web-development no-no's, that's all. What caused me to want to ask SO this question is that I had to make some tweaks to make my responsive design work.

For instance, I had to make a lot of my positions "absolute" and had to do some hacking in my HTML, making a different nav bar if it's mobile, desktop or tablet (you'll see I haven't done anything with tablet) design. I'm also pretty new to jQuery, although I'm a programmer so I think that code is okay, but I'll post it anyways. Some other stuff as well, would really appreciate ANY feedback, and I really want some good criticism so I can become better.

live demo

HTML

```

$(document).ready(function () {

$('nav').hide();
$('.maDown').hide();

$('.assignmentsDown').click(function() {

$('nav.aDown').slideDown(2000);
if($('.wDown').is(":visible"))
{
$('nav.wDown').slideToggle(300);
}

if($('.pDown').is(":visible"))
{
$('nav.pDown').slideToggle(300);
}

$('nav.aDown').slideDown(2000);

});

$('.MassignmentsDown').click(function() {

$('.maDown').show();

});

$('.writingDown').click(function() {
$('nav.wDown').slideDown(2000);

if($('.aDown').is(":visible"))
{
$('nav.aDown').slideToggle(300);
}

if($('.pDown').is(":visible"))
{
$('nav.pDown').slideToggle(300);
}

});

$('.presentationDown').click(function() {
$('nav.pDown').slideDown(2000);

if($('.wDown').is(":visible"))
{
$('nav.wDown').slideToggle(300);
}

if($('.aDown').is(":visible"))
{
$('nav.aDown').slideToggle(300);
}

});

}); //closes ready







Al Gore Rhythm Labs
Chicago, IL

Solution

From a once over:

  • $(document).ready(function () {



  • Indenting, you are not doing it consistently



  • $('.assignmentsDown').click(function() { and



$('.writingDown').click(function() { and

$('.presentationDown').click(function() { all use copy pasted code, you should consider a generic function that can be used for all 3 situations

  • It is consider better form nowadays to have your JavaScript in a separate .js file

Context

StackExchange Code Review Q#45846, answer score: 4

Revisions (0)

No revisions yet.