I bet this will only confuse you, but this is the way I'd do the popup part:
<ul id="menu"> <li class="one"><a href="#web"></a></li> <li class="two alt"><a href="#content"></a></li> <li class="three"><a href="#pc"></a></li> <li class="four alt"><a href="#blog"></a></li> <li class="five"><a href="#me"></a></li> <li class="six alt"><a href="#contact"></a></li> </ul> <div id="popBoxes"> <div class="popBox web"> </div> <div class="popBox content"> </div> <div class="popBox pc"> </div> etc... </div> jQuery(function($){ $('.menu li').click(function(){ var n = $(this).attr('href').substring(1); // web, content, pc, etc.. $('#popBoxes').children('.'+n).fadeIn(500) // show the one you want .siblings().hide() // hide the others $("#menu").hide(); $("#menuTop li a").fadeIn("500"); }); }); This single function would work for all menu items, and it's unnecessary to save the popup state. On Jan 8, 1:08 pm, Eric Garside <gars...@gmail.com> wrote: > Okay, the first step here is to be smart about how you write your > html, and manage your hide/show. Use HTML to group your elements > together, so you only ever need to hide a single element, and use the > ref tag on your anchor (<a>) tags to store a code-readable value of > the popup you want to fire. > > <div id="popBoxes"> > <div class="popBox"> > ... > </div> > <div class="popBox"> > ... > </div> > <div class="popBox"> > ... > </div> > </div> > > <a class="launchpop" ref="1">Click to launch pop box 1</a> > <a class="launchpop" ref="2">Click to launch pop box 2</a> > > Doing it this way will save you a BUNCH of headaches later, as we'll > see coming up. Second, be smart about what your code is actually > doing. From first glance, it looks to me that you're never going to > have a situation where more than one popup box is open at a time. If > I'm correct in my assumption, then this should be super helpful for > you: > > var frame = {}; > > function hidePopup(){ > frame.last.hide(); > frame.menu.customFadeIn('fast'); > > } > > function showPopup(label){ > if ( frame.last) > frame.last.hide(); > frame.last = frame.popup.eq(label); > frame.last.customFadeIn('fast'); > frame.top.customFadeIn("fast"); > > } > > jQuery(document).ready(function(){ > frame.menu = jQuery('#menu'); > frame.top = jQuery('#menuTop li a'); > frame.popup = jQuery('#popBoxes').children(); > jQuery('launchpop').click(function(){ > showPopup(jQuery(this).attr('ref')) > return false; > }); > > }); > > Now, I've introduced a new thing here, the "frame" object. In code > where I'm going to be reusing a bunch of references, I keep references > to them in a frame object, so I don't make jQuery go through the > document and find them again. It also really helps to trim up the > code, as you can see there. In the doc.ready function we do the > assignments that make the whole thing work. > > That code should easily replace all the code you had before, and > should work even more effectively. A couple points to understand about > the code there: > > 1. Storing references is always faster than doing the lookups again. > 2. If you hide/show a container element, you don't have to hide/show > all of it's children. It's actually a bunch of extra work/code with no > purpose. Even if you told jQuery to show '#popBoxContent1', if > '#popBox1' was still hidden, the content wouldn't show up anywhere. > 3. I assume you're also using another javascript framework that uses > the "$" function? If your not, you can easily replace all the 'jQuery > (...)' calls with '$(...)', which is nice, since the smaller your JS > files, the faster they get to your users (size matters a lot in JS). > Also, if you aren't using another framework, you can replace this > entire line: 'jQuery(document).ready(function(){' with '$(function(){' > 4. Make use of attribute tags and classes to do your work for you. > Putting ref="1" on the <a> tag is one way to really help. If you know > you're going to be repeating patterns of code, try to find the > simplest way to do it. One thing you should practice to really help > yourself in programming javascript is being super lazy. The lazier you > are, the better your code will work. Frequently ask the question "What > can I do to make this really easy for me to do?". > > That's all I got for now. If you've got questions on any of the code > or ideas I dropped in here, let me know. :) > > On Jan 8, 7:29 am, the_guv <the_...@guvnr.com> wrote: > > > well, a big tx to MorningZ...I have - _or rather MorningZ has_ - managed > > to strip a bunch of extraneous code, but I don't see how I can shorten > > the final function calls. > > > what I have now is ... > > > var Status = {}; > > Status[1] = 0; > > Status[2] = 0; > > Status[3] = 0; > > Status[4] = 0; > > Status[5] = 0; > > Status[6] = 0; > > > function loadPopup(label) { > > if (Status[label] == 0) { > > jQuery("#menu").hide(); > > jQuery("#popBox,#popBox" + label + > > ",#popBoxStrap" + label + > > ",#popBoxContent" + label).customFadeIn("fast"); > > jQuery("#menuTop li a").customFadeIn("fast"); > > Status[label] = 1; > > } > > > } > > > function disablePopup(label) { > > if (Status[label] == 1) { > > jQuery("#popBox,#popBox" + label + > > ",#popBoxStrap" + label + > > ",#popBoxContent" + label).hide(); > > jQuery("#menu").customFadeIn("fast"); > > Status[label] = 0; > > } > > > } > > > jQuery(document).ready(function(){ > > > jQuery("#popBox,#popBox1,#popBox2,#popBox3,#popBox4,#popBox5,#popBox6,#menu > > Top > > li a").hide(); > > > jQuery(".one").click(function(){ > > disablePopup(2); > > disablePopup(3); > > disablePopup(4); > > disablePopup(5); > > disablePopup(6); > > loadPopup(1); > > return false; > > > }); > > > jQuery(".two").click(function(){ > > disablePopup(1); > > disablePopup(3); > > disablePopup(4); > > disablePopup(5); > > disablePopup(6); > > loadPopup(2); > > return false; > > > }); > > > (there are a further 4 of these last 2 functions... .three, .four, > > .five, .six) > > > ... > > > it is working a treat, but to compress my code for these final commands, > > MorningZ suggests something like... > > > function menuClick(label) { > > $.each(Status, function(key, val) { > > if (key == label) { > > loadPopup(label); > > } > > else { > > disablePopup(label); > > } > > }); > > > } > > > ... but I can't get that to work. > > > I have tried adding a new class to the menu items, and changing the $ > > to jQuery as I'm using the noconflict rule, but this keeps breaking. > > If anyone can educate me I'd be grateful. > > > This is otherwise working atwww.guvnr.com. > > > Many thanks, > > > Olly. > > > MorningZ wrote: > > > I don't think you'll see many people go through all that code and > > > rewrite it for you, but i will provide some tips to make things easier > > > > 1) Be consistant in your naming.... for instance > > > - You have "popBox1 2 3 ... etc"... yet your "status" names are "One > > > Two Three etc".... why no be consisant and use numbers there? > > > > 2) Use an object to track your "status" > > > - instead of > > > var popupOneStatus = 0; > > > var popupTwoStatus = 0; > > > var popupThreeStatus = 0; > > > var popupFourStatus = 0; > > > var popupFiveStatus = 0; > > > var popupSixStatus = 0; > > > > something like > > > var Status = {}; > > > Status[1] = 0; > > > Status[2] = 0; > > > Status[3] = 0; > > > Status[4] = 0; > > > Status[5] = 0; > > > Status[6] = 0; > > > > is MUCH easier to access, later down the road (keeping in mind point > > > #1) > > > > 3) With that done, you can stop repeating code, this would replace all > > > "Popup" functions > > > > function loadPopup(label) { > > > if (Status[label] == 0) { > > > jQuery("#menu").hide(); > > > jQuery("#popBox,#popBox" + label + > > > ",#popBoxStrap" + label + > > > ",#popBoxContent" + label).fadeIn("500"); > > > jQuery("#menuTop li a").fadeIn("500"); > > > Status[label] = 1; > > > } > > > } > > > > function disablePopup(label) { > > > if (Status[label] == 1) { > > > jQuery("#popBox,#popBox" + label + > > > ",#popBoxStrap" + label + > > > ",#popBoxContent" + label).hide("500"); > > > jQuery("#menu").fadeIn("500"); > > > Status[label] = 0; > > > } > > > } > > > > now a quick function that takes in what popup you want loaded (i don't > > > have the time to show the HTML, but maybe this will get the gears > > > turning) > > > > function menuClick(label) { > > > $.each(Status, function(key, val) { > > > if (key == label) { > > > loadPopup(label); > > > } > > > else { > > > disablePopup(label); > > > } > > > }); > > > } > > > > On Jan 7, 3:38 am, the_guv <the_...@guvnr.com> wrote: > > > >> I'm using a lot of jQuery for my new site, and wonder ... > > > >> How can I better compress my code, which seems very long. > > > >> If any kind souls could tip me off with some pointers, I'd be much, > > >> much, very much obliged. > > > >> Olly > > >> the_guv > > >> guvnr.com > > > >> The site ishttp://guvnr.com > > > >> My jQ is (hopefully fairly self-explanatory) ... (html follows) ... > > > >> jQuery.noConflict(); > > > >> // *** POPUPS/SERVICE PANELS *** (this largest section of code > > >> controls my tab panels for the sections titled Web, Content, PC, Blog, > > >> Me and Content...) > > > >> var popupOneStatus = 0; > > >> var popupTwoStatus = 0; > > >> var popupThreeStatus = 0; > > >> var popupFourStatus = 0; > > >> var popupFiveStatus = 0; > > >> var popupSixStatus = 0; > > > >> //LOAD POPUP > > >> function loadPopupOne(){ > > >> if(popupOneStatus==0){ > > >> jQuery("#menu").hide(); > > >> jQuery("#popBox,#popBox1,#popBoxStrap1,#popBoxContent1").fadeIn > > >> ("500"); > > >> jQuery("#menuTop li a").fadeIn("500"); > > >> popupOneStatus = 1;}} > > > >> function loadPopupTwo(){ > > >> if(popupTwoStatus==0){ > > >> jQuery("#menu").hide(); > > >> jQuery("#popBox,#popBox2,#popBoxStrap2,#popBoxContent2").fadeIn > > >> ("500"); > > >> jQuery("#menuTop li a").fadeIn("500"); > > >> popupTwoStatus = 1;}} > > > >> function loadPopupThree(){ > > >> if(popupThreeStatus==0){ > > >> jQuery("#menu").hide(); > > >> jQuery("#popBox,#popBox3,#popBoxStrap3,#popBoxContent3").fadeIn > > >> ("500"); > > >> jQuery("#menuTop li a").fadeIn("500"); > > >> popupThreeStatus = 1;}} > > > >> function loadPopupFour(){ > > >> if(popupFourStatus==0){ > > >> jQuery("#menu").hide(); > > ... > > read more »