SitePoint Sponsor

User Tag List

Results 1 to 5 of 5
  1. #1
    SitePoint Zealot
    Join Date
    Sep 2004
    Location
    usa
    Posts
    141
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    need help fixing exp/collaps text script

    hi all,

    i'm pretty sure the issue is with the "gintNumberofQuestions=2;" line location. it appears to be outside of any script. but i have no idea where it should be.

    also i would like to replace the image specs in the expand/collapse all script with the words "expand all" and "collapse all." i tried doing it with missing images, but the words don't toggle back and forth, it stays at expand all always.

    thanks in advance for your help.

    Code:
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
    <title>Untitled Document</title>
    </head>
    
    <body>
    
    <!--Collapse text script, paste into "body"-->
    
    <Script Language=JavaScript>
    function ChangeView(iintQuestion)
    {
    var lstrName = "Question"+iintQuestion;
    		
    	if (eval(lstrName+'.style.display')=="block")
    	{
    		eval("document.all."+lstrName+".style.display='none'");
    	} 
    	else
    	{
    		eval("document.all."+lstrName+".style.display='block'");
    	}
    	
    	return false;
    }
    
    gintNumberofQuestions=2; 
    <!--change to match number of questions-->
    
    function ExpandAll()
    {
    	var lstrName;
    	for (var i=1;i<=gintNumberofQuestions;i++)
    	{
    		lstrName = "Question"+i;
    		if (eval('document.all.'+lstrName+'.style.display')=="none") {
    			document.all.expandLink.innerText="Collapse All";
    			eval("document.all."+lstrName+".style.display='block'");
    			document.all.expandImg.src="minus.gif";
    		} else {
    			document.all.expandLink.innerText="Expand All";
    			eval("document.all."+lstrName+".style.display='none'");
    			document.all.expandImg.src="plus.gif";
    		}
    	}
    
    	return false;
    }	
    
    </Script>	
    			
    <!--expand all/collapse all command, paste into body at top of expanding text-->
    			
    			<a href="#" OnClick="blur();ExpandAll();return false;">
    <img id="expandImg" name="expandImg" src="plus.gif" border="0" width="11" height="11">
    </a><a href="#" OnClick="blur();ExpandAll();return false;" name="expandLink" id="expandLink" style="width: 92; height: 16">Expand All</a><br>
                    <br>                
    <a HREF="#" OnClick="blur();ChangeView(1);return false;">FIRST QUESTION HERE</a> <br>
                    <br>
    
    
    <!--question cluster, copy and paste up to number of questions, changeview and question numbers must match-->
    <div id="Question1" style="display: none">FIRST ANSWER HERE<br><br></div>
                    
      <a HREF="#" OnClick="blur();ChangeView(2);return false;">SECOND QUESTION HERE<br></a>
                    <br>
    <div id="Question2" style="display: none">SECOND ANSWER HERE<br><br></div>
                      
             
    
    </body>
    </html>

  2. #2
    SitePoint Zealot
    Join Date
    Sep 2004
    Location
    usa
    Posts
    141
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    i just opened it on a pc and this actually seems to work in IE/PC.

    but not FF/PC.

    or FF-Safari/MAC.

    thanks again. :-)

  3. #3
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,702
    Mentioned
    101 Post(s)
    Tagged
    4 Thread(s)
    The script is using eval in horrendous ways, and is making use of an IE assumption that global variables are created for all html identifiers.

    Change

    Code javascript:
    eval("document.all."+lstrName+".style.display='none'");

    to

    Code javascript:
    document.getElementById(lstrName).style.display = 'none';

    or even better, set assign the element itself to a variable

    Code javascript:
    el = document.getElementById('Question' + iintQuestion);
    ...
    el.style.display = 'none';

    Also, you should not set the display style to block, as that causes trouble when the element originally wasn't a block to begin with. It's far better to set it to an empty string which allows the element to revert back to the default display type that it had.

    With those improvements you can take the code from

    Code javascript:
    function ChangeView(iintQuestion)
    {
    var lstrName = "Question"+iintQuestion;
     
    	if (eval(lstrName+'.style.display')=="block")
    	{
    		eval("document.all."+lstrName+".style.display='none'");
    	} 
    	else
    	{
    		eval("document.all."+lstrName+".style.display='block'");
    	}
     
    	return false;
    }

    and turn it into

    Code javascript:
    function ChangeView(iintQuestion) {
    	var el = document.getElementById('Question' + iintQuestion);
    	if (el.style.display === '') {
    		el.style.display = 'none';
    	} else {
    		el.style.display = '';
    	}
    	return false;
    }

    The gintNumberofQuestions is an awful hack. You should not have to specify ahead of time how many elements there are. Instead, you should just keep processing them until a matching element isn't found.

    With those changes and the previously mentioned ones, you can change

    Code javascript:
    gintNumberofQuestions=2; 
    <!--change to match number of questions-->
     
    function ExpandAll()
    {
    	var lstrName;
    	for (var i=1;i<=gintNumberofQuestions;i++)
    	{
    		lstrName = "Question"+i;
    		if (eval('document.all.'+lstrName+'.style.display')=="none") {
    			document.all.expandLink.innerText="Collapse All";
    			eval("document.all."+lstrName+".style.display='block'");
    			document.all.expandImg.src="minus.gif";
    		} else {
    			document.all.expandLink.innerText="Expand All";
    			eval("document.all."+lstrName+".style.display='none'");
    			document.all.expandImg.src="plus.gif";
    		}
    	}
    	return false;
    }

    into the following

    Code javascript:
    function ExpandAll() {
    	var questionNumber = 1;
    	var el = document.getElementById('Question' + questionNumber);
    	var expandLink = document.getElementById('expandLink');
    	var expandImg = document.getElementById('expandImg');
    	while (el) {
    		if (el.style.display === 'none') {
    			expandLink.innerText = 'Collapse All';
    			el.style.display = '';
    			expandImg.src = 'minus.gif';
    		} else {
    			expandLink.innerText = 'Expand All';
    			el.style.display = 'none';
    			expandImg.src = 'plus.gif';
    		}
    		questionNumber += 1;
    		el = document.getElementById('Question' + questionNumber);
    	}
    	return false;
    }

    I'm still not keen about that innerText part, but that will do for the javascript for now. We haven't even touched on the html yet. It's currently a mess and is going to cause problems when javascript isn't enabled. I'll look at that in my next post.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  4. #4
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,702
    Mentioned
    101 Post(s)
    Tagged
    4 Thread(s)
    With the html, I want to turn it from

    Code html4strict:
    <a href="#" OnClick="blur();ExpandAll();return false;"><img id="expandImg" name="expandImg" src="plus.gif" border="0" width="11" height="11"></a>
    <a href="#" OnClick="blur();ExpandAll();return false;" name="expandLink" id="expandLink" style="width: 92; height: 16">Expand All</a><br>
    <br>                
    <a HREF="#" OnClick="blur();ChangeView(1);return false;">FIRST QUESTION HERE</a><br>
    <br>
     
    <div id="Question1" style="display: none">FIRST ANSWER HERE<br><br></div>
    <a HREF="#" OnClick="blur();ChangeView(2);return false;">SECOND QUESTION HERE<br></a>
    <br>
    <div id="Question2" style="display: none">SECOND ANSWER HERE<br><br></div>

    into this

    Code html4strict:
    <p class="question">FIRST QUESTION HERE</p>
    <p class="answer">FIRST ANSWER HERE</p>
    <p class="question">SECOND QUESTION HERE</p>
    <p class="answer">SECOND ANSWER HERE</p>

    Identifier attributes aren't of much use. While they provide useful hooks from the script into the html, it is difficult to apply any presentational styling to them. identifiers really should be restricted to marking out the top level of a major section.

    But now we have a lot of class attributes, which while allowing us to style things, further clutters things on the page. When there is a lot of them as we can see in the above code, that's an indication that there is some other way that can more effectively convey the intended structure.

    In this case, the definition list is the structure that we should use here. It will make it easy to apply stylings to different sections, and also makes it easy from the scripting side of things too.

    So the html that we want to finish up with is:

    Code html4strict:
    <dl class="questions">
        <dt>FIRST QUESTION HERE</dt>
        <dd>FIRST ANSWER HERE</dd>
        <dt>SECOND QUESTION HERE</dt>
        <dd>SECOND ANSWER HERE</dd>
    </dl>

    That is how the html should look, and only takes a few things to turn it into our desired look and feel.

    If you don't like how the web browser pushes in the answer part, you can easily do something about that with a bit of css

    Code css:
    dl.questions dd {
        margin: 0;
    }

    Currently the script is above the body content, so we'll just put the script at the bottom which is in accordance with the best practices for speeding up your web site.

    I will also place the script code in an external file, so that the body code now looks like this:

    Code html4strict:
    <dl class="questions">
        <dt>FIRST QUESTION HERE</dt>
        <dd>FIRST ANSWER HERE</dd>
        <dt>SECOND QUESTION HERE</dt>
        <dd>SECOND ANSWER HERE</dd>
    </dl>
    <script src="js/toggleQuestions.js"></script>

    With the scripting, we want to place a link above the first question with its image, and then we want to place an onclick action on the questions that will activate the answers.

    Now that we know what we're after, the third and final part will take us through the scripting.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  5. #5
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,702
    Mentioned
    101 Post(s)
    Tagged
    4 Thread(s)
    I'm going to use an object called Expand with methods like Expand.change() and Expand.all(). It is a self-invoking function that runs some init code, and returns the methods that we will be providing access to. This helps to keep things nice and tidy.

    The structure for it is going to be

    Code javascript:
    var Expand = {
        change: function (question) {
            ...
        },
        all: function () {
            ...
        },
        createLink: function () {
            ...
        },
        init: function () {
            ...
        }
    };
    Expand.init();

    Another type of structure would allow the createLink function to be used by the other functions, but not reveal it to the outside world, however that can be for some other time.

    Let's work on placing the link above the first question. This will mean obtaining a reference to the definition list, creating the elements for the link, and placing it before the definition list.

    Code javascript:
    init: function () {
    	var link = Expand.createLink('collapse'),
    		els = document.getElementsByTagName('dl'),
    		elsLen = els.length,
    		el,
    		i;
    	for (i = 0; i < elsLen; i += 1) {
    		el = els[i];
    		if (el.className === 'questions') {
    			el.insertBefore(link, el.firstChild);
    			link.onclick();
    			el.onclick = Expand.change;
    		}
    	}
    }

    We have added a collapse link, and then activated it, which hides the answers when poeple first load the page.

    When we create the link, we'll want it to look something like this:

    Code html4strict:
    <a href="#"><img src="plus.gif">Expand All</a>

    It may be better to use a button instead of a command link because links tell people they're going to go to another page, yet according to the usability article about command links, it's still alright to use links when the consequences are small.

    Here is the script that creates the link for us, and will be used to change the link when everything is expanded or collapsed.

    Code javascript:
    createLink: function (type) {
    	var info = {
    		expand: {text: 'Expand All', img: 'plus.gif'},
    		collapse: {text: 'Collapse All', img: 'minus.gif'}
    	},
    	    a = document.createElement('a'),
    	    text = document.createTextNode(info[type].text),
    	    img = document.createElement(info[type].img);
    	a.href = '#'
    	a.state = type;
    	a.onclick = Expand.all;
    	a.appendChild(img);
    	a.appendChild(text);
    	return a;
    },

    Because the text and image are changed regularly, it helps if the references to it are kept in the same location. That way it's much easier to rename them to something else later on.

    We don't need to attach an event on to each question for when we click on it. We can instead have only one event on the dl element itself which will capture all clicks inside of it.

    When we've got the element that was clicked on, if it's a dt element (the question) we walk forward until we find a dd element (the answer) which we then can toggle.

    Heres how we do it.

    Code javascript:
    change: function (evt) {
    	evt = evt || window.event;
    	var targ = evt.target || evt.srcElement;
    	if (targ.nodeType === 3) { // Safari uses text node
    		targ = targ.parentNode;
    	}
    	if (targ.nodeName === 'DT') {
    		// get dd element
    		while (targ.nodeName && targ.nodeName !== 'DD') {
    			targ = targ.nextSibling;
    		}
    	}
    	if (targ.nodeName !== 'DD') {
    		return false;
    	}
    	if (targ.style.display === 'none') {
    		targ.style.display = '';
    	} else {
    		targ.style.display = 'none';
    	}
    	return false;
    },

    When it comes to expanding or collapsing all the answers, we'll just walk through them and toggle the ones that don't match the desired state. After they've all been processed we can change the link as well.

    Code javascript:
    all: function () {
    	var state = this.state,
    		link,
    		els = this.parentNode.getElementsByTagName('dd'),
    		elsLen = els.length,
    		el,
    		i;
    	for (i = 0; i < elsLen; i += 1) {
    		el = els[i];
    		if (state === 'expand' && el.style.display === 'none') {
    			Expand.change({target: el});
    		} else if (state === 'collapse' && el.style.display !== 'none') {
    			Expand.change({target: el});
    		}
    	}
    	if (state === 'expand') {
    		link = Expand.createLink('collapse');
    		this.parentNode.replaceChild(link, this);
    	} else {
    		link = Expand.createLink('expand');
    		this.parentNode.replaceChild(link, this);
    	}
    	return false;
    },

    And that is complete. The html has been kept nice and tidy, and the script is easy to modify and work with.

    Here's the full script.

    Code javascript:
    var Expand = {
    	change: function (evt) {
    		evt = evt || window.event;
    		var targ = evt.target || evt.srcElement;
    		if (targ.nodeType === 3) { // Safari uses text node
    			targ = targ.parentNode;
    		}
    		if (targ.nodeName === 'DT') {
    			// get dd element
    			while (targ.nodeName && targ.nodeName !== 'DD') {
    				targ = targ.nextSibling;
    			}
    		}
    		if (targ.nodeName !== 'DD') {
    			return false;
    		}
    		if (targ.style.display === 'none') {
    			targ.style.display = '';
    		} else {
    			targ.style.display = 'none';
    		}
    		return false;
    	},
    	all: function () {
    		var state = this.state,
    			link,
    			els = this.parentNode.getElementsByTagName('dd'),
    			elsLen = els.length,
    			el,
    			i;
    		for (i = 0; i < elsLen; i += 1) {
    			el = els[i];
    			if (state === 'expand' && el.style.display === 'none') {
    				Expand.change({target: el});
    			} else if (state === 'collapse' && el.style.display !== 'none') {
    				Expand.change({target: el});
    			}
    		}
    		if (state === 'expand') {
    			link = Expand.createLink('collapse');
    			this.parentNode.replaceChild(link, this);
    		} else {
    			link = Expand.createLink('expand');
    			this.parentNode.replaceChild(link, this);
    		}
    		return false;
    	},
    	createLink: function (type) {
    		var info = {
    			expand: {text: 'Expand All', img: 'plus.gif'},
    			collapse: {text: 'Collapse All', img: 'minus.gif'}
    		},
    		    a = document.createElement('a'),
    		    text = document.createTextNode(info[type].text),
    		    img = document.createElement(info[type].img);
    		a.href = '#';
    		a.state = type;
    		a.onclick = Expand.all;
    		a.appendChild(img);
    		a.appendChild(text);
    		return a;
    	},
    	init: function () {
    		var link = Expand.createLink('collapse'),
    			els = document.getElementsByTagName('dl'),
    			elsLen = els.length,
    			el,
    			i;
    		for (i = 0; i < elsLen; i += 1) {
    			el = els[i];
    			if (el.className === 'questions') {
    				el.insertBefore(link, el.firstChild);
    				link.onclick();
    				el.onclick = Expand.change;
    			}
    		}
    	}
    };
    Expand.init();
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •