wesleytodd / nighthawk

A wrapper around Express' router for the browser
ISC License
55 stars 7 forks source link

Add data-skip-router #7

Closed ynonp closed 8 years ago

ynonp commented 8 years ago

Allow skipping the router by specifying a special data-skip-router attribute on a local link

In some cases it's useful to have an a element that doesn't go through the router, because it is handled in a different or special way.

In my use case normal routing calls the server (via ajax) to fetch data for the next page, but some pages are closely related (for example multiple tabs), so when a user enters one tab I load all data for all tabs. Changing tabs now no longer has to go through the normal router middleware to fetch all data. This change allows me to mark the tab links as a elements that skip the router, AND handle them separately with onClick handler.

wesleytodd commented 8 years ago

Im not sure that this change is required to get behavior like you describe. I think if you bind directly to the links for your onClick and stopPropagation on it, then Nighthawk should never see the click, and then not do anything. If you can show that this is not the case let me know and we can explore this further.

ynonp commented 8 years ago

Hi Wes,

The following test fails. Is that what you had in mind? something else?

    it('should not intercept click if stopPropagation was called', function () {
        router._processRequest = function () {
            throw new Error('Should not have been called!!');
        };

        evt.target = document.createElement('a');
        evt.target.setAttribute('href', '/foo');
        evt.target.addEventListener('click', function (e) {
            e.stopPropagation();
        });

        router.onClick(evt, evt.target);
    });
wesleytodd commented 8 years ago

Not exactly, because the stopPropagation is a browser event thing, this test does not behave the same way. The tests for this package do not try to actually mimic browser events, they just are testing internal behavior.

An actual test would be to create a test page with code like that above, and try clicking the link. The only other way would be to use a testing tool like selenium to actually imitate a real click event. But personally I try to avoid those kind of tests because they are hard to write and maintain.

EDIT: and after re-reading it one more time, I think that the event listener you registered isn't being called in the above code.

ynonp commented 8 years ago

Thanks! After double checking using stopPropagation did solve my issue and prevented routing. No need for this PR