Prinzhorn / skrollr-menu

skrollr plugin for hash navigation
MIT License
285 stars 143 forks source link

Ignore events handled by other listeners #40

Open stianlik opened 10 years ago

stianlik commented 10 years ago

Avoid scrolling if link is used by a third-party. E.g. if I use Twitter Bootstrap with the carousel-component in to show images, I do not want my page to start scrolling when users click next/prev.

Related to https://github.com/Prinzhorn/skrollr/pull/474

Prinzhorn commented 10 years ago

You should just stopPropagation then. There are valid cases where you prevented the default, but still want skrollr-menu.

stianlik commented 10 years ago

Good point, however, this may be a problem if you are using third-party code (in my case http://getbootstrap.com/javascript/#carousel) that call event.preventDefault() for handled events. I guess one could add an event listener that gets triggered between the carousel and skrollr-menu, but that does not seem ideal.

Would it be acceptable if I made it configurable? I.e.

skrollr.menu.init({
    ignoreDefaultPrevented: true | false
})

or a more general version

skrollr.menu.init({
    handleClick: function(event) {
        if (/* I want this event to be handled by skrollr-menu */) {
            return true;
        }
        return false;
    }
})
Prinzhorn commented 10 years ago

We should probably handle this in some way or another, but can't you just use a button element? I don't know why the bootstrap docs use links there. It's not best practice, because it's not what links are made for. You can just use data-target instead of href on any non-a element for all Bootstrap JavaScript plugins.

stianlik commented 10 years ago

I hadn't thought of that, using a button will solve the issue for my current project, thanks for the tip. I guess most cases can solved be this way. Although, I still think it will be useful to support defaultPrevented in some way, it is not critical.

meza commented 10 years ago

I would love this so much right about now :D

Prinzhorn commented 10 years ago

I would love this so much right about now :D

What is your use case? There is probably a workaround.

meza commented 10 years ago

<a href="mailto:.." and <a href="tel:..."

Also, it seems to disregard target="_blank" with the mailto link, which is pretty annoying :(

Prinzhorn commented 10 years ago

<a href="mailto:.." and <a href="tel:..."

they should both be ignored by skrollr-menu completely (https://github.com/Prinzhorn/skrollr-menu/blob/master/src/skrollr.menu.js#L71). If not, please create a demo on jsbin.com