Making code more efficient?

[Schmoopy]

Hi there, I recently uploaded an image map with centres on it that the user can click and text appears next to the map describing the centre. At the moment it all works fine, but it doesn't seem to be very efficient to me as there is lots of code which I think could be condensed down, although I'm not sure how.

You can see what I mean here:

http://www.bombthehills.com/ridingspots-uk.html

The code I am referring to is this:


<area shape="circle" coords="283,656,7" href="javascript:ReverseDisplay('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="ivyleaf">
<area shape="circle" coords="299,647,7" href="javascript:ReverseDisplay('swmbc'); javascript:HideContent('ivyleaf'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="MBC">
<area shape="circle" coords="351,636,8" href="javascript:ReverseDisplay('quantock'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="quantock">
<area shape="circle" coords="459,643,8" href="javascript:ReverseDisplay('haredown'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="haredown">
<area shape="circle" coords="501,641,8" href="javascript:ReverseDisplay('knockhill'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="knockhill">
<area shape="circle" coords="285,253,9" href="javascript:ReverseDisplay('airfusion'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="airfusion">
<area shape="circle" coords="488,616,8" href="javascript:ReverseDisplay('xbp'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="xbp">
<area shape="circle" coords="464,606,8" href="javascript:ReverseDisplay('oakwood'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="oakwood">
<area shape="circle" coords="447,595,8" href="javascript:ReverseDisplay('waterside'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="waterside">
<area shape="circle" coords="291,291,8" href="javascript:ReverseDisplay('newmilns'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="newmilns">
<area shape="circle" coords="364,476,8" href="javascript:ReverseDisplay('halesb'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="halesb">
<area shape="circle" coords="361,514,10" href="javascript:ReverseDisplay('edge'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="edge">
<area shape="circle" coords="359,545,8" href="javascript:ReverseDisplay('courtfarm'); javascript:HideContent('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="courtfarm">
etc... (Too long for post)

What I want to happen is that when you click on a centre, it hides any centre that is currently showing and replaces it with the one the user clicks, so instead of writing out HideContent for every single possibility is there not a way where i can use (*) or something to hide all? This would make it much more readable and save on time and file size.

Hope you can help with this,

Thanks,

Jack.

[rangana]

First, this part:


function ReverseDisplay(d) {
if(document.getElementById(d).style.display == "none") { document.getElementById(d).style.display = "block"; }
else { document.getElementById(d).style.display = "none"; }
}


...could be trimmed to:


function ReverseDisplay(d) {
var el=document.getElementById(d);
el.style.display=el.style.display!='none'?'none':'';
}


You could modify your HideContent function to:


var hideEl=['ivyleaf','swmbc','quantock','haredown','knockhill',
'airfusion','xbp','oakwood','waterside','newmilns','halesb','edge','courtfarm',
'redhill','outtograss','bugsboarding'
]; // Place in this array the ID of the elements involved

function HideContent() {
for(var i=0;i<hideEl.length;i++)
document.getElementById(hideEl[i]).style.display = "none";
}


That way, you can change (for example this line in) your markup:


<area shape="circle" coords="283,656,7" href="javascript:ReverseDisplay('ivyleaf'); javascript:HideContent('swmbc'); javascript:HideContent('quantock'); javascript:HideContent('haredown'); javascript:HideContent('knockhill'); javascript:HideContent('airfusion'); javascript:HideContent('xbp'); javascript:HideContent('oakwood'); javascript:HideContent('waterside'); javascript:HideContent('newmilns'); javascript:HideContent('halesb'); javascript:HideContent('edge'); javascript:HideContent('courtfarm'); javascript:HideContent('redhill'); javascript:HideContent('outtograss'); javascript:HideContent('bugsboarding');" alt="ivyleaf">


...to:


<area shape="circle" coords="283,656,7" href="#" onclick="HideContent();ReverseDisplay('ivyleaf');return false;" alt="ivyleaf">


