ClaytonAlanKinder
ClaytonAlanKinder

Reputation: 335

How can I make this jQuery code more concise and efficient?

I've been working on a portfolio for some time now. It'll take a long time before I have the skills necessary to complete it, but I go back and work on it a little after I figure some tricks out.

Right now, I just have the mobile navigation done. It works how I want it to, but the jQuery code involved is a little lengthy.

Here is the relevant HTML:

<header class="mobile-header">
    <span>Menu</span>
    <img src="img/nav-icon.png" id="pull">
</header>

<ul class="mobile-navigation">
    <li>
        <span class="ornament-left ornament1">N</span>
        <a href="#" class="navLink1">Home</a>
        <span class="ornament-right ornament1">N</span>
    </li>
    <li>
        <span class="ornament-left ornament2">N</span>
        <a href="#" class="navLink2">About</a>
        <span class="ornament-right ornament2">N</span>
    </li>
    <li>
        <span class="ornament-left ornament3">N</span>
        <a href="#" class="navLink3">Work</a>
        <span class="ornament-right ornament3">N</span>
    </li>
    <li>
        <span class="ornament-left ornament4">N</span>
        <a href="#" class="navLink4">Contact</a>
        <span class="ornament-right ornament4">N</span>
    </li>
</ul>

NOTE: The reason that there are random N's everywhere is that I downloaded an ornamental font and N is the ornament that I wanted.

Relevant jQuery:

$(function(){
    var navLink1 = $('.navLink1')
        navLink2 = $('.navLink2')
        navLink3 = $('.navLink3')
        navLink4 = $('.navLink4')
        ornament1 = $('.ornament1')
        ornament2 = $('.ornament2')
        ornament3 = $('.ornament3')
        ornament4 = $('.ornament4')

    $(navLink1).hover(
        function() {
            ornament1.fadeTo('med', 1.0);
        }, function() {
            ornament1.fadeTo('med', 0);
        }
    );

    $(navLink2).hover(
        function() {
            ornament2.fadeTo('med', 1.0);
        }, function() {
            ornament2.fadeTo('med', 0);
        }
    );

    $(navLink3).hover(
        function() {
            ornament3.fadeTo('med', 1.0);
        }, function() {
            ornament3.fadeTo('med', 0);
        }
    );

    $(navLink4).hover(
        function() {
            ornament4.fadeTo('med', 1.0);
        }, function() {
            ornament4.fadeTo('med', 0);
        }
    );
})

As you can tell, that jQuery takes up a lot of space. I figure there's some way to use a counter to make it more efficient but I can't figure it out.

For a live demo of my site, take a look at this jsFiddle.

http://jsfiddle.net/n3wyde0h/

It's kind of rough, seeing as it's normally a dropdown menu that would slide down after clicking the menu icon that's not present in the JSfiddle, but you get the idea. Also, the N's would of course be ornaments.

How do I get the same result with less code?

Upvotes: 0

Views: 37

Answers (3)

Steven Web
Steven Web

Reputation: 1961

Mario Araque is doing it best from my point of view. I would also add the stop function like:

        $('.navLink').hover(
            function () {
                $(this).closest("li").find('.ornament').stop(true).fadeTo('med', 1.0);
            }, function () {
                $(this).closest("li").find('.ornament').stop(true).fadeTo('med', 0);
            }
        );

This will clear the fade queue and the hover effect appears only once!

Upvotes: 0

jibe
jibe

Reputation: 645

Hi you can do it like this

$(function(){
    $('.navLink1', '.navLink2', '.navLink3', '.navLink4').hover{
        // Get current id
        var current_id_splitted = $(this).attr('id').split('navLink');
        var current_id          = current_id_splitted[1];

        // Do the ornament
        $('.ornament' + current_id)
            .fadeTo('med', 1.0)
            .fadeTo('med', 0);
    }
})

And if you can update hour HTML by puting a navLink class for the 4 elements,

$(function(){
        $('.navLink').hover{
            // Get current id
            var current_id_splitted = $(this).attr('id').split('navLink');
            var current_id          = current_id_splitted[1];

            // Do the ornament
            $('.ornament' + current_id)
                .fadeTo('med', 1.0)
                .fadeTo('med', 0);
        }
    })

But this is optional,

Have a good weekend

Upvotes: 1

Mario Araque
Mario Araque

Reputation: 4572

You can merge all your events into one. For example this HTML:

<ul class="mobile-navigation">
    <li>
        <span class="ornament-left ornament">N</span>
        <a href="#" class="navLink">Home</a>
        <span class="ornament-right ornament">N</span>
    </li>
    <li>
        <span class="ornament-left ornament">N</span>
        <a href="#" class="navLink">About</a>
        <span class="ornament-right ornament">N</span>
    </li>
    <li>
        <span class="ornament-left ornament">N</span>
        <a href="#" class="navLink">Work</a>
        <span class="ornament-right ornament">N</span>
    </li>
    <li>
        <span class="ornament-left ornament">N</span>
        <a href="#" class="navLink">Contact</a>
        <span class="ornament-right ornament">N</span>
    </li>
</ul>

And the Javascript code:

$(function(){
   $('.navLink').hover(function() {
       $(this).parents('li:first').find('.ornament').fadeTo('med', 1.0);
   }, function() {
       $(this).parents('li:first').find('.ornament').fadeTo('med', 0);
   });
})

You don't need all this variables, also remove it.

Hope it helps.

Regards.

Upvotes: 1

Related Questions