views:

67

answers:

3

The following code works just fine in IE, FF, Chrome and Safari, but I am wondering how efficient it actually is when scaled to a lot of users and whether it is accessible to screen readers/special needs users/etc? Specifically, is there a better way to write the jQuery so it uses less $(this) references, and does that even cause any performance hit to use? Also, would setting the display to none on the labels limit the accessibility to people using screen readers? I add in the title attribute when the page is loaded for this purpose, but does that even help/is it a good idea? Also, I am aware that if javascript is disabled all this would be useless, but just wondering here.

<html>
    <head>
     <title></title>
     <style type="text/css">
      fieldset {
       border: none;
       padding: 10px;
       width: 600px;
       margin-top: 5px;
      }

      label { 
       display: none;
      }

      input {
       width: 150px;
       margin: 5px;
       float: left;
       display: block;
      }

      br {
       clear: both;
      }
     </style>
     <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.3.2/jquery.min.js"&gt;&lt;/script&gt;
     <script type="text/javascript">
      $(document).ready(function() {
       GetInputLabels();
       $("input").focus(function() {
        if ($(this).val() == $(this).prev("label").text()) {
         $(this).val('').css("color", "#000");
        }     
       });
       $("input").blur(function() {
        if ($(this).val() == '') {
         $(this).val($(this).prev("label").text()).css("color", "#bbb");
        } 
       });
       function GetInputLabels() {
        $("input").each(function() {
         var label = $(this).prev("label").text();
         $(this).val(label).css("color", "#bbb").attr("title", label);
        });
       }
      });
     </script>
    </head>
    <body>
     <form>
      <fieldset>
       <label for="fname">First Name</label>
       <input id="fname" type="text" /> <br/>

       <label for="lname">Last Name</label>
       <input id="lname" type="text" /> <br/>

       <label for="email">Email</label>
       <input id="email" type="text" /> <br/>
      </fieldset>
     </form>
    </body>
</html>
A: 

One simple thing would be to set var element = $(this); in the beginning of your anonymous functions and use this variable. This will avoid to do the lookup several times.

As for the screen readers, setting labels to visibilty: hidden (or display: none) will prevent them from being read aloud. The right approach seems to put them offscreen for normal users with a position:absolute solution. The following link gives more information about that : http://www.webaim.org/techniques/css/invisiblecontent/

Wookai
+1  A: 

Your code looks OK, but you can cache jQuery DOM elements so you dont have to traverse the DOM structure more than necessary. You can also chain events to make fewer lookups:

var input = $('input');
input.focus(fn).blur(fn)...

you can also cache this to avoid extra jQuery calls:

var context = $(this);

You should not hide using display:none if you want to access screen readers. I recommend using something lke position:absolute; left:-10000px; instead.

Here's an optimized version of your script:

jQuery(function($) {

    var input = $('input');
    var GetInputLabels = function() {
        input.each(function() {
            var elem = $(this);
            var label = elem.prev('label').text();
            elem.val(label).css("color", "#bbb").attr("title", label);
        });
    }

    GetInputLabels();

    input.focus(function() {
        var elem = $(this);
        if (elem == elem.prev('label').text()) {
            elem.val('').css("color", "#000");
        }                                       
    }).blur(function() {
        var elem = $(this);
        if (elem.val() == '') {
            elem.val(elem.prev("label").text()).css("color", "#bbb");
        } 
    });
});
David
These answers work for me. Thanks all.
ryanulit
A: 

One important thing you need to remember is to cache your DOM queries. You can also chain together your code.

var inputs = null;
function getInputs() {
    if (inputs == null) {
        inputs = $("input");
    }

    return inputs;
}


$(document).ready(function() {
    getInputs().each(function() {
        var elm = $(this);
        var label = elm.prev("label").text();
        elm.val(label).css("color", "#bbb").attr("title", label);
    }).focus(function() {
        var elm = $(this);
        if (elm .val() == elm .prev("label").text()) {
             elm .val('').css("color", "#000");
        }                                       
    }).blur(function() {
        var elm = $(this);
        if (elm.val() == '') {
            elm.val(elm.prev("label").text()).css("color", "#bbb");
        } 
    });
});
ChaosPandion
One typo in there. $("inputs") should be $("input"), but after fixing that it works. I didn't even think about chaining events; good idea.
ryanulit