Avoid using javascript pseudo url as it will result into your page's invalidity.

Hope that makes sense.

[Schmoopy]

Just implemented the code, thanks a lot for that, it's going to save me a lot of time when adding new centres :D

Now just to sort out the weird blank space at the bottom that occurs when you click on a centre =/

I guess that's to do with the CSS though,

Still, thanks for this great code :)

Jack.

[Schmoopy]

Only thing that's not quite working yet is the show content part:

function ReverseDisplay(d) {
var centres=document.getElementById(d);
centres.style.display=centres.style.display!='none'?'none':'';
}

It does work when I keep it like it was before, but the centres can't be hidden again, as in, once you've clicked on a centre you can't hide it again, it will stay there until a new centre is clicked, which isn't too much of a problem but it's still just a little thing.

There's something wrong with the last part of the code I think :

!='none'?'none':'';

But I'm just that bad at javascript I can't fix it myself :( *ashamed*

Never seen a ? used in javascript so it's sort of new for me,

Hope this is just a quick typo,

Jack :)

[rangana]

Update your page.

Also, ensure that HideContent(); comes early before ReverseDisplay('element_id');.

As in:


<area shape="circle" coords="283,656,7" href="#" onclick="HideContent();ReverseDisplay('ivyleaf');return false;" alt="ivyleaf">

[Schmoopy]

I haven't updated the page yet because it doesn't fully work, nothing's happening if I use the code you submitted for the "ReverseDisplay" function =/

JS :


function HideContent() {
for(var i=0;i<hideCentres.length;i++)
document.getElementById(hideCentres[i]).style.display = "none";
}

function ShowContent(d) {
document.getElementById(d).style.display = "block";
}

function ReverseDisplay(d) {
var centres=document.getElementById(d);
centres.style.display=centres.style.display!='none'?'none':'';
}

var hideCentres=['ivyleaf','swmbc','quantock','haredown','knockhill',
'airfusion','xbp','oakwood','waterside','newmilns','halesb','edge','courtfarm',
'redhill','outtograss','bugsboarding'];

HTML:


<area shape="circle" coords="283,656,7" href="#" onclick="HideContent(); ReverseDisplay('ivyleaf'); return false;" alt="ivyleaf">
<area shape="circle" coords="299,647,7" href="#" onclick="HideContent(); ReverseDisplay('swmbc'); return false;" alt="SWMBC">
<area shape="circle" coords="351,636,8" href="#" onclick="HideContent(); ReverseDisplay('quantock'); return false;" alt="quantock">
<area shape="circle" coords="459,643,8" href="#" onclick="HideContent(); ReverseDisplay('haredown'); return false;" alt="haredown">
<area shape="circle" coords="501,641,8" href="#" onclick="HideContent(); ReverseDisplay('knockhill'); return false;" alt="knockhill">
<area shape="circle" coords="285,253,9" href="#" onclick="HideContent(); ReverseDisplay('airfusion'); return false;" alt="airfusion">
<area shape="circle" coords="488,616,8" href="#" onclick="HideContent(); ReverseDisplay('xbp'); return false;" alt="xbp">
<area shape="circle" coords="464,606,8" href="#" onclick="HideContent(); ReverseDisplay('oakwood'); return false;" alt="oakwood">
<area shape="circle" coords="447,595,8" href="#" onclick="HideContent(); ReverseDisplay('waterside'); return false;" alt="waterside">
<area shape="circle" coords="291,291,8" href="#" onclick="HideContent(); ReverseDisplay('newmilns'); return false;" alt="newmilns">
<area shape="circle" coords="364,476,8" href="#" onclick="HideContent(); ReverseDisplay('halesb'); return false;" alt="halesb">
<area shape="circle" coords="361,514,10" href="#" onclick="HideContent(); ReverseDisplay('edge'); return false;" alt="edge">
<area shape="circle" coords="359,545,8" href="#" onclick="HideContent(); ReverseDisplay('courtfarm'); return false;" alt="courtfarm">
<area shape="circle" coords="368,572,8" href="#" onclick="HideContent(); ReverseDisplay('redhill'); return false;" alt="redhill">
<area shape="circle" coords="378,552,7" href="#" onclick="HideContent(); ReverseDisplay('outtograss'); return false;" alt="outtograss">
<area shape="circle" coords="399,560,8" href="#" onclick="HideContent(); ReverseDisplay('bugsboarding'); return false;" alt="bugsboarding">

