pleek Posted June 24, 2009 Share Posted June 24, 2009 HTML <SELECT NAME="BG" ID="BGChooser" onchange="changeimg()"> <OPTION VALUE="0">Choose a Background... <OPTION VALUE="0">None <OPTION VALUE="1">Little Big Planet <OPTION>Other Background </SELECT> Javascript function changeimg(){ document.getElementById(\'Card\').src=\'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php?UserName=' .$UserName. '&Posts=' .$UserPosts. '&PS=' .$PersonalText. '&Site=' .$Website. '&Av=' .$UserAvatar. '&Group=' .$Group. '&BG=\' + document.getElementById(\'BGChooser\').value; } The code above doesn't work but when i replace "document.getElementById(\'BGChooser\').value" with "1" it works fine. How am i supposed to do this so when the user changes the dropdown menu it gets the value and adds it to the url in the function? Quote Link to comment Share on other sites More sharing options...
KevinM1 Posted June 24, 2009 Share Posted June 24, 2009 Your event handler is already tied to a particular element, so you don't need to reacquire it. Try changing it from document.getElementById(/* your element id */).value to simply this.value. Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 24, 2009 Share Posted June 24, 2009 Well, I would first suggest rewriting the code so it is more readable. In my opinion a line of code should not be so long that you can't immediately tell what it is doing. Second, that code is "out of context". It is apparent that it is being written within PHP, yet is presented outside the actual PHP code that is writing it. So, it is difficult to determine exactly what the problem may be. Wither post the code showing how PHP is writing it OR just post the code that is rendered in the HTML. I think the problem has something to do with the escaping of quote marks. When writing JavaScript within PHP trying to track the correct escaping between the two can get confusing making it easy to make mistakes. If I need to insert variable content within JavaScript using PHP I typically write the JavaScript as plain HTML in the page (i.e. outside PHP tags) and just insert the variable content where needed. I would also change the function to accept a parameter for the select object rather than hard coding the reference in the function. Makes your code much more reusable. HTML (notice the (this) and ('Card') in the onchange call) <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, 'Card');"> <OPTION VALUE="0">Choose a Background... <OPTION VALUE="0">None <OPTION VALUE="1">Little Big Planet <OPTION>Other Background </SELECT> Server-side script function changeimg(selObj, targetID) { //Generate the background source URL var newSrc; newSrc += 'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php'; newSrc += '?UserName=<?php echo $UserName; ?>'; newSrc += '&Posts=<?php echo $UserPosts; ?>'; newSrc += '&PS=<?php echo $PersonalText; ?>'; newSrc += '&Site=<?php echo $Website; ?>'; newSrc += '&Av=' .$UserAvatar newSrc += '&Group=' .$Group newSrc += '&BG=' + selObj.options[selObj.selectedIndex].value; //Set the target source document.getElementById(targetID).src = newSrc; return; } EDIT: Added the targeteID reference for the function Also, I notice teh select field has an 'other' option with no value. The function currently wouldn't handle that. I assume you are aware and just haven't added that functionality yet. Quote Link to comment Share on other sites More sharing options...
pleek Posted June 24, 2009 Author Share Posted June 24, 2009 i tried both of the posted methods and i couldn't get either to work. Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 24, 2009 Share Posted June 24, 2009 "It didn't work" is not helpful. I did notice that I forgot to finish formatting the last two lines that generate the newSrc value. Also, I found that trying to do a += to a variable that has not yet been assigned a value results in "undefined" being added to the beginning. I have made the corrections below. However, you need to realize that if ANY of the values you are using have values that are not appropriate for a URL (e.g. spaces and some special characters) it will also fail. So, I also added rawurlencode() to each of the variables being written. The below code is a complete page that shows the code working. Instead of actually changing the src of the target object I am printing the result tothe page so you can see what is happening. Just uncomment the line before and remove the line that prints the code to the page for actual implementation <?php $UserName = "MJones"; $UserPosts = "25"; $PersonalText = "TheText"; $Website = "www.domain.com"; $UserAvatar = "MyAvatar"; $Group = "TheGroup"; ?> <html> <head> <script type="text/javascript"> function changeimg(selObj, targetID) { //Generate the background source URL var newSrc; newSrc = 'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php'; newSrc += '?UserName=<?php echo rawurlencode($UserName); ?>'; newSrc += '&Posts=<?php echo rawurlencode($UserPosts); ?>'; newSrc += '&PS=<?php echo rawurlencode($PersonalText); ?>'; newSrc += '&Site=<?php echo rawurlencode($Website); ?>'; newSrc += '&Av=<?php echo rawurlencode($UserAvatar); ?>' newSrc += '&Group=<?php echo rawurlencode($Group); ?>' newSrc += '&BG=' + selObj.options[selObj.selectedIndex].value; //Set the target source //document.getElementById(targetID).src = newSrc; document.getElementById(targetID).innerHTML = newSrc; return; } </script> </head> <body> <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, 'Card');"> <OPTION VALUE="0">Choose a Background...</OPTION> <OPTION VALUE="0">None</OPTION> <OPTION VALUE="1">Little Big Planet</OPTION> <OPTION>Other Background</OPTION> </SELECT> <DIV id="Card"></DIV> </body> </html> Quote Link to comment Share on other sites More sharing options...
pleek Posted June 24, 2009 Author Share Posted June 24, 2009 well by "it didn't work right" i meant it didn't do anything, worked as good as the nonworking code i posted. I'll try your updated script and get back to you with the results. Thanks for the help. Quote Link to comment Share on other sites More sharing options...
pleek Posted June 25, 2009 Author Share Posted June 25, 2009 ok, this code (your code) works great by itself... <?php $UserName = "MJones"; $UserPosts = "25"; $PersonalText = "TheText"; $Website = "www.domain.com"; $UserAvatar = "MyAvatar"; $Group = "TheGroup"; echo' <html> <head> <script type="text/javascript"> function changeimg(selObj, targetID) { //Generate the background source URL var newSrc; newSrc = \'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php\'; newSrc += \'?UserName=' .$UserName. '\'; newSrc += \'&Posts=' .$UserPosts. '\'; newSrc += \'&PS=' .$PersonalText. '\'; newSrc += \'&Site=' .$Website. '\'; newSrc += \'&Av=' .$UserAvatar. '\' newSrc += \'&Group=' .$Group. '\' newSrc += \'&BG=\' + selObj.options[selObj.selectedIndex].value; //Set the target source //document.getElementById(targetID).src = newSrc; document.getElementById(targetID).innerHTML = newSrc; return; } </script> </head> <body> <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, \'Card\');"> <OPTION VALUE="0">Choose a Background...</OPTION> <OPTION VALUE="0">None</OPTION> <OPTION VALUE="1">Little Big Planet</OPTION> <OPTION>Other Background</OPTION> </SELECT> <DIV id="Card"></DIV> </body> </html>'; but when i insert it into my page, when i click on "Card Two" it doesn't change the "Card" src and it doesn't change the innerhtml of "Options" either. Here's the html.... (Yes its enclosed in php) <select name="CardSelection" multiple size="5"> <option onclick="Change1()">Card One <option onclick="Change2()">Card Two </select> <td ID="Options" valign="top"> <hr> <BR> <DIV ALIGN="center"> <img ID="Card" src="http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardOne/index.php? UserName=' .ucwords($UserName). '&Posts=' .$UserPosts. '&PS=' .$PersonalText. '&Site=' .$Website. '&Av=' .$UserAvatar. '&Group=' .$Group. '"> </DIV> And here is the javascript... <SCRIPT TYPE="text/javascript"> function Change2() { document.getElementById(\'Card\').src=\'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php?UserName=' .$UserName. '&Posts=' .$UserPosts. '&PS=' .$PersonalText. '&Site=' .$Website. '&Av=' .$UserAvatar. '&Group=' .$Group. '\'; document.getElementById(\'Options\').innerHTML=\' <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, \'Card\');"> <OPTION VALUE="0">Choose a Background...</OPTION> <OPTION VALUE="0">None</OPTION> <OPTION VALUE="1">Little Big Planet</OPTION> <OPTION>Other Background</OPTION> </SELECT>\'; } function Change1() { document.getElementById(\'Card\').src=\'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardOne/index.php?UserName=' .$UserName. '&Posts=' .$UserPosts. '&PS=' .$PersonalText. '&Site=' .$Website. '&Av=' .$UserAvatar. '&Group=' .$Group. '\'; document.getElementById(\'Options\').innerHTML=\'\'; } function changeimg(selObj, targetID) { //Generate the background source URL var newSrc; newSrc = \'http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php\'; newSrc += \'?UserName=' .$UserName. '\'; newSrc += \'&Posts=' .$UserPosts. '\'; newSrc += \'&PS=' .$PersonalText. '\'; newSrc += \'&Site=' .$Website. '\'; newSrc += \'&Av=' .$UserAvatar. '\' newSrc += \'&Group=' .$Group. '\' newSrc += \'&BG=\' + selObj.options[selObj.selectedIndex].value; //Set the target source document.getElementById(targetID).src = newSrc; return; } </SCRIPT> Now javascript:change1() and javascript:change2() both worked before adding your code to the innerHTML. So im not sure what part of it is stopping it from working. See any errors or problems? Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 25, 2009 Share Posted June 25, 2009 I already told you what I thought the problem was before - a mismatch of escaped quotes between PHP and JavaScript. And I stated that was the reason why I recommend that you do not write JavaScript from within PHP and gave a perfectly acceptable method of making the javascript variable using PHP without the problems of escaping the quotes. Then you go and take the code I provided and reverse engineer it back to how you were trying to do it before and you have created the same problems as before. Now the code is much more difficult to read than what I provided. Plus, you didn't use the rawurlencode() which would fix the other possible problem of invalid characters int he URL. I wish you all the luck, but if you don't want to bastardize the solution I provided and it doesn't work, I'll just recommend you to use it as provided. Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 25, 2009 Share Posted June 25, 2009 I just noticed your other two functions Change1() and Change2(). Using javascript to insert a select list via innerHTML is probably not the best choice. I would recommend just changing the style to hide/unhide the select list. Quote Link to comment Share on other sites More sharing options...
pleek Posted June 25, 2009 Author Share Posted June 25, 2009 my syntac for the javascript is fine, it works as long as i don't change the onchange() handler to onchange(this, 'Card'). As soon as i do that it doesn't work. And as i said your code works perfectly, and the way i recoded it is actually the correct way of inserting php variables into html, at least thats how i've been tought. addint <?PHP tags to every part of the php code is very time consuming and if your code is mostly php you would spend hours just writing that code. As for the rawurlencode() i don't disagree with it and i will probably add it back in later, but for the sake of simplicity i removed it. Just because your personal syntac style doesn't match mine doesn't make either of our syntac wrong . As for your suggestion to change the Change 1 & 2 functions to hide/show i agree and thanks for pointing that out. innerhtml was just the first thing i thought of to do what i needed it to. I will try that and i think that might solve my problem. I thank you for your help, it is very appreciated! Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 25, 2009 Share Posted June 25, 2009 Well, let's just agree that we will disagree then. You are right in that there is nothing inherently wrong with your approach. My problem with writing the javascript using PHP leads to problem with following quote marks and ensuring they are escaped (and in some cases double escaped) as needed. I don't see how my apprach leads to any more work. The last code I posted gave an example of how the Javascript can be created outside the PHP tags and just include the variable content as needed. In my opinion it is much easier to control. But, it's your code. However, if you are going to write the javascript using PHP echo's, why are you using single quote marks to echo the code and then having to exit out of quote marks to append the variables. Just use double quotes and include the variable inside the quote marks. Much more efficient and easier to read I think //Single quotes echo 'This car is ' . $color . ' and gets ' . $mpg; //Double quotes - easier to read echo "This car is $color and gets $mpg"; Okay, after taking another look at the code I do see the error. And it is a combination of two of the problems I identified - primarily an issue with the escaping of quot marks and also the fact that you are writing a select field using innerHTML! If you had taken my approach the problem would have been self evident. Or at the very least, you should have been looking at the HTMNL source of the processed page. The rendered Javascript (based on the code above) will look something like this (put in quote tags so I can highlight the specific problem): <SCRIPT TYPE="text/javascript"> function Change2() { document.getElementById('Card').src='http://craigh.tlcrepair.net/ConsoleCard/ConsoleCardTwo/index.php?UserName=UserName&Posts=UserPosts&PS=PersonalText&Site=Website&Av=UserAvatar&Group=Group'; document.getElementById('Options').innerHTML= ' <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, 'Card');"> <OPTION VALUE="0">Choose a Background...</OPTION> <OPTION VALUE="0">None</OPTION> <OPTION VALUE="1">Little Big Planet</OPTION> <OPTION>Other Background</OPTION> </SELECT>'; } In the resulting JavaScript you start declaring the code to write into the DIV using innerHTML using a single quote (blue). But, within that code you are creating a parameter that is also declared with single quotes (red). So, when the javascript gets to the first red quote it thinks that is the end of the content to assign to the innerHTML value. If you want to keep the code as you currently have it you will need to "double" escape those red quote marks. This gets very tricky and gets error prone which is why I suggest you don't do it that way. To "double" escape it, that line would look something like this in the PHP file: <SELECT NAME="BG" ID="BGChooser" onchange="changeimg(this, \\\'Card\\\');"> Although I don't know why you want to make it hard on yourself. Just declare the Javascript outside the PHP tags and it is so much easier to create and maintain. Quote Link to comment Share on other sites More sharing options...
pleek Posted June 26, 2009 Author Share Posted June 26, 2009 Well, let's just agree that we will disagree then... However, if you are going to write the javascript using PHP echo's, why are you using single quote marks to echo the code and then having to exit out of quote marks to append the variables. Just use double quotes and include the variable inside the quote marks. Much more efficient and easier to read I think //Single quotes echo 'This car is ' . $color . ' and gets ' . $mpg; //Double quotes - easier to read echo "This car is $color and gets $mpg"; thank you for pointing that out, i had forgotten that double quotes work that way which makes the javascript part much easier. I was using single quotes because of all the html, which i didn't want to escape all the double quotes in my html code. Also your idea to use javascript visibility to show my html help me so much. Now my code is 100% working! I really appreciate your time and explaining things. People like you is why this forum is the best one out there like it. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.