You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Andrew Grieve <ag...@chromium.org> on 2012/08/24 17:37:29 UTC

Channel.fire() and onPause/onResume

I was looking through the channel code and noticed that the fired state is
never assigned to false. This makes sense for channels like onDeviceReady,
when you'd want this to stay active. But...

It's a bit strange for things like onPause and onResume. It means that if
the app has ever been paused or resumed, then every new listener going
forward is going to fire upon subscription even though the current state is
not paused/resumed. I tested this out locally, and it appears to be true.
So...

How about we add a method called "fireOnce", which calls fire() and then
sets fired = false so that new subscribers will not be immediately
triggered.

Re: Channel.fire() and onPause/onResume

Posted by Andrew Grieve <ag...@chromium.org>.
I took a stab at this:
https://github.com/agrieve/incubator-cordova-js/tree/channel2

I'll wait until 2.1.0 is out before merging.

Be happy to see alternates proposed, but it might not be worth the ROI to
put much more into this I think.


On Mon, Aug 27, 2012 at 12:51 PM, Andrew Grieve <ag...@chromium.org>wrote:

> Some thoughts on this:
>
> Uses:
> 1. Start-up / single-fire events
> 2. Multi-fire custom event types "menubutton", "searchbutton",
> "backbutton", "pause", "resume"
> 3. Hooking document.addEventListener(foo)
>
> Currently state:
> channel.js does 1 well, but does 2 except for the fired state behaviour,
> and 3 is done by cordova.js.
>
> Thoughts:
> - Channel does a good job at #1, although it could be simplified if that's
> all we need it for.
> - We would get truer (proper propagation) event behaviour if we just fired
> custom events on actual DOM via createEvent & dispatchEvent
> - Using DOM events is a bit harder to debug since it goes in & out of
> native. It's also slower for this reason.
>
> How to make Channel good at both 1 and 2:
> - Add a mode to make a channel as sticky or not
> - If Sticky, don't hold onto subscribers after initial fire (free up
> memory)
> - If not sticky, don't fire upon subscription
>
> I'm thinking that we don't need a dramatic change.
>
>
>
>
>
> On Mon, Aug 27, 2012 at 9:50 AM, Filip Maj <fi...@adobe.com> wrote:
>
>> I think it's time for a review of this module and how it fits into
>> cordova-js. Spell out exactly what we need from it, and then find a
>> suitable replacement or do a rewrite. A few of the channels have specific
>> firing needs e.g. deviceready. I think we can get something much
>> smaller/simpler in there.
>>
>> If I remember correctly, I hacked this out of complete ui, a
>> Nitobi-written UI library from like.. 2007. Pretty sure it's Dave
>> Johnson's code too! It may have been too much for our purposes any ways.
>>
>> On 8/24/12 12:15 PM, "Andrew Grieve" <ag...@chromium.org> wrote:
>>
>> >I like that idea. Channel already has an opts param in its ctor, so I'll
>> >just throw a boolean into it.
>> >
>> >channel.create('pause', { multi: true });
>> >
>> >Created a bug for this: https://issues.apache.org/jira/browse/CB-1274
>> >
>> >
>> >
>> >
>> >On Fri, Aug 24, 2012 at 1:46 PM, Gord Tanner <go...@tinyhippos.com>
>> wrote:
>> >
>> >> I know there was some talk on the mailing list awhile ago of replacing
>> >>the
>> >> channel module with something a little simpler.
>> >>
>> >> If we do want to patch this issue I would suggest having this done at
>> >> construction time.
>> >>
>> >> var deviceready = channel.create('deviceready', channel.types.once);
>> >> var pause = channel.create('pause', channel.types.refireable);
>> >>
>> >> (naming in example above is for discussion purposes only ;) )
>> >>
>> >> On Fri, Aug 24, 2012 at 11:37 AM, Andrew Grieve <agrieve@chromium.org
>> >> >wrote:
>> >>
>> >> > I was looking through the channel code and noticed that the fired
>> >>state
>> >> is
>> >> > never assigned to false. This makes sense for channels like
>> >> onDeviceReady,
>> >> > when you'd want this to stay active. But...
>> >> >
>> >> > It's a bit strange for things like onPause and onResume. It means
>> >>that if
>> >> > the app has ever been paused or resumed, then every new listener
>> going
>> >> > forward is going to fire upon subscription even though the current
>> >>state
>> >> is
>> >> > not paused/resumed. I tested this out locally, and it appears to be
>> >>true.
>> >> > So...
>> >> >
>> >> > How about we add a method called "fireOnce", which calls fire() and
>> >>then
>> >> > sets fired = false so that new subscribers will not be immediately
>> >> > triggered.
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Gord Tanner
>> >> Senior Developer / Code Poet
>> >> tinyHippos Inc.
>> >> @tinyhippos
>> >>
>>
>>
>