Nothing happens when you click on the centres, only if I revert back to the old ReverseDisplay function does anything happen.

Are you sure it's right?

Jack.

[Schmoopy]

Oh wait, I added "block" after the 'none'?'none':'block';

And that's worked, it's still not hiding it though if the centre is already visible, only changing when a new centre is clicked, updated version at:

http://www.bombthehills.com/ridingspots-uk.html

Jack.

[rangana]

It seemed that you haven't updated your ukmap.js (http://www.bombthehills.com/ukmap.js).

[Schmoopy]

Ok updated it again, but since it's still not working, and i'm using the exact code you said it's here:

http://www.bombthehills.com/ridingspots-ukbeta.html

and the JS file at:

http://www.bombthehills.com/ukmap2.js

But still nothing happening :(

[rangana]

This file: http://www.bombthehills.com/ridingspots-ukbeta.html

...is still pointing on ukmap.js

[Schmoopy]

Ok yea sorry, my mistake :P

It's now pointing to ukmap2.js :)

[rangana]

I don't think I understand the flow of your page.

What should happen when I click on the mapped area? What's changing? I don't see any difference.

[Schmoopy]

If you go to http://www.bombthehills.com/ridingspots-uk.html

You'll see that when you click on the icons on the map, text appears to the left of the map, and when you click again, it should hide, but it's not at the moment.

But when I use the code you've given me at ukmap2.js, nothing happens when you click on the centres,

entiende?

You know what's wrong with it?

Thanks,

Jack.

[rangana]

No, I don't understand as the page you are pointing as an example throws error.

[Schmoopy]

the ridingspots-uk one? Or the ridingspots-ukbeta one?

The ridingspots-uk one is the one that works for me, using a modified version of your code, and the ridingspots-ukbeta one doesn't work, and that one is using the code you said earlier in the thread =/

Jack.

[rangana]

Change ReverseDisplay function to:


function ReverseDisplay(d) {
var centres=document.getElementById(d);
centres.style.display='block';
}

[clueful]

If you go to http://www.bombthehills.com/ridingspots-uk.html

You'll see that when you click on the icons on the map, text appears to the left of the map, and when you click again, it should hide


function ShowMapAreaDiv()
{
this.elems=[];
this.params=arguments;

this.init=function()
{
var args=this.params, areas=document.getElementById( args[0] ).getElementsByTagName('area');

for(var i=1, len=args.length; i<len; i++)
{
this.elems[ args[i] ] = document.getElementById(args[i]);

if(!this.elems[ args[i] ])
alert( args[i] +' Not Found');

areas[i-1].onclick=(function(obj, ident)
{
return function()
{
var elems=obj.elems;

if(elems[ident].style.display=='block')
elems[ident].style.display='none';
else
for(var i in elems)
elems[i].style.display = (elems[i]===elems[ident]) ? 'block' : 'none';

return false;
}
})(this, args[i]);
}
}

this.init();
}



<script type="text/javascript" >

new ShowMapAreaDiv('map','ivyleaf','swmbc','quantock','haredown','knockhill','airfusion','xbp','oakwood','waterside','newmilns','halesb','edge','courtfarm','redhill ','outtograss','bugsboarding');

</script>

[Schmoopy]

Gracia por tu ayuda,

Sorry my spanish isn't great...

Thanks, all works well now,

Jack

[Schmoopy]

Nice, even better, exactly how I wanted it thanks :)