sugafree Posted June 25, 2014 Share Posted June 25, 2014 Hi, Im trying to run the showPic function without putting any JS into HTML(when i used onclick = showPic() return false, it worked fine). I added prepareGallery function for this reason and I cant get it working. I think I can see it changes the pics but link will be executed also. Anyone got an idea where I made the mistake? <body><h1>Snapshots</h1><ol id="links"><li><a href="benfica.jpg" title="benfica">Benfica</a></li><li><a href="yamaha.jpg" title="yamaha">Yamaha</a></li><li><a href="island.jpg" title="island">Island</a></li><li><a href="opossum.JPG" title="opossum">Opossum</a></li></ol><img id="placeholder" src="1.jpg" alt="my image gallery" /><p id="desc">Choose an image</p></body>function showPic(whichpic) {if (!document.getElementById("placeholder")) return false; var source = whichpic.getAttribute("href");var placeholder = document.getElementById("placeholder");placeholder.setAttribute("src",source);if (!document.getElementsById("desc")) return false;var titletext = whichpic.getAttribute("title");var desc = document.getElementById("desc"); desc.firstChild.nodeValue = titletext;return false;}function prepareGallery(){if (!document.getElementById || !document.getElementsByTagName) return false;var links = document.getElementById("links");var actLinks = links.getElementsByTagName("a");for (var i = 0; i < actLinks.length; i++){actLinks[i].onclick = function(){showPic(this);return false;}}}window.onload = prepareGallery; Link to comment Share on other sites More sharing options...
justsomeguy Posted June 25, 2014 Share Posted June 25, 2014 You're passing this, which is set to the window object, not the element. You should use a closure to pass the element into the event handler. Link to comment Share on other sites More sharing options...
sugafree Posted June 25, 2014 Author Share Posted June 25, 2014 (edited) which supposed to be whichpic right? Now just tried showPic() and showPic(whicpic). No luckyet Edited June 25, 2014 by sugafree Link to comment Share on other sites More sharing options...
justsomeguy Posted June 25, 2014 Share Posted June 25, 2014 actLinks[i].onclick = (function(el) { return function(){ showPic(el); return false; }})(actLinks[i]); Link to comment Share on other sites More sharing options...
sugafree Posted June 25, 2014 Author Share Posted June 25, 2014 still the same... could you tell me what is (el)? also why do i need (actLinks); at the very end Link to comment Share on other sites More sharing options...
davej Posted June 25, 2014 Share Posted June 25, 2014 Try... <!DOCTYPE html><html><head><meta charset="utf-8"/><title>???</title><script>window.onerror = function(m, u, l){alert('Javascript Error: '+m+'nURL: '+u+'nLine Number: '+l);return true;}</script><script>if (typeof cs1 != 'undefined'){alert('codespace collision: cs1');}else{cs1 = {};cs1.showPic = function() {var source = this.getAttribute("href");document.getElementById("placeholder").setAttribute("src",source);var titletext = this.getAttribute("title");document.getElementById("desc").firstChild.nodeValue = titletext;return false;}cs1.prepareGallery = function(){var actLinks = document.getElementById("links").getElementsByTagName("a");for (var i = 0; i < actLinks.length; i++){actLinks[i].onclick = cs1.showPic;}}window.onload = cs1.prepareGallery;}</script></head><body><h1>Snapshots</h1><ol id="links"><li><a href="benfica.jpg" title="benfica">Benfica</a></li><li><a href="yamaha.jpg" title="yamaha">Yamaha</a></li><li><a href="island.jpg" title="island">Island</a></li><li><a href="opossum.JPG" title="opossum">Opossum</a></li></ol><img id="placeholder" src="1.jpg" alt="my image gallery" /><p id="desc">Choose an image</p></body></html> ...or to get more into the mood of these other suggestions try... <script>window.onload = function(){showPic = function() { var source = this.getAttribute("href");document.getElementById("placeholder").setAttribute("src",source);var titletext = this.getAttribute("title");document.getElementById("desc").firstChild.nodeValue = titletext;return false;}var actLinks = document.getElementById("links").getElementsByTagName("a");for (var i = 0; i < actLinks.length; i++){ actLinks[i].onclick = showPic;}}//end of window.onload</script> Link to comment Share on other sites More sharing options...
justsomeguy Posted June 25, 2014 Share Posted June 25, 2014 still the same... could you tell me what is (el)? also why do i need (actLinks); at the very end el is the parameter to the function, and actLinks gets passed to that function as the parameter. Inside the function, el is the anchor element.Are you doing any debugging? Are you checking for error messages in the console? You should verify inside the showpic function what the element that you're passing is set to, and step through the function to figure out what it is actually doing. If it's not executing the final return statement then there is a fatal error stopping it, so figure out what the error is. Link to comment Share on other sites More sharing options...
Hadien Posted June 26, 2014 Share Posted June 26, 2014 try and look at Justsomeguy's code step-by-step. work by each indent level of the code and ignore all other code on other indents, per step.so the 1st indent is: actLinks[i].onclick = (function(el) { ........})(actLinks[i]); Notice that he defined the entire function inside parenthesis. Doing so puts the function in a scope that no one can use after the semi-colon. This is for good reason as this "wrapper function" is never needed anywhere else and would just pollute the namespace. second benefit of wrapping it in parenthesis is that you can instantly call the function. And that is what the 2nd set of parenthesis is for. It tells the javascript interpreter to immediately run the function that was just defined with the 2nd parenthesis holding the parameter for the function (meaning that actLinks will become el). Its basically the same as this code... actLinks[i].onclick = function(el) { return function(){ showPic(el); return false; }}actLinks[i].onclick = actLinks[i].onclick(actLinks[i]); They both basically do the same thing. Yet JustsomeGuys is more straightforward and concise, and doesn't write and rewrite the onclick handler multiple times. 1 Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now