Re: Channel.fire() and onPause/onResume

Posted by Andrew Grieve <ag...@chromium.org>.
Some thoughts on this:

Uses:
1. Start-up / single-fire events
2. Multi-fire custom event types "menubutton", "searchbutton",
"backbutton", "pause", "resume"
3. Hooking document.addEventListener(foo)

Currently state:
channel.js does 1 well, but does 2 except for the fired state behaviour,
and 3 is done by cordova.js.

Thoughts:
- Channel does a good job at #1, although it could be simplified if that's
all we need it for.
- We would get truer (proper propagation) event behaviour if we just fired
custom events on actual DOM via createEvent & dispatchEvent
- Using DOM events is a bit harder to debug since it goes in & out of
native. It's also slower for this reason.

How to make Channel good at both 1 and 2:
- Add a mode to make a channel as sticky or not
- If Sticky, don't hold onto subscribers after initial fire (free up memory)
- If not sticky, don't fire upon subscription

I'm thinking that we don't need a dramatic change.





On Mon, Aug 27, 2012 at 9:50 AM, Filip Maj <fi...@adobe.com> wrote:

> I think it's time for a review of this module and how it fits into
> cordova-js. Spell out exactly what we need from it, and then find a
> suitable replacement or do a rewrite. A few of the channels have specific
> firing needs e.g. deviceready. I think we can get something much
> smaller/simpler in there.
>
> If I remember correctly, I hacked this out of complete ui, a
> Nitobi-written UI library from like.. 2007. Pretty sure it's Dave
> Johnson's code too! It may have been too much for our purposes any ways.
>
> On 8/24/12 12:15 PM, "Andrew Grieve" <ag...@chromium.org> wrote:
>
> >I like that idea. Channel already has an opts param in its ctor, so I'll
> >just throw a boolean into it.
> >
> >channel.create('pause', { multi: true });
> >
> >Created a bug for this: https://issues.apache.org/jira/browse/CB-1274
> >
> >
> >
> >
> >On Fri, Aug 24, 2012 at 1:46 PM, Gord Tanner <go...@tinyhippos.com> wrote:
> >
> >> I know there was some talk on the mailing list awhile ago of replacing
> >>the
> >> channel module with something a little simpler.
> >>
> >> If we do want to patch this issue I would suggest having this done at
> >> construction time.
> >>
> >> var deviceready = channel.create('deviceready', channel.types.once);
> >> var pause = channel.create('pause', channel.types.refireable);
> >>
> >> (naming in example above is for discussion purposes only ;) )
> >>
> >> On Fri, Aug 24, 2012 at 11:37 AM, Andrew Grieve <agrieve@chromium.org
> >> >wrote:
> >>
> >> > I was looking through the channel code and noticed that the fired
> >>state
> >> is
> >> > never assigned to false. This makes sense for channels like
> >> onDeviceReady,
> >> > when you'd want this to stay active. But...
> >> >
> >> > It's a bit strange for things like onPause and onResume. It means
> >>that if
> >> > the app has ever been paused or resumed, then every new listener going
> >> > forward is going to fire upon subscription even though the current
> >>state
> >> is
> >> > not paused/resumed. I tested this out locally, and it appears to be
> >>true.
> >> > So...
> >> >
> >> > How about we add a method called "fireOnce", which calls fire() and
> >>then
> >> > sets fired = false so that new subscribers will not be immediately
> >> > triggered.
> >> >
> >>
> >>
> >>
> >> --
> >> Gord Tanner
> >> Senior Developer / Code Poet
> >> tinyHippos Inc.
> >> @tinyhippos
> >>
>
>

Re: Channel.fire() and onPause/onResume

Posted by Filip Maj <fi...@adobe.com>.
I think it's time for a review of this module and how it fits into
cordova-js. Spell out exactly what we need from it, and then find a
suitable replacement or do a rewrite. A few of the channels have specific
firing needs e.g. deviceready. I think we can get something much
smaller/simpler in there.

If I remember correctly, I hacked this out of complete ui, a
Nitobi-written UI library from like.. 2007. Pretty sure it's Dave
Johnson's code too! It may have been too much for our purposes any ways.

On 8/24/12 12:15 PM, "Andrew Grieve" <ag...@chromium.org> wrote:

