0

I'm trying to avoid having the same lines of Javascript for the same purpose.

I have 3 sections:

<div class="specs"></div>
<div class="description"></div>
<div class="comments"></div>

And these 3 links:

<a href="#" class="readMore desc">Produkt beskrivelse</a>
<a href="#" class="readMore spec">Produkt specs</a>
<a href="#" class="readMore facebook"></i>Kommentarer</a>

And this javascript which, on click scrolls to the section

$(".facebook").on('click', function () {
    $('html, body').animate({
        scrollTop: $(".comments").offset().top - 200
    }, 1000);
});

$(".readMore.desc").on('click', function () {
    $('html, body').animate({
        scrollTop: $(".description").offset().top - 200
    }, 1000);
});

$(".readMore.spec").on('click', function () {
    $('html, body').animate({
        scrollTop: $(".specs").offset().top - 200
    }, 1000);
});

These 3 pieces of javascript code is annoying because it does the exact same thing.

A live example can be seen here a live example. You'll see the 3 buttons on the right of the product image.

I don't know if a solution could be to add an array of some sort?

Improve this question

7 Answers 7

Reset to default
3

One way of handling this is giving each link a data- property that describes where the link should scroll to. You can use .data() to access these properties.

$(".readMore").on('click', function() {
  // Get the selector of where to scroll to
  var selector = $(this).data('selector');
  $('html, body').animate({
    scrollTop: $(selector).offset().top - 200
  }, 1000);
});
html, body {
  height: 100%;
}
div {
  height: 100%;
  margin-top: 20px;
  border: solid 1px #000;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<a href="#" class="readMore desc" data-selector=".description">Produkt beskrivelse</a>
<a href="#" class="readMore spec" data-selector=".specs">Produkt specs</a>
<a href="#" class="readMore facebook" data-selector=".comments">Kommentarer</a>

<div class="specs">
  Specs
</div>
<div class="description">
  Description
</div>
<div class="comments">
  Comments
</div>

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

1 Comment

I added this as the answer because I like the way the data attribute was used and the code is very clean.
2

Common classes (which you have) and data attributes will save you here.

<a href="#" class="readMore desc" data-dest=".comments">Produkt beskrivelse</a>
<a href="#" class="readMore spec" data-dest=".specs">Produkt specs</a>
<a href="#" class="readMore facebook" data-dest=".description"></i>Kommentarer</a>

And now, one handler to rule them all:

$(".readMore").on('click', function () {
    var dest = $(this).data("dest");

    $('html, body').animate({
        scrollTop: $(dest).offset().top - 200
    }, 1000);
});
Improve this answer

1 Comment

@MikeC -- Hah, yep! You took it 1 step further tho with the CSS - I stay away from devilish part of web-design :D
1 
//extraced the common parts
function scrollToTop ( elementSelector ) {
    $('html, body').animate({
        scrollTop: $(elementSelector).offset().top - 200
    }, 1000);
}

$(".facebook").on('click', function () {
    scrollToTop('.comments');
});

$(".readMore.desc").on('click', function () {
    scrollToTop('.description');
});

$(".readMore.spec").on('click', function () {
    scrollToTop('.specs');
});
Improve this answer

Comments

0

Use a helper function instead of copy-pasting your code

function foo(target, element) {
  target.on('click', function () {
      $('html, body').animate({
          scrollTop: element.offset().top - 200
      }, 1000);
  });
}

foo($(".facebook"), $(".comments"));
foo($(".readMore.desc"), $(".description"));
foo($(".readMore.spec"), $(".specs"));
Improve this answer

4 Comments

Nice suggestion, but one thing I'd point out: Don't use top as the variable name, as it's reserved. I wouldn't expect top.offset().top to work.
Oh yes, i'll rename it to element
I would have used this one but I think Mike C's suggestion was a little cleaner. Although I learned greatly from this
Yes, i like his solution either
0

Probably better you just read the class on the object, split it to get the value you want. As such: 

$('.readMore').on('click', function() {
    var classes = $(this).attr('class');
    var cursor = class.split(' ')[1];

    if(cursor == 'facebook') {
       ...
    }else if(cursor == 'desc') {
       ...
    } else if(cursor == 'spec') {
       ...
    } 
});
Improve this answer

Comments

0

First you'll need to map which dom is effecting which. you could have solved this by using some kind of class name convention. I'll assume you can't decide on the class names. So let's create a map/object/hash

var map = {
  spec: "specs",
  desc: "description",
  facebook: "comments,
}

Now let's just iterate the map and add the functionality

Object.keys(map).forEach(function(key) {
  var value = map[key];
  $(".readMore." + key).on('click', function () {
    $('html, body').animate({
        scrollTop: $("." + value).offset().top - 200
    }, 1000);
  });
})

And now you are a happy coder.

Improve this answer

Comments

0

If you've learned closures, I prefer those to make re-usable events more readable...

I have a jsFiddle for this here

// use a closure to make your event's callback,
// with the target as a parameter
function makeClickFn(target) {
    return function() {
        $('html, body').animate({
            scrollTop: $(target).offset().top - 200
        }, 1000);
   };
}

var clickFn;

// facebook comments
clickFn = makeClickFn('.comments');
$(".facebook").on('click', clickFn);

// readmore description
clickFn = makeClickFn('.description');
$(".readMore.desc").on('click', clickFn);

// readmore specs
clickFn = makeClickFn('.specs');
$(".readMore.spec").on('click', clickFn);
Improve this answer

Comments