kupriyanenko / jbone

JavaScript Library for Events and DOM manipulation. Replaces jQuery for Backbone (2.5kb gzipped)
http://jbone.js.org
MIT License
279 stars 35 forks source link

Method one() is not working as expected #45

Closed henryruhs closed 9 years ago

henryruhs commented 9 years ago

The method one() from event.js worked fine in 1.0.22 but fails now.

I don't have the time for reengineering your changes for the on() and Event. Sorry, but your code lacks of documentation and is not that easy to understand for me.

Nothing is lost because I can provide a modified test that is working in 1.0.22 but fails in 1.0.25

    it('one(event, callback)', function() {
        var div = jBone('<div>'), counter = 0, fn = function() {
            counter++;
        };

        div.one('click', fn);

        div.trigger('click').trigger('click');

        expect(counter).be.eql(1);

        var divs = jBone('<input><input>');

        divs.one('foo', fn);

        divs.eq(0).trigger('foo');

        divs.one('foo', fn);

        divs.eq(1).trigger('foo');
        divs.eq(1).trigger('foo');

        expect(counter).be.eql(4);
    });

The issue appears once the one() was fired and added again... it does not fire another time

henryruhs commented 9 years ago

For those who run into the same issue, here is a tempfix branch that can be used temporary with bower:

"dependencies": {
     "jbone": "https://github.com/redaxmedia/jbone.git#tempfix"
}

I just reverted the event.js from 10.0.22 and made a new build.

kupriyanenko commented 9 years ago

Thanks for founded bug. Fixed in 1.0.26

henryruhs commented 9 years ago

I cannot reproduce this with a unit test, but in my pageflip app the one() method is not working multiple times with latest jBone - only once... Works perfect with jQuery and Zepto!

henryruhs commented 9 years ago

Working on() method:

    fn.on = function(event) {
        //
        // no problem here
        //
        addListener = function(el) {
            jBone.setId(el);
            events = jBone.getData(el).events;
            event.split(" ").forEach(function(event) {
                var eventOptions = {};
                if (data) {
                    eventOptions.data = data;
                }
                eventType = event.split(".")[0];
                namespace = event.split(".").splice(1).join(".");
                events[eventType] = events[eventType] || [];
                fn = function(e) {
                    if (e.namespace && e.namespace !== namespace) {
                        return;
                    }
                    expectedTarget = null;
                    if (!target) {
                        callback.call(el, new BoneEvent(e, eventOptions));
                    } else if (~jBone(el).find(target).indexOf(e.target) || (expectedTarget = jBone.contains(jBone(el).find(target), e.target))) {
                        expectedTarget = expectedTarget || e.target;
                        eventOptions.currentTarget = expectedTarget;
                        callback.call(expectedTarget, new BoneEvent(e, eventOptions));
                    }
                };
                events[eventType].push({
                    namespace: namespace,
                    fn: fn,
                    originfn: callback
                });
                el.addEventListener && el.addEventListener(eventType, fn, false);
            });
        };
        for (; i < length; i++) {
            addListener(this[i]);
        }
        return this;
    };

Not working on() method:

    fn.on = function(types) {
        //
        // no problem here
        //
        for (; i < length; i++) {
            jBone.event.add(this[i], types, handler, data, selector);
        }
        return this;
    };

Please check addListener() vs. event.add() ... it does not attach events the same way and effects the one() somehow...

kupriyanenko commented 9 years ago

Can you share with me your code for pageflip, if I will reproduce it, I will fix it :)

henryruhs commented 9 years ago

I cannot setup the pageflip online because it is still in development and needs an API to load the brochures... I cannot reproduce the problem in a jsfiddle, because the pageflip is a complex framework.

  1. After navigate a Event renderStart get fired as the pageflip refreshed the brochure and UI
  2. A Method does look if the top or bottom slide is opened and fires the Event slideTopOpen once
  3. Event renderStart is binded with on() and slideTopOpen with one() but gets re-added again once the whole pageflip container refreshed

Object jBone._cache.events may cause the problem... I made the same amount of action in both screens. I hope you can fix this somehow...

Latest jBone - the overview fails to render after multiple navigate:

Fixed jBone

Fixed jBone - the overview does render perfect:

Latest jBone

kupriyanenko commented 9 years ago

Please try version 1.0.27, I think bug should be fixed.

kupriyanenko commented 9 years ago

And I hope tests for your case: add test for handle multiple on()/off()

henryruhs commented 9 years ago

Works for me, thank you so much!