>I like that idea. Channel already has an opts param in its ctor, so I'll
>just throw a boolean into it.
>
>channel.create('pause', { multi: true });
>
>Created a bug for this: https://issues.apache.org/jira/browse/CB-1274
>
>
>
>
>On Fri, Aug 24, 2012 at 1:46 PM, Gord Tanner <go...@tinyhippos.com> wrote:
>
>> I know there was some talk on the mailing list awhile ago of replacing
>>the
>> channel module with something a little simpler.
>>
>> If we do want to patch this issue I would suggest having this done at
>> construction time.
>>
>> var deviceready = channel.create('deviceready', channel.types.once);
>> var pause = channel.create('pause', channel.types.refireable);
>>
>> (naming in example above is for discussion purposes only ;) )
>>
>> On Fri, Aug 24, 2012 at 11:37 AM, Andrew Grieve <agrieve@chromium.org
>> >wrote:
>>
>> > I was looking through the channel code and noticed that the fired
>>state
>> is
>> > never assigned to false. This makes sense for channels like
>> onDeviceReady,
>> > when you'd want this to stay active. But...
>> >
>> > It's a bit strange for things like onPause and onResume. It means
>>that if
>> > the app has ever been paused or resumed, then every new listener going
>> > forward is going to fire upon subscription even though the current
>>state
>> is
>> > not paused/resumed. I tested this out locally, and it appears to be
>>true.
>> > So...
>> >
>> > How about we add a method called "fireOnce", which calls fire() and
>>then
>> > sets fired = false so that new subscribers will not be immediately
>> > triggered.
>> >
>>
>>
>>
>> --
>> Gord Tanner
>> Senior Developer / Code Poet
>> tinyHippos Inc.
>> @tinyhippos
>>


Re: Channel.fire() and onPause/onResume

Posted by Andrew Grieve <ag...@chromium.org>.
I like that idea. Channel already has an opts param in its ctor, so I'll
just throw a boolean into it.

channel.create('pause', { multi: true });

Created a bug for this: https://issues.apache.org/jira/browse/CB-1274




On Fri, Aug 24, 2012 at 1:46 PM, Gord Tanner <go...@tinyhippos.com> wrote:

> I know there was some talk on the mailing list awhile ago of replacing the
> channel module with something a little simpler.
>
> If we do want to patch this issue I would suggest having this done at
> construction time.
>
> var deviceready = channel.create('deviceready', channel.types.once);
> var pause = channel.create('pause', channel.types.refireable);
>
> (naming in example above is for discussion purposes only ;) )
>
> On Fri, Aug 24, 2012 at 11:37 AM, Andrew Grieve <agrieve@chromium.org
> >wrote:
>
> > I was looking through the channel code and noticed that the fired state
> is
> > never assigned to false. This makes sense for channels like
> onDeviceReady,
> > when you'd want this to stay active. But...
> >
> > It's a bit strange for things like onPause and onResume. It means that if
> > the app has ever been paused or resumed, then every new listener going
> > forward is going to fire upon subscription even though the current state
> is
> > not paused/resumed. I tested this out locally, and it appears to be true.
> > So...
> >
> > How about we add a method called "fireOnce", which calls fire() and then
> > sets fired = false so that new subscribers will not be immediately
> > triggered.
> >
>
>
>
> --
> Gord Tanner
> Senior Developer / Code Poet
> tinyHippos Inc.
> @tinyhippos
>

Re: Channel.fire() and onPause/onResume

Posted by Gord Tanner <go...@tinyhippos.com>.
I know there was some talk on the mailing list awhile ago of replacing the
channel module with something a little simpler.

If we do want to patch this issue I would suggest having this done at
construction time.

var deviceready = channel.create('deviceready', channel.types.once);
var pause = channel.create('pause', channel.types.refireable);

(naming in example above is for discussion purposes only ;) )

On Fri, Aug 24, 2012 at 11:37 AM, Andrew Grieve <ag...@chromium.org>wrote:

> I was looking through the channel code and noticed that the fired state is
> never assigned to false. This makes sense for channels like onDeviceReady,
> when you'd want this to stay active. But...
>
> It's a bit strange for things like onPause and onResume. It means that if
> the app has ever been paused or resumed, then every new listener going
> forward is going to fire upon subscription even though the current state is
> not paused/resumed. I tested this out locally, and it appears to be true.
> So...
>
> How about we add a method called "fireOnce", which calls fire() and then
> sets fired = false so that new subscribers will not be immediately
> triggered.
>



-- 
Gord Tanner
Senior Developer / Code Poet
tinyHippos Inc.
@tinyhippos