0

I have to admit, I came up with this solution by pure intuition from seeing many jQuery scripts during all the time I've been experimenting and learning on my own.

Since I don't have anyone to ask in person these kind of questions, that's why I come here for help and guidance.

--

Inspired by Wufoo's plans' forms where the row you need to focus is highlighted while filling out the form, I created this script (I am not a jQuery or JavaScript guru, I'm still learning and practicing):

//Improve usability by highlighting the row the user needs to focus on:
$(function() {
        //When input element has focus
        $('input').focus(function(){
            //Add the class to its corresponding row
            $(this).parent().parent('.row').addClass('focus'),
                //And when it loses focus
                $('input').blur(function(){
                    //Remove the class
                    $(this).parent().parent('.row').removeClass('focus');
                });
        });
});

So I was wondering if this script has a way of being written better or optimized/shorten in any way. If not, and the way I wrote it's Ok, that's fine, all I'd like to learn is ways of optimizing code when possible, that's all.

I created a Demo in Codepen if you want to check it out.

Thanks in advance.

Improve this question
2

3 Answers 3

Reset to default
4

You can do this:

//Improve usability by highlighting the row the user needs to focus on:
$(function() {
        $('input').focus(function(){
            $(this).closest('.row').addClass('focus'); // Change here                    
        }).blur(function(){ // Change here
           $(this).closest('.row').removeClass('focus'); // Change here
        });
});

Or even shorter!

$(function(){
    $("input").on("focus blur", function(){
        $(this).closest(".row").toggleClass("focus");
    });
});

You did 1 thing totally wrong!! Each time you focus you bind a new blur to each input!

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Like the super-short version, very neat :)
This is great, thanks for the help :) Demo has been updated as well. - Regarding what I did 'so' wrong, what does "binding a new blur to each input" mean? Thanks again.
You did a $("input").blur(fu which is a new bind within a $("input").focus(. So each time you focus. You do a new bind. Which means if you focus 1 input 2 times. You got 2 blur events. If you focus 1 more, you got 3 blur events etc. You don't want to do that. So don't bind within a bind. You can just bind the blur beforehand.
4

Code looks good. The only changes I'd make are to use closest() instead of chaining parent() calls, as if you change the DOM around you would need to change your jQuery code. Also, I'd move the blur() handler outside of the focus() handler. Try this:

$(function() {
    $('input')
        .focus(function() {
            $(this).closest('.row').addClass('focus');
        })
        .blur(function() {
            $(this).closest('.row').removeClass('focus');
        });
});
Improve this answer

1 Comment

Thanks for the explanation about using .closest(), I had a feeling using double .parent() wasn't the best way. Your code worked great but Niel's short version was right on the money. Gave you an upvote though. Thanks.
1

I would try to keep my even handlers and event binding separated. This is convenient to do with the .on() which actually binds the event to all the current and future .row elements unlike the .focus() which only binds the event on the rows you had on the page when the binding happened.

This way you don't need to wrap your code into $(function(){}); either. And you can have the event binding in an included .js file instead of mixing your jQuery with your markup.

Additionally, use .closest() instead of .parent().parent() because it won't break if you choose to change your structure a bit. Provided that your .row is still there.

var onRowFocus = function(){
    $('.row').removeClass('focus');
    $(this).closest('.row').addClass('focus');
};

$('.row').on('focus', 'input', onRowFocus);

Also, you don't need to bind the .blur event at all. Simply remove the focus class from all rows before you add it to the one on focus. Less event handlers, less code, easier to maintain.

Improve this answer

1 Comment

Oh, well, this is quite nice as well and works great. Will consider the lesson here and your explanations. Gave you an upvote. Thanks a lot.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.