2
\$\begingroup\$

I am writing a function for a rotating navigation with JS and jQuery. I am using a plugin called jQuery transit for animating rotation. The code works, but it is very long for such a simple function. Is there any way this can be improved?

var rotation = 0;
$('nav ul li a').bind('click', function() {
'use strict';
var item = $(this).parent();
var itemI = item.index();
var navAll = $(this).parents('ul');

var condition = rotation + '_' + itemI;

switch(condition)
{
   case '0_0': 
        //do nothing
        break;
    case '0_1':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '0_2':
        navAll.transition({rotate: '-=180deg'});
        $('nav ul li a').transition({rotate:'180deg'});
        rotation = 180;
        break;
    case '0_3':
        navAll.transition({rotate: '+=90deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;


    case '90_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '90_1':
        navAll.transition({rotate: '-90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '90_2':
        navAll.transition({rotate: '+=90deg'});
        $('nav ul li a').transition({rotate:'-180deg'});
        rotation = 180;
        break;
    case '90_3':
        break;

    case '180_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '180_1':
        navAll.transition({rotate: '-90deg'});
        $('nav ul li a').transition({rotate:'90deg'});
        rotation = 270;
        break;
    case '180_2':
        //do nothing
        break;
    case '180_3':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;


    case '270_0':
        navAll.transition({rotate: '0deg'});
        $('nav ul li a').transition({rotate:'0deg'});
        rotation = 0;
        break;
    case '270_1':
        //do nothing
        break;
    case '270_2':
        navAll.transition({rotate: '-=90deg'});
        $('nav ul li a').transition({rotate:'180deg'});
        rotation = 180;
        break;
    case '270_3':
        navAll.transition({rotate: '-=180deg'});
        $('nav ul li a').transition({rotate:'-90deg'});
        rotation = 90;
        break;


}

});
\$\endgroup\$
1
  • \$\begingroup\$ Just realized that the $('nav ul li a') should be put in a variable. Any other suggestions? \$\endgroup\$
    – Devender
    Commented Dec 29, 2012 at 20:39

3 Answers 3

2
\$\begingroup\$

Instead of that complex switch statement, I'd just use an option object:

var allLinks = $('nav ul li a');
var rotation = 0;

$('nav').on('click', 'ul li a', function() {
    'use strict';
    var $this = $(this);
    var item = $this.parent();
    var navAll = $this.parents('ul');

    var option = {
        '0_1': {
            rotateAll: '-=90deg',
            rotateThis: '90deg',
            rotation: 270
        },
        '0_2': {
            rotateAll: '-=180deg',
            rotateThis: '180deg',
            rotation: 180
        }
        // Add them all here
    }[ rotation + '_' + item.index() ];

    if ( option ) {
        navAll.transition({rotate: option.rotateAll});
        allLinks.transition({rotate: option.rotateThis});
        rotation = option.rotation;
    }
}

There are 2 more things to consider:

  1. Are you sure you really need that nav ul li a selector? Wouldn't nav a do the job just the same? While we're at it, why are you listening to the click on the a elements, and then find the li via .parent()? Can't you just listen to the click event on the lis themselves?
  2. Is there any method to that configuration? I wasn't able to deduce any algorithm from what you've provided, but I'm sure there is. You'd be better off calculating it on the fly, if possible.
\$\endgroup\$
1
  • \$\begingroup\$ one methodical aspect I can see: rotation = (3 - item.index()) * 90 \$\endgroup\$
    – Stuart
    Commented Dec 31, 2012 at 2:10
2
\$\begingroup\$

I tried to deduce what it is that you're doing, and what I came up with is that you have four items arranged like a cross and you rotate it when one is clicked so that item is on top. You're also rotating each item, individually, so they continue to be upright during the rotation.

If that's what you have, I think this code should do the same thing. It's set to work with a variable number of items, so, in theory, you should be able to add to or remove from the set and the rotations will adjust.

Here is a demo.

The items in the <ul> are arranged in a circle via CSS.

<ul>
    <li id="a">0</li>
    <li id="b">1</li>
    <li id="c">2</li>
    <li id="d">3</li>
    <li id="e">4</li>
    <li id="f">5</li>
    <li id="g">6</li>
    <li id="h">7</li>
</ul>

The JavaScript:

var idx = 0,
    $lis = $('li'),
    len = $lis.length,
    center = (len - 1) / 2,
    halfLen = len / 2,
    degChg = 360 / len,
    $ul = $('ul');

$ul.on('click', 'li', function() {

    var $this = $(this),
        newIdx = $this.index(),
        chg = getPositionChg(idx, newIdx) * degChg;

    $ul.transition({rotate: '-=' + chg + 'deg'});
    // couldnt get this to work without using
    // the .each() method
    $lis.each(function() {
        $(this).transition({rotate: '+=' + chg + 'deg'});
    });
    idx = newIdx;
});

function getPositionChg(curIdx, nextIdx) {
    if (curIdx < center) {
        // if curIdx is left of center and change does not
        // cross the 0 index (is clockwise)
        if (nextIdx > curIdx + halfLen) {
            return nextIdx - len - curIdx;
        } else {
            return nextIdx - curIdx;
        }       
    } else {
        // if curIdx is right of center and change does
        // cross the 0 index
        if (nextIdx <= curIdx - halfLen) {
            return len - curIdx + nextIdx;
        } else {
            return nextIdx - curIdx;
        }
    }
}

​
\$\endgroup\$
1
  • \$\begingroup\$ I'd recommend extracting the position of the list elements as well so that is dynamically generated with the script. \$\endgroup\$
    – Shmiddty
    Commented Jan 4, 2013 at 19:15
1
\$\begingroup\$

You could do something like this, although there is a slight risk of making the code more cryptic than it is already.

var rotation = 0;
var r1Options = [[undefined, 0, 0, 0],
                 [-90, -90, -90, undefined],
                 [-180, 90, undefined, -180],
                 [90, undefined, -90, -180]],
    r2Options = [[undefined, 0, 0, 0],
                 [90, 90, 90, undefined],
                 [180, -180, undefined, 180],
                 [-90, undefined, -90, -90]];

function rotateString(degs) {
    var sign = '';
    if (degs < 0) sign = '-';
    else if (degs > 0) sign = '+';
    return sign + '=' + Math.abs(degs) + 'deg';
}
$('nav ul li a').bind('click', function() {
    'use strict';
    var item = $(this).parent();
    var itemI = item.index();
    var navAll = $(this).parents('ul');
    var r1 = r1Options[itemI][rotation / 90],
        r2 = r2Options[itemI][rotation / 90];
    if (r1 != undefined && r2 != undefined) {
        navAll.transition({rotate: rotateString(r1)});
        $('nav ul li a').transition({rotate: rotateString(r2)});
        rotation = (3 - itemI) * 90;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.