M.O.S. Studios Posted June 21, 2015 Share Posted June 21, 2015 so I have an object that contains the information I need to create a navbar within my CMS. I have the code working, but I feel I can make it more efficient. at the moment here is the OOO of the script: Iterate through the object and does three JQuery commands: 1. Creates an LI object 2. give the new LI tag a: content = ' ', an new ID (taken from the object's key it's iterating through), and add an internal DIV; 3. populate that new internal DIV with the URL the LI goto 4. iterate through all the new LI's and create a 'click' listener that send the browser to that link. A few things should be noted. The DIV's that are created are invisible. they are just there to hold the information for the script This is a image based Navbar. So the LI's link to a StyleSheet. Thus the unique id I think the first and second step can be combined into one. I also feel If I could assign each LI the listener as they are being created I could make the whole thing cleaner. When I try this way, all the links end up leading to the same place. I remember this having something to do with "reference" information opposed to "String". If I remember right, When the script iterates and assigns the URL to the listener, it's not saving the information, it's saving a reference to a variable. When that variable changes, it alters all the variables that reference it. I term 'function factory" comes to mind; but a google search didn't help Any thoughts? Here is the code var navArray = {'navHOME' : 'index.php', 'navLOGIN' : 'login.php', 'navCART' : 'CART.php', 'navcontact' : 'contact.php'}; for (keyValue in navArray){ /*step 1 */ $('#navMain ul').append('<li>'); /*step 2 */ $('#navMain ul li').last().text(' ').attr('id', keyValue).append('<div>'); /*step 3 */ $('#navMain ul li:last div').attr('id', 'value').text(navArray[keyValue]); } /*step 4 */ $('#navMain ul > li').on('click', function(){window.location = $(this).children('div#value').text();}); Quote Link to comment Share on other sites More sharing options...
requinix Posted June 21, 2015 Share Posted June 21, 2015 (edited) You can combine 1-3 by constructing the whole HTML, rather than starting with the and modifying it with jQuery. " " + navArray[keyValue] + " "The URL isn't going to be HTML-unsafe so you don't need to escape it using something like .text(). Step 4 can be done with just one listener on the parent element. $("#navMain ul").on("click", "li", function() { window.location = $(this).children("div#value").text(); });Otherwise you're creating event listeners for every element and you don't need to do that. Plus this can be executed before adding the LIs so you don't have to worry about that. And by the way, the ID on the DIV is wrong: IDs need to be unique across the entire document, but fortunately you don't need an ID in the first place because there's just the one div so .children("div") would be just fine. But more important than all this: (1) why aren't you using just plain links and (2) why are you doing this in Javascript when it's just a static set of links? Edited June 21, 2015 by requinix Quote Link to comment Share on other sites More sharing options...
M.O.S. Studios Posted June 21, 2015 Author Share Posted June 21, 2015 (edited) I didn't know that about Id's. I thought they had to be unique within their own siblings. I am making it using PNG's and CSS for two reasons 1. I need unique font, and this is the best way to get it 2. When I need to translate the page, I can have one script to change the CSS Here is how I ended up doing it, But I may change that now to have one listener like you described. (function( $ ){ $.fn.navMenuADD = function(URL){$(this).click(function(){window.location = URL;});}; })( jQuery ); for (keyValue in navArray){ $('#navMain ul').append('<li>'); $('#navMain ul li').last().text(' ').attr('id', keyValue).navMenuADD(navArray[keyValue]); } Edited June 21, 2015 by M.O.S. Studios Quote Link to comment Share on other sites More sharing options...
M.O.S. Studios Posted June 21, 2015 Author Share Posted June 21, 2015 I've changed it. it now works masterfully effectively. for (keyValue in navArray){ $('#navMain ul').append('<li>'); $('#navMain ul li').last().text(' ').attr('id', keyValue).append('<div>'); $('#navMain ul li:last div').text(navArray[keyValue]); } $('#navMain ul').on('click', 'li', function(){window.location = $(this).children('div').text();}); Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.