1

I have an on function that has two mouse events inside of it mouseenter and mouseleave. When these events are triggered they run different functions, one adds a class, the other removes it.

$(this).siblings('.testimonial').find('p').addClass('unseen');

$(this).siblings('.testimonial').find('p').removeClass('unseen');

The thing is, I’m doing the following DOM traversal twice:

$(this).siblings('.testimonial').find('p')

But I don’t see how I can save this traversal as a variable in one function and use it as another. Here is my full JS code:

JavaScript

(function ($) {

    var testimonial = $('.testimonial');
    var testimonialHeight = testimonial.outerHeight();
    var testimonialWidth = testimonial.outerWidth();

    testimonial.find('p').addClass('unseen');

    testimonial.css({
        height: testimonialHeight,
        width: testimonialWidth
    });

    $('.client').on({
        mouseenter: function() {
            $(this).siblings('.testimonial').find('p').removeClass('unseen');
        },
        mouseleave: function() {
            $(this).siblings('.testimonial').find('p').addClass('unseen');
        }
    });

})(jQuery);

HTML

<ul class="list-unstyled list-inline">
  <li>
    <div class="testimonial"><p>Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged.<p></div>
      <img class="client" src="https://s3.amazonaws.com/uifaces/faces/twitter/jsa/128.jpg" alt="" />
  </li>
  <li>
    <div class="testimonial"><p>Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged.</p></div>
    <img class="client" src="https://s3.amazonaws.com/uifaces/faces/twitter/gerrenlamson/128.jpg" alt="" />
  </li>
  <li>
    <div class="testimonial"><p>Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book.</p></div>
    <img class="client" src="https://s3.amazonaws.com/uifaces/faces/twitter/jadlimcaco/128.jpg" alt="" />
  </li>
</ul>

Can anybody suggest a better way of doing this?

Thanks.

  • 3
    Please show us the relevant HTML to we can understand the DOM traversal you're asking about and which piece of your code are you asking about? Also, what exact problem are you trying to solve? Does the code not work? Or are you thinking that it is too slow? – jfriend00 Feb 03 '15 at 22:15
  • Why are you finding `p` when you can just do `.testimonial p`? Also, have you tried using `hover`? And why not do the traversal in one variable, and then just add/hide class to that variable...kinda like you did with `testimonial`. – Waxi Feb 03 '15 at 22:17
  • @jfriend00 I’ve added the HTML above. The code works but the problem is that I’m repeating myself. I’m doing the same DOM traversal twice. I want the code to work but I also want it to be DRY. – IeriOggiDomani Feb 04 '15 at 00:50
  • @slime because when I use `testimonial.find('p')` I’m not traversing the whole DOM again. If I use `$('.testimonial p')` then I would be. I think the HTML I’ve added above might make things clearer. – IeriOggiDomani Feb 04 '15 at 00:56
  • Are you talking about the two copies of this code: `$(this).siblings('.testimonial').find('p')`? If so, please say THAT in your question. Lots of your code is doing DOM traversal so that phrase by itself does not tell us WHICH line of code you're asking about. Please work on being more clear with your question. If you're asking about a specific line of code, then tell us exactly which line of code. – jfriend00 Feb 04 '15 at 00:59
  • This should be done using only CSS imho, something like here: http://jsfiddle.net/c9zcsjz0/ – A. Wolff Feb 04 '15 at 11:51
  • @A.Wolff no because I want the `.testimonial p` to appear only when hovering over the image, not hovering over the list item. I will create a JSFiddle later and update my question to show you what I mean. But thank you for your recommendation. – IeriOggiDomani Feb 05 '15 at 11:42
  • You just have to modificate a little your HTML markup then to use only CSS, e.g: http://jsfiddle.net/c9zcsjz0/1/ – A. Wolff Feb 05 '15 at 13:09

5 Answers5

4

You can change to have a common event handler for both events and set the operation depending upon which event it was:

$('.client').on("mouseenter mouseleave", function(e) {
    var method = e.type === "mouseenter" ? "removeClass" : "addClass";
    $(this).siblings('.testimonial').find('p')[method]('unseen');
});

Here's an explanation of what's going on:

  • .on("mouseenter mouseleave", function(e) {...}) hooks up multiple events to the same event handler.
  • e.type is the name of the event for the current event so when you have multiple events triggering the same event handler, you can see which event it was that triggered.
  • var method = e.type === "mouseenter" ? "removeClass" : "addClass" is like an if/else statement where it assigns either "removeClass" or "addClass" to the method variable based on the value of e.type. It's called the ternary operator.
  • obj[method] is a property reference using a variable for the name of the property instead of a string literal. So obj.addClass is the same as obj[method] when method is "addClass". Adding the () onto the end to make it a function call, then obj.addClass('unseen') is the same as obj[method]('unseen') when method is "addClass".

So, to break that last line down again:

// find the right paragraphs
$(this).siblings('.testimonial').find('p')

// get the property whose name is in the method variable
$(this).siblings('.testimonial').find('p')[method]

// call that property as a function and pass it 'unseen'
$(this).siblings('.testimonial').find('p')[method]('unseen');

