current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

Empty Textboxes, class="loc" display "(City, State \ Country)" using italics and silver. Of course, Textboxes that do have data are displayed according to page defaults.

I noticed chaining and caching could be used. There are inheritances issues, so I am not bothering with using .addClass instead of targeting with .css.

    $(function () {
        var loc = $('.loc');

        function desc() {
            var tar = $(this);
            if (tar.val().length == 0) {
                tar.val("(City, State \\ Country)").css({
                    "color": "silver",
                    "fontStyle": "italic"
                });
            };
        }

        function data() {
            var tar = $(this);
            if (tar.val() == "(City, State \\ Country)") {
                tar.val("").css({
                    "color": "black",
                    "fontStyle": "normal"
                });
            };
        }

        loc.each(desc).blur(desc).click(data);
    });

How far off base am I on understanding proper formatting in jQuery?

share|improve this question
up vote 3 down vote accepted

A couple of things:

  • Anytime you compare to 0 (zero) you want to make sure you use the exactly equal operator === instead of ==. What would make your code even more expressive is doing something like if(tar.val().length){} . This way your code is saying is "if the value of tar is blank then do this"
  • I'd also bring the last line to the top of your function and place it under the var loc statement. The functions will be hoisted above all that so they'll be there even if they are defined after the calls.
  • this could prob be re-factored into one function with a simple if/else statement checking the value of the field.
  • Instead of using the each to call the data function ,assuming you don't have any other events attached to blur event you could call blur explicitly at the end of your chain to invoke that functionality ...click(data).blur().
  • I would look into ANeves suggestions as well.
share|improve this answer
    
About your second point, doesn't one need to define desc before calling loc.each(desc)? – ANeves Oct 20 '11 at 3:43
up vote 2 down vote

You should be using input's placeholder attribute instead of implementing the functionality by yourself.

I suggest you read this article on stackoverflow: http://stackoverflow.com/questions/7557801/can-i-use-jquery-to-blank-textarea-fields-or-ajax-like-input-boxes

If you want to go the extra mile to ensure that even old browsers will have the functionality, the selected answer in that article explains how.

share|improve this answer
up vote 1 down vote

Additionally to the answers of bittersweetryan and ANeves, I'd like to suggest:

1) Don't set the styles directly, but instead use a class, so that the styles are not hard coded into the script - especially important if the default font style isn't black and non-italic:

function desc() {
    var tar = $(this);
    if (tar.val().length == 0) {
        tar.val("(City, State \\ Country)").addClass("placeholder");
    };
}

function data() {
    var tar = $(this);
    if (tar.val() == "(City, State \\ Country)") {
        tar.val("").removeClass("placeholder");
    };
}

with

.loc.placeholder {
  color: silver;
  font-style: italic;
}

2) You should call data on focus instead of click, because a input field can obtain focus in other methods other than clicking.

3) Finally you should move the string constants out of the code and make them parameters, so that the code is more flexible, for example, if you want to use it with other inputs.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.