taggs74 Posted January 9, 2018 Share Posted January 9, 2018 Hi, I'm having a problem changing my image onclick attribute. I think it is a syntax issue around the var imageindex1 but I'm not sure. I think the error is with line 3 because if I take it out the page runs without error albeit not to the desired effect. Any help is much appreciated. document.getElementById("menu_image_1").src = $(this).find(":selected").attr("data-src1"); imageindex1=parseFloat($(this).find(":selected").attr("data-img-index")) +1 document.getElementById("menu_image_1").onclick = currentDiv(imageindex1); Many thanks Link to comment Share on other sites More sharing options...
Ingolme Posted January 9, 2018 Share Posted January 9, 2018 When setting a function to an event handler, you have to pass a reference to the function, not call the function. This means that the function cannot have arguments and it cannot use parentheses. This would be the correct way to add currentDiv as an event handler: document.getElementById("menu_image_1").onclick = currentDiv; The currentDiv function can access the image element using the variable called "this" Link to comment Share on other sites More sharing options...
taggs74 Posted January 9, 2018 Author Share Posted January 9, 2018 if I use document.getElementById("menu_image_1").onclick = currentDiv(1); the page works fine and passes parameter with my function running correctly. I can change "currentDiv(to any number in my index )" and all is good. the variable "this" is taken from another element that "helps" determine the parameter passed to the function. My problem is, I need to change calculate the argument before it is passed. Link to comment Share on other sites More sharing options...
justsomeguy Posted January 9, 2018 Share Posted January 9, 2018 Quote if I use document.getElementById("menu_image_1").onclick = currentDiv(1); the page works fine If you do that, what you are telling Javascript to do is to execute the currentDiv function immediately, pass it the value 1, and save the return value of that function as the click handler for the other element. What that does not do is assign currentDiv as the click handler. You're assigning the return value of that function (if any), not the function itself, because you are telling it to execute the function immediately. Since you're using jQuery, you can use bind to specify a click handler and what data you want to send to that handler: http://api.jquery.com/bind/ More generally, you can use a closure as the click handler and execute your other function from inside that. Link to comment Share on other sites More sharing options...
iwato Posted January 9, 2018 Share Posted January 9, 2018 (edited) NO! Do not use the .bind( ) function, it has been deprecated. Use the more flexible .on( ) function. Quote As of jQuery 3.0, .bind() has been deprecated. It was superseded by the .on() method for attaching event handlers to a document since jQuery 1.7, so its use was already discouraged. For earlier versions, the .bind() method is used for attaching an event handler directly to elements. Handlers are attached to the currently selected elements in the jQuery object, so those elements must exist at the point the call to .bind() occurs. For more flexible event binding, see the discussion of event delegation in .on(). Great explanation! Quote If you do that, what you are telling Javascript to do is to execute the currentDiv function immediately, pass it the value 1, and save the return value of that function as the click handler for the other element. What that does not do is assign currentDiv as the click handler. You're assigning the return value of that function (if any), not the function itself, because you are telling it to execute the function immediately. Have a great day! Roddy Edited January 9, 2018 by iwato Link to comment Share on other sites More sharing options...
dsonesuk Posted January 10, 2018 Share Posted January 10, 2018 It would help if you show example of all html and JavaScript related to this problem, currently i see code that seems to be related to a slideshow with a dropdown (':selected'). Link to comment Share on other sites More sharing options...
taggs74 Posted January 10, 2018 Author Share Posted January 10, 2018 (edited) Thank you justsomeguy that makes a lot more sense of what is going on. I had a little look at .bind() and read, as iwato says, that it is now deprecated, so started looking at .on() but to no avail. I am just a newbie so please be patient with my lack of coding skills. dsonesuk here is my code (I think I have managed to anonymise it) Quote <!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> <title>Shop</title> <meta content="en-gb" http-equiv="Content-Language" /> <meta content="text/html; charset=utf-8" http-equiv="Content-Type" /> </head> <body> <!--#include virtual="/includes/header.html" --> <!--#include virtual="/includes/navigation.html" --> <table width="1000px" align="center" border="1"> <tr> <td class="style_shop_text" colspan="3"> <a href="/shop/shop_prod1_menu_1.shtml"> <img alt="Product 1" src="/images/site/shop/prod1.jpg" </a> </td> </tr> <div class="w3-content" style="max-width:1000px"> <tr> <td style="width: 333px" valign="top"> <P class="style_shop_text"></br> Product - £11</br></br> Dimensions : 5cm (approx)</br></br> BLAH BLAH BLAH</br> </P> </td> <td valign="top" align="center" valign="top" colspan="2"> <img class="mySlides" src="/images/work/prod 1-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 1-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 1-3.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-3.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-3.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 4-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 4-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 4-3.jpg" width="400" height="400" alt="Product 1"> </td> </tr> <tr> <td> <form target="paypal" action="https://www.paypal.com/cgi-bin/webscr" method="post"> <input type="hidden" name="cmd" value="_s-xclick"> <input type="hidden" name="hosted_button_id" value="xxxxxxxxxxx"> <table> <tr> <td> <input type="hidden" name="on0" value="Product_combo"> </td> </tr> <tr> <td> <select name="os0" id="Product_combo"> <option value="Product 1-1" data-img-index="0" data-src1="/images/work/prod 1-1.jpg" data-src2="/images/work/prod 1-2.jpg" data-src3="/images/work/prod 1-3.jpg">Product 1 £11.00 GBP</option> <option value="Product 1-2" data-img-index="3" data-src1="/images/work/prod 2-1.jpg" data-src2="/images/work/prod 2-2.jpg" data-src3="/images/work/prod 2-3.jpg">Product 2 £11.00 GBP</option> <option value="Product 1-3" data-img-index="6" data-src1="/images/work/prod 3-1.jpg" data-src2="/images/work/prod 3-2.jpg" data-src3="/images/work/prod 3-3.jpg">Product 3 £11.00 GBP</option> <option value="Product 1-4" data-img-index="9" data-src1="/images/work/prod 4-1.jpg" data-src2="/images/work/prod 4-2.jpg" data-src3="/images/work/prod 4-3.jpg">Product 4 £11.00 GBP</option> </select> </td> </tr> </table> <input type="hidden" name="currency_code" value="GBP"> <input type="image" src="http://www.mywebsite.co.uk/images/site/button_add_to.jpg" border="0" name="submit" alt="PayPal – The safer, easier way to pay online."> <img alt="" border="0" src="https://www.paypalobjects.com/en_GB/i/scr/pixel.gif" width="1" height="1"> </form> <form target="paypal" action="https://www.paypal.com/cgi-bin/webscr" method="post" > <input type="hidden" name="cmd" value="_cart"> <input type="hidden" name="business" value="xxxxxxxxxxxxxx"> <input type="hidden" name="display" value="1"> <input type="image" src="http://www.mywebsite.co.uk/images/site/button_view_cart.jpg" border="0" name="submit" alt="View Shopping basket"> <img alt="" border="0" src="https://www.paypalobjects.com/en_GB/i/scr/pixel.gif" width="1" height="1"> </form> </td> <div class="w3-row-padding w3-section"> <td colspan="2" valign="top"> <table width="100%" border="0" valign="top"> <tr> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_1" height="200" src="/images/work/product 1-1.jpg" width="200" alt="Product" onclick="currentDiv(1)"> </td> </div> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_2" height="200" src="/images/work/product 1-2.jpg" width="200" alt="Product" onclick="currentDiv(2)"> </td> </div> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_3" height="200" src="/images/work/product 1-3.jpg" width="200" alt="Product" onclick="currentDiv(3)"> </td> </div> </tr> </table> </td> </div> <tr> <td colspan="3"> </BR> </td> </tr> </div> </table> <script> $("#Product_combo").on('change', function(){ document.getElementById("prod_menu_image_1").src = $(this).find(":selected").attr("data-src1"); document.getElementById("prod_menu_image_2").src = $(this).find(":selected").attr("data-src2"); document.getElementById("prod_menu_image_3").src = $(this).find(":selected").attr("data-src3"); imageindex1=parseFloat($(this).find(":selected").attr("data-img-index")) +1; imageindex2=parseFloat($(this).find(":selected").attr("data-img-index")) +2; imageindex3=parseFloat($(this).find(":selected").attr("data-img-index")) +3; $("#prod_menu_image_1").on("click", currentDiv(imageindex1)); $("#prod_menu_image_1").on("click", currentDiv(imageindex2)); $("#prod_menu_image_1").on("click", currentDiv(imageindex3)); }); var slideIndex = 1; showDivs(slideIndex); function plusDivs(n) { showDivs(slideIndex += n); } function currentDiv(n) { showDivs(slideIndex = n); } function showDivs(n) { var i; var x = document.getElementsByClassName("mySlides"); var dots = document.getElementsByClassName("product_slides"); if (n > x.length) {slideIndex = 1} if (n < 1) {slideIndex = x.length} for (i = 0; i < x.length; i++) { x.style.display = "none"; } for (i = 0; i < dots.length; i++) { dots.className = dots.className.replace("w3-opacity-off", ""); } x[slideIndex-1].style.display = "block"; dots[slideIndex-1].className += " w3-opacity-off";}</script> <!--#include virtual="/includes/footer.html" --> </body> </html> Edited January 10, 2018 by taggs74 Link to comment Share on other sites More sharing options...
dsonesuk Posted January 10, 2018 Share Posted January 10, 2018 WOW! that's got to be the most work intensive, mind boggling way of doing this. Since it is constant 1-2-3 can't this be done in a for loop, without custom data for versions 1, 2, 3.. The 'data-src' could hold a array of values, which the loop can retrieve. Link to comment Share on other sites More sharing options...
taggs74 Posted January 10, 2018 Author Share Posted January 10, 2018 I'm still struggling with the line $("#prod_menu_image_1").on("click", currentDiv(imageindex1)); to get the element on click to change...... any help is much appricated... Link to comment Share on other sites More sharing options...
justsomeguy Posted January 10, 2018 Share Posted January 10, 2018 You're still doing the same thing you started with. You are not passing the function as an event handler, you are just executing it immediately when that code runs. Any time you put parentheses after a function name you are telling it to execute that function immediately. If you don't want to do that, do not put parentheses after the function name. $("#prod_menu_image_1").on("click", function() { currentDiv(imageindex1) }); $("#prod_menu_image_1").on("click", {index: imageindex1}, currentDiv); Those do slightly different things. The first one uses a closure, and every time the click event runs it executes currentDiv(imageindex1) with whatever the current value of that variable is, assuming it is available globally. The second one uses jQuery's on function to specify that an object should be available inside the click handler with a property called index that will have the value of imageindex1 when that click handler was added. That data is available on the event object according to the jQuery documentation. 1 Link to comment Share on other sites More sharing options...
taggs74 Posted January 10, 2018 Author Share Posted January 10, 2018 Dude, you are a hero.....works perfectly... as you can tell I'm not a programmer and certainly need to increase my knowledge....but "playing" certainly helps... thanks so much justsomeguy Link to comment Share on other sites More sharing options...
dsonesuk Posted January 11, 2018 Share Posted January 11, 2018 Here's an example of letting JavaScript do the work for you, once you give it required minimum data. NOTE: its not a good idea to have spacing in image file names, it tends to cause problems in coding, use underscore or hyphens instead. <!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <meta name="viewport" id="viewport" content="target-densitydpi=high-dpi,initial-scale=1.0" /> <title>Document Title</title> <script type="text/javascript" src="//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script> <script type="text/javascript" src="//ajax.googleapis.com/ajax/libs/jqueryui/1.10.4/jquery-ui.min.js"></script> <link rel="stylesheet" href="https://www.w3schools.com/w3css/4/w3.css"> <style> .mySlides {display:none} .demo {cursor:pointer} </style> </head> <body> <table width="1000px" align="center" border="1"> <tr> <td class="style_shop_text" colspan="3"> <a href="/shop/shop_prod1_menu_1.shtml"> <img alt="Product 1" src="/images/site/shop/prod1.jpg"> </a> </td> </tr> <div class="w3-content" style="max-width:1000px"> <tr> <td style="width: 333px" valign="top"> <P class="style_shop_text"></br> Product - £11</br></br> Dimensions : 5cm (approx)</br></br> BLAH BLAH BLAH</br> </P> </td> <td valign="top" align="center" valign="top" colspan="2"> <img class="mySlides" src="/images/work/prod 1-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 1-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 1-3.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 2-3.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 3-3.jpg" width="400" height="400" alt="Product 1"> <!-- <img class="mySlides" src="/images/work/prod 4-1.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 4-2.jpg" width="400" height="400" alt="Product 1"> <img class="mySlides" src="/images/work/prod 4-3.jpg" width="400" height="400" alt="Product 1">--> </td> </tr> <tr> <td> <form target="paypal" action="https://www.paypal.com/cgi-bin/webscr" method="post"> <input type="hidden" name="cmd" value="_s-xclick"> <input type="hidden" name="hosted_button_id" value="xxxxxxxxxxx"> <table> <tr> <td> <input type="hidden" name="on0" value="Product_combo"> </td> </tr> <tr> <td> <select name="os0" id="Product_combo"> <option value="Product 1-1" data-src='["prod 1-1", "prod 1-2", "prod 1-3"]'>Product 1 £11.00 GBP</option> <option value="Product 1-2" data-src='["prod 2-1", "prod 2-2", "prod 2-3"]'>Product 2 £11.00 GBP</option> <option value="Product 1-3" data-src='["prod 3-1", "prod 3-2", "prod 3-3"]'>Product 3 £11.00 GBP</option> <!-- <option value="Product 1-4" data-img-index="9" data-src1="/images/work/prod 4-1.jpg" data-src2="/images/work/prod 4-2.jpg" data-src3="/images/work/prod 4-3.jpg">Product 4 £11.00 GBP</option>--> </select> </td> </tr> </table> <input type="hidden" name="currency_code" value="GBP"> <!-- <input type="image" src="http://www.mywebsite.co.uk/images/site/button_add_to.jpg" border="0" name="submit" alt="PayPal – The safer, easier way to pay online."> <img alt="" border="0" src="https://www.paypalobjects.com/en_GB/i/scr/pixel.gif" width="1" height="1">--> </form> <form target="paypal" action="https://www.paypal.com/cgi-bin/webscr" method="post" > <input type="hidden" name="cmd" value="_cart"> <input type="hidden" name="business" value="xxxxxxxxxxxxxx"> <input type="hidden" name="display" value="1"> <!-- <input type="image" src="http://www.mywebsite.co.uk/images/site/button_view_cart.jpg" border="0" name="submit" alt="View Shopping basket"> <img alt="" border="0" src="https://www.paypalobjects.com/en_GB/i/scr/pixel.gif" width="1" height="1">--> </form> </td> <div class="w3-row-padding w3-section"> <td colspan="2" valign="top"> <table width="100%" border="0" valign="top" id="imageTable"> <tr> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_1" height="200" src="/images/work/prod 1-1.jpg" width="200" alt="Product" onclick="currentDiv(1)"> </td> </div> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_2" height="200" src="/images/work/prod 1-2.jpg" width="200" alt="Product" onclick="currentDiv(2)"> </td> </div> <div class="w3-col s4" valign="top"> <td style="width:33%" align="center"> <img class="w3-opacity w3-hover-opacity-off" id="prod_menu_image_3" height="200" src="/images/work/prod 1-3.jpg" width="200" alt="Product" onclick="currentDiv(3)"> </td> </div> </tr> </table> </td> </div> <tr> <td colspan="3"> </BR> </td> </tr> </div> </table> <script> $("#Product_combo").on('change', function() { parentTable = document.getElementById("imageTable"); imgTag = parentTable.getElementsByTagName('img'); dataSrcArray = JSON.parse($(this).find(":selected").attr("data-src")); $.each(imgTag, function(key) { $(this).attr('src', "/images/work/" + dataSrcArray[key] + ".jpg"); }); var myslideElem = document.getElementsByClassName("mySlides"); $.each(myslideElem, function(indexkey) { $(this).css({"display": "none"}); myslideThis = $(this); $.each(imgTag, function() { if (myslideThis.attr('src') === decodeURIComponent($(this).attr('src'))) { $(this).attr("title", indexkey + 1); $(this).attr("onclick", "currentDiv(" + (indexkey + 1) + ")"); } if (decodeURIComponent(imgTag[0].src).indexOf(myslideThis.attr('src')) !== -1) { //imgTag[0].attr('src', myslideThis.src); myslideThis.css({"display": "block"}); } }); }); }); function plusDivs(n) { showDivs(slideIndex += n); } function currentDiv(n) { showDivs(slideIndex = n); } var slideIndex = 1; showDivs(slideIndex); function showDivs(n) { var i; var x = document.getElementsByClassName("mySlides"); var dots = document.getElementsByClassName("product_slides"); if (n > x.length) { slideIndex = 1; } if (n < 1) { slideIndex = x.length; } for (i = 0; i < x.length; i++) { x[i].style.display = "none"; } for (i = 0; i < dots.length; i++) { dots.className = dots.className.replace("w3-opacity-off", ""); } x[slideIndex - 1].style.display = "block"; dots[slideIndex - 1].className += " w3-opacity-off"; } </script> </body> </html> Now could add extra custom data for image path, or add full image file name and path, (but it looks cleaner without the path) to data-src, up to you. 1 Link to comment Share on other sites More sharing options...
taggs74 Posted January 11, 2018 Author Share Posted January 11, 2018 Thanks so much dsonesuk. Your Code looks alot cleaner than mine and also could well make it easier for me to add extra data, if needed, later on. spaces in file names noted. Cheers dude! 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