One possible useful tool for DRY is .hover() because it is a shortcut for mouseenter and mouseleave. If you know that the relevant paragraphs are always marked as unseen before hover and no other code in the page ever messes with the unseen class (something you don't say anything about in your question), then you can use a shortcut using .hover()

$('.client').hover(function() {
    $(this).siblings('.testimonial').find('p').toggleClass('unseen');
});

The more common way of just moving repeated code into a common function that you can use in both places would look like this:

function getSiblingParagraphs(parent) {
    return $(parent).siblings('.testimonial').find('p');
}

$('.client').on({
    mouseenter: function() {
        getSiblingParagraphs(this).removeClass('unseen');
    },
    mouseleave: function() {
        getSiblingParagraphs(this).addClass('unseen');
    }
 });
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • 1
    @IeriOggiDomani - Added new method to my answer using a single common event handler that is DRY and also compact. – jfriend00 Feb 04 '15 at 01:10
  • The first way has a few concepts I don’t understand so I have yet to try, but the second option is exactly what I was looking for. Thanks! – IeriOggiDomani Feb 04 '15 at 01:53
  • by the way, in the first solution, on line `$(this).siblings('.testimonial').find('p')[method]('unseen');` is `[method]` referencing an array? and why isn’t it following a `.`? As you can probably tell I’m far from being intermediate :-P – IeriOggiDomani Feb 04 '15 at 01:57
  • 1
    @IeriOggiDomani - I added some explanation to my answer. The short answer is that `obj[someVariable]` is a means of accessing a property of `obj` whose property name is in `someVariable`. It's how you access properties when the name is in a variable, not in a string literal. So, if `someVariable` was `"removeClass"`, then `obj[someVariable]()` is the same as `obj.removeClass()`. This makes it easier to be DRY when everything in the line of code is the same except the method name. You can put the method name in a variable and then execute the statement using the `obj[method]` syntax. – jfriend00 Feb 04 '15 at 02:23
  • If all the question was about DRYing code, it seems better to use relevant methods as hover in/out handler with toggleClass() method – A. Wolff Feb 04 '15 at 11:25
  • @A.Wolff - I wasn't sure about the initial state of the class on every possible target element because `.toggleClass()` only does what this code does if you know that every single paragraph will always be `'unseen'` before hover and that no other code ever changes it. I had no way of knowing if that was always true. I will add a note about `.hover()` to my answer. – jfriend00 Feb 04 '15 at 16:41
  • This is a **great** answer, there is 3 different solutions given, all very well-explained. There is so much knowledge here that's going to help me level-up my JS!!! – IeriOggiDomani Feb 05 '15 at 10:17
1

You could use following logic if it does really matter to cache the specific siblings children elements and not all .testimonial p:

$('.client').each(function () {
    this._testimonialP = $(this).siblings('.testimonial').find('p').addClass('unseen');// But .unseen should be set in HTML markup by default
}).hover(function () {
    this._testimonialP.toggleClass('unseen');
});
A. Wolff
  • 74,033
  • 9
  • 94
  • 155
  • This creates custom DOM properties that contain references to other DOM objects. In the past, this has been known to cause memory leaks in some browsers. This also would not work if any of the siblings or sibling content were dynamically created or ever changed. – jfriend00 Feb 04 '15 at 01:03
  • This works great. This is even better because if JavaScript isn’t working then the `.testimonial p`’s appear by default. Why is there a comment saying `But .unseen should be set in HTML markup by default` if you’re adding it in the JS? One more question (out of curiosity) — why do you prepend an underscore on the `_testimonialP` variable? Thank you for your answer. – IeriOggiDomani Feb 04 '15 at 01:14
0

If you're looking for a DRY way to do it, you could write a reusable function that finds the sibling you want.

function findTestimonialParagraph($root) {
    return $root.siblings('.testimonial').find('p');
}

$('.client').on({
    mouseenter: function() {
        findTestimonialParagraph($(this)).removeClass('unseen');
    },
    mouseleave: function() {
        findTestimonialParagraph($(this)).addClass('unseen');
    }
});

This way, if you need to change how the testimonial paragraph is accessed, you only need to do it in one place.

cvializ
  • 616
  • 3
  • 11
-1

You could simply do the following:

var $client = $('.client');
var $clientTestimonialParagraphs = $client.siblings('.testimonial').find('p');

$client.on({
        mouseenter: function() {
            $clientTestimonialParagraphs.removeClass('unseen');
        },
        mouseleave: function() {
            $clientTestimonialParagraphs.addClass('unseen');
        }
    });

I hope that this helps.

skay-
  • 1,546
  • 3
  • 16
  • 26
  • Perhaps the pollution of the global scope? And the waste to save something before it is ever used? – mplungjan Feb 03 '15 at 22:30
  • Its not polluting the global scope... the code (from the author Question) its within a function (so function scope). As for saving something before it is used... nothing wrong with that, the alternative is lazy instantiation; both have pros and cons. – skay- Feb 03 '15 at 22:36
  • This may not work. Assuming there could be more than one `.client` object, then the paragraphs found must be relevant to the one that received the mouse event, not all of them. – jfriend00 Feb 04 '15 at 01:05
  • I have since edited the question and added the HTML to show what the JS is manipulating. With this solution, this will select **all** instances of `.client` on the page. As you can see in my HTML, there are actually 3 `div`s with the `.client` class applied. This means that when I hover one `.client` the `p` tag in every `div.testimonial` will get an `unseen` class applied. Thanks for your answer though. – IeriOggiDomani Feb 04 '15 at 01:05
-1

Try to store selected items

var p = testimonial.find('p').addClass('unseen');

and then operate with stored ones

$('.clienet').hover(
    function(){ p.removeClass('unseen') }, 
    function(){ p.addClass('unseen') }
)
mashi
  • 532
  • 3
  • 6