5
\$\begingroup\$

When you click open - a div slides out. If you click on the new div, an additional div slides out from under it. This is working great, but I need two things that I cant figure out!

  1. How can I condense the jQuery so I don't have to add a class to it every time I want a new slider? Is it possible to do a .sibling kind of thing, or something like this?

  2. I've tried making a close button, but I cannot get it to work the way I want. When you click close, I want the bottom div to close first, then the slide out div to close. And this button would need to work even if only one div is open.

jsFiddle

$(document).ready(function() {
   $('.cambridge').hide();  
   $("#test").click(function () {
      $(".cambridge").toggle("slide", { direction: "right" }, 1000);
   });
   $('.shopping').hide();  
   $("#test2").click(function () {
      $(".shopping").toggle("slide", { direction: "up" }, 1000);
   });
});
\$\endgroup\$

migrated from stackoverflow.com Nov 27 '12 at 17:43

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ I figured out the second part! Now if you hit the close button, the bottom slider closes first, then the main slider closes second. jsfiddle.net/avXSd/78 I still think there is a way to condense this function so that I don't have to edit it every time I want to add a new div. Any thoughts? \$\endgroup\$ – Anthony A Nov 27 '12 at 19:16
  • \$\begingroup\$ I don't understand your first question. What Css Class are you talking about? i don't see you're adding any class. Regarding the second question, you can use the complete callback function on the toggle fn. // That's the second question $('#showmap').click(function() { $(".shopping").toggle("slide", { direction: "up" }, 1000, function () { $(".cambridge").toggle("slide", { direction: "right" }, 1000); }); }); \$\endgroup\$ – Raul Fernandez Apr 13 '16 at 10:53
1
\$\begingroup\$

A quick review:

JSHint.com

  • Your code passes all checks, well done

Naming

  • test and test2 are unfortunate names for elements, I am sure you can come up with something better

Counter Proposal

Looking at the code, there is definitely repetition in making those sliders, you can extract what is common in to a function, and then use that function for any future sliders:

(This is blatantly stolen/modified from the deleted answer):

$(document).ready(function() {

   function registerSlider( buttonId, sliderClass, direction){
     $(sliderClass).hide();  
     $(buttonId).click(function () {
       $(sliderClass).toggle("slide", { direction: direction }, 1000);
     });
   }   
   registerSlider("#test", '.cambridge', 'right' );
   registerSlider("#test2", '.shopping', 'up' );

});

Your questions

  1. I showed in my counter proposal how you can condense the code, but I do believe you will need each time a distinct class

  2. Finding the answer to that question is not trivial (we would need a working sample), and not something codereview does.

\$\endgroup\$
  • 1
    \$\begingroup\$ Haha, your answer tricked me into answering a 4 year old question as well. Well done. :D \$\endgroup\$ – insertusernamehere Jan 11 '17 at 18:53
1
\$\begingroup\$

Let me show you another way, how you can handle this. This solution will work with any number of slides. I hope this helps you. The basic idea is this:

  • open a slide
  • save this slide on a stack
  • continue opening slides or
  • close one or more slides

In case of closing it will close all slides up to the selected one. This way you don't need a close all button and you can't get a result where a "nested" slide is still open but its parent is closed.

HTML

To loosen the JavaScript from the markup I've introduced some data-*-attributes:

  • data-slide="[name]"
  • data-id="[name]"
  • data-direction="[right|up]"

data-slide="[name]" represents a trigger for a slide. In your case:

<div data-slide="slide-1">Toggle Slider</div>

data-id="[name]" identifies a slide. Also information about the direction is stored here:

<div class="cambridge slideout" data-id="slide-1" data-direction="right"> 

JavaScript

const DELAY = 1000;
var stack = [];

function close(e) {
    var value = null;

    if (!stack.length) {
        return;
    }

    value = stack.pop();
    value.removeClass('active').toggle('slide', {direction: value.data('direction')}, DELAY);

    if (!e.length || e.data('id') != value.data('id')) {
        setTimeout(function() { close(e); }, DELAY);
    }
}

$('[data-slide]').click(function() {
    var e = $('[data-id="' + $(this).data('slide') + '"]');

    if (!e.hasClass('active')) {
        e.addClass('active').toggle('slide', {direction: e.data('direction')}, DELAY);
        stack.push(e);
    } else {
        close(e);
    }
});

Advantages

  • HTML and JavaScript are more decoupled
  • no class- or id-selectors necessary
  • no close all function necessary (but you can simply call close() to close all slides anyway)
  • no ghost slides are visible if you close a "parent" element

Further Improvements

This code can have side effects, when you open a slide while it's closing multiple others. This should be addressed.

jsFiddle Demo

Try before buy

\$\endgroup\$
  • 1
    \$\begingroup\$ Very nice answer ;) +1 \$\endgroup\$ – konijn Jan 11 '17 at 19:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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