Why don't work this code? change criteria

Discussion in 'JavaScript' started by piropeator, Apr 29, 2009.

  1. #1
    I want to find one of two criteria: Code or Name.
    If I check in Code, only enter numeric value, clear input text and run Option1.php.
    If I check in Name, enter alphanumeric value, clear input text and run Option2.php.


    <html>
    <head>
    <SCRIPT LANGUAGE="JavaScript">
    function redireccionar(valor)
    {
    if (window.document.formulario.opciones.checked==true)
    {document.formulario.action="./OPTION1.PHP" ;}
    else
    {document.formulario.action="./OPTION2.PHP" ;}
    document.formulario.submit();
    }
    </SCRIPT>
    </head>
    <body>
    <form name="formulario" method="post">
    <input type="radio" name="opciones" value="1" checked="True">CODE:
    <input type="radio" name="opciones" value="2">NAME:
    <input type="text" name="Texto" onMouseOver="(window.document.formulario.Texto.focus());">
    <input type="button" name="Boton" Value="Search" onClick="redireccionar(window.document.formulario.opciones.value);">
    </form>
    </body>
    </html>

    Why don't work? :eek:
     
    piropeator, Apr 29, 2009 IP
  2. dimitar christoff

    dimitar christoff Active Member

    Messages:
    882
    Likes Received:
    62
    Best Answers:
    0
    Trophy Points:
    90
    #2
    first off...

    redireccionar(window.document.formulario.opciones.value); <-- you pass a value... so that's to be used (1 or 2)? Yet, within the function redireccionar you don't really care what the value is, you go and look at window.document.formulario.opciones.checked instead...

    now, a few ideas:
    - dependent on your doctype, checked="true" is deprecated, the correct value _SHOULD_ be checked="checked"... firefox, for instance, will return checked="checked" (which can also evaluate as true because it's set and not null). you may get into problems over this in different browsers...
    - window.document.formulario.element etc is not a great way to access elements, you really want to steer away from this kind of coding and start using id="" and document.getElementById instead where you can....
    - NEVER use inline js like onclick= and onmouseover= on elements. its a BAD practice and can create circular references, which cause memory leaks etc.
    - when you say 'don't work' you really ought to say what does not work, post some errors and so forth! to help you debug, run firefox and install the firebug plugin, press f12, enable console and look at all the errors. you can also use console.log() to output stuff, like:

    if (blah) { console.log("starting loop 1..."); } etc etc

    question: as it stands, which page does it go to, option1, option2, refreshes the page that has the form or does nothing at all?

    the problem with the above code is that "opciones" really is an array of items, only one of which has the selected="selected" attribute. you cannot get the 'value' of an array - which is what you are trying to do - well, not unless you prototype the radio element anyway...

    edit: here's what 5 mins of refactoring can do:
    <html>
    <head>
    <script language="javascript">
    var redireccionar = function(valor) {
        // need to find what is checked first and use its value in the redirect...
        var selected = '', len = valor.length; // we only have 2 but this will support nn number of radio options... 
        while(len--) {
            if (valor[len].checked)
                selected = valor[len].value;
        }
    
        // use the selected value= and append to option .php.
        document.getElementById("formulario").setAttribute("action", "./OPTION"+selected+".PHP");
        document.getElementById("formulario").submit();
    };
    
    window.onload = function() {
        // wait for page load and assign click and mouseover events that affect DOM
        document.getElementById("button").onclick = function() {
            // button click gets me the options and sends me to my redirect function
            redireccionar(document.formulario.opciones);
        };
    
        document.getElementById("texto").onmouseover = function() {
            // a simple mouseover focus. 
            this.focus();
        };
    };
    
    </script>
    </head>
    <body>
        <form name="formulario" id="formulario" method="post">
        <input type="radio" name="opciones" value="1" checked="checked">CODE:
        <input type="radio" name="opciones" value="2">NAME:
        <input type="text" name="Texto" id="texto">
        <input type="button" name="Boton" id="button" value="Search">
    </form>
    </body>
    </html>
    
    PHP:
    this now works properly in firefox 3 and IE7. notice the use of id=" and references.

    I hope this helps you and sets you on the right path for proper and modern javascript programming :)
     
    dimitar christoff, Apr 29, 2009 IP
  3. piropeator

    piropeator Well-Known Member

    Messages:
    194
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    121
    #3
    Thanks but, I use IE7 and does not work.
    :confused:
     
    piropeator, Apr 30, 2009 IP
  4. dimitar christoff

    dimitar christoff Active Member

    Messages:
    882
    Likes Received:
    62
    Best Answers:
    0
    Trophy Points:
    90
    #4
    dimitar christoff, May 1, 2009 IP
  5. camjohnson95

    camjohnson95 Active Member

    Messages:
    737
    Likes Received:
    17
    Best Answers:
    0
    Trophy Points:
    60
    #5
    Hi, I didn't realise it was considered bad practice to use inline JS, what I would like to know is if it is necessary for each event e.g ..button..onclick = function () { , to be within the window.onload event?
    e.g: would..
    
    document.getElementById("button").onclick = function() {        // button click gets me the options and sends me to my redirect function        redireccionar(document.formulario.opciones);    };
    
    Code (markup):
    work if it wasn't within the window.onload function? And if so why is it necessary that it is?
     
    camjohnson95, May 1, 2009 IP
  6. dimitar christoff

    dimitar christoff Active Member

    Messages:
    882
    Likes Received:
    62
    Best Answers:
    0
    Trophy Points:
    90
    #6
    first of all, look at it from the point of view, you are to build semantic sites with js, css and html clearly separated wherever possible. this means, no inline css, no inline js.

    whereas inline css is just messy (imo, evangelists may claim otherwise) and can have some adverse effects on seo, inline event binding in js can also have memory leaks implications (circular references which can’t be handled by the garbage collection of most browsers). things like onclick="blah();return false;" etc are to be avoided at all costs (check posts at bottom).

    consider maintenance /scalability as well, if you change a function in an include and some selectors vs changing every single line that may call this function...

    accessibility... visibility of your code (a href="javascript:blah" is just really really shite)...

    anyway, if we accept this as the best practice and the correct thing to do, then why do it onload / domready / document.ready etc (whatever your framework calls it)?

    the reason is: because you can only refer to elements that are already in the dom.

    in other words, if you do:
    
    <script>
    alert(document.getElementById("foo").style.display);
    </script>
    
    ...
    <div id="foo" style="display: none"></div>
    
    PHP:
    this won't work because at the runtime of the js, foo has not been rendered and added to the dom elements that can be manipulated yet.

    a possible workaround is to have the script at the bottom of the page right before you close your body/html tags.

    some interesting topics on the subject (i am choosing ones outside of the mootools community so that I cannot be accused of bias)
    http://www.smashingmagazine.com/2008/09/16/jquery-examples-and-best-practices/

    http://stackoverflow.com/questions/...t-document-ready-function-rather-than-onclick

    i have been meaning to do a recap of the basic best practices in modern javascript for a while but there simply isn't enough time :)
     
    dimitar christoff, May 1, 2009 IP
    camjohnson95 and Unni krishnan like this.
  7. piropeator

    piropeator Well-Known Member

    Messages:
    194
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    121
    #7
    Excellent !!

    But, I have one more question.
    If I want to choose CODE run Code.php
    and I want to choose NAME run Name.php

    How do I do in this case? :rolleyes:

    Thanks.
     
    piropeator, May 3, 2009 IP
  8. dimitar christoff

    dimitar christoff Active Member

    Messages:
    882
    Likes Received:
    62
    Best Answers:
    0
    Trophy Points:
    90
    #8
    ERM.

    use the value of the radio as storage:
    <html>
    <head>
    <script language="javascript">
    var redireccionar = function(valor) {
        // need to find what is checked first and use its value in the redirect...
        var selected = '', len = valor.length; // we only have 2 but this will support nn number of radio options... 
        while(len--) {
            if (valor[len].checked)
                selected = valor[len].value;
        }
    
        // use the selected value= and redirect to it
        document.getElementById("formulario").setAttribute("action", selected); // <-- changed
        document.getElementById("formulario").submit();
    };
    
    window.onload = function() {
        // wait for page load and assign click and mouseover events that affect DOM
        document.getElementById("button").onclick = function() {
            // button click gets me the options and sends me to my redirect function
            redireccionar(document.formulario.opciones);
        };
    
        document.getElementById("texto").onmouseover = function() {
            // a simple mouseover focus. 
            this.focus();
        };
    };
    
    </script>
    </head>
    <body>
        <form name="formulario" id="formulario" method="post">
        <input type="radio" name="opciones" value="code.php" checked="checked">CODE: <!--changed value= -->
        <input type="radio" name="opciones" value="name.php">NAME: <!--changed value= -->
        <input type="text" name="Texto" id="texto">
        <input type="button" name="Boton" id="button" value="Search">
    </form>
    </body>
    </html>
    PHP:
     
    dimitar christoff, May 3, 2009 IP
  9. Unni krishnan

    Unni krishnan Peon

    Messages:
    237
    Likes Received:
    9
    Best Answers:
    2
    Trophy Points:
    0
    #9
    Thank You Dimitar for the info about the disadvantages of inline js.
    I use inline js in my works.
    Will try to implement the event-based style of scripting you suggest.

    Just one doubt..

    Will the addition of a global handler like 'window.onload' will add overhead to the program?
    Once again thanking for the info.


    Unni Krishnan
    TechnoTwins Design and Solutions.
     
    Unni krishnan, May 5, 2009 IP
  10. piropeator

    piropeator Well-Known Member

    Messages:
    194
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    121
    #10
    Why does not work to me? :'(

    and one thing more: When I change radio button does not erase the value in the input text.

    I think this is necesary.
     
    piropeator, May 7, 2009 IP
  11. piropeator

    piropeator Well-Known Member

    Messages:
    194
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    121
    #11
    Hey guys, I make it work. Changed the value of radio button.
    Thank you.
     
    piropeator, May 8, 2009 IP