onfire icon indicating copy to clipboard operation
onfire copied to clipboard

Stopping an event from climbing is not working as expected

Open Orion98MC opened this issue 15 years ago • 4 comments

Let's say "Doug" is parent of "Mick" who is a parent of "Joe" :) joe.fire(:starving) # He is 5 years old ... Mick wants to intercept :starving and also fire(:diner_is_ready) when the diner is ready. Here is an irb log of what it could look like:

$ ./script/console 
Loading development environment (Rails 2.3.8)
>> require 'vendor/plugins/onfire/lib/onfire.rb'
=> []
>> 
?> class Doug;include Onfire;attr_accessor :parent;def initialize;on(:starving) {puts "What ???"};on(:diner_is_ready){puts "Ok great!";};end;end
=> nil
>> class Mick;include Onfire;attr_accessor :parent;def initialize;on(:starving){|event| puts "Stop whining!";event.stop!};end;end
=> nil
>> class Joe;include Onfire;attr_accessor :parent;end
=> nil
>> 
?> doug = Doug.new
=> #<Doug:0x1035920f0 @event_table=#<Onfire::EventTable:0x103591fb0 @source2evt={nil=>{:diner_is_ready=>[#<Proc:0x00000001035aa420@(irb):3>], :starving=>[#<Proc:0x00000001035aa5d8@(irb):3>]}}>>
>> mick = Mick.new
=> #<Mick:0x10358f0d0 @event_table=#<Onfire::EventTable:0x10358ef90 @source2evt={nil=>{:starving=>[#<Proc:0x000000010359b470@(irb):4>]}}>>
>> mick.parent = doug
=> #<Doug:0x1035920f0 @event_table=#<Onfire::EventTable:0x103591fb0 @source2evt={nil=>{:diner_is_ready=>[#<Proc:0x00000001035aa420@(irb):3>], :starving=>[#<Proc:0x00000001035aa5d8@(irb):3>]}}>>
>> joe = Joe.new
=> #<Joe:0x103588f28>
>> joe.parent = mick
=> #<Mick:0x10358f0d0 @parent=#<Doug:0x1035920f0 @event_table=#<Onfire::EventTable:0x103591fb0 @source2evt={nil=>{:diner_is_ready=>[#<Proc:0x00000001035aa420@(irb):3>], :starving=>[#<Proc:0x00000001035aa5d8@(irb):3>]}}>>, @event_table=#<Onfire::EventTable:0x10358ef90 @source2evt={nil=>{:starving=>[#<Proc:0x000000010359b470@(irb):4>]}}>>
>> 
?> joe.fire(:starving)
Stop whining!
=> nil
>> 
?> class Mick;def diner_is_ready;on(:starving) {fire(:diner_is_ready);};end;end
=> nil
>> 
?> mick.diner_is_ready
=> [#<Proc:0x000000010359b470@(irb):4>, #<Proc:0x0000000103578ab0@(irb):15>]
>> joe.fire(:starving)
Stop whining!
=> nil

Expected behavior:

The fire(:diner_is_ready) should bubble to doug.

Patch:

$ diff ORIGINAL/onfire/lib/onfire.rb PATCHED/onfire/lib/onfire.rb 
25c25
<     parent.bubble_event(event)

---
>     parent.bubble_event(event) unless event.stopped? # DISCUSS TP: It's stopped in #process_event, we don't let it climb
30c30,31
<       return if event.stopped?

---
>       # return if event.stopped?
>       # DISCUSS TP: It leaves a chance to the local handlers to stop the event BUT also do local stuff

Orion98MC avatar Aug 22 '10 08:08 Orion98MC

So, your requirement is that if an instance has multiple handlers for the same event, and and one of these stops the event, the remaining handlers on that instance should be executed nevertheless?

Well, that's not the way I'd expect it!

From another perspective, you just have a couple of handlers in a chain:

A --> B --> C --> D

Now if B stop!s the event, the handler chain will stop. Isn't that the way it should work? It's not a "stop bubbling!" but more a "stop penetrating the remaining handlers!".

What do you think?

apotonick avatar Aug 23 '10 16:08 apotonick

I think It's not that obvious.

Onfire is much like the responder-chain for ruby. In a responder chain the only decision that is taken is "should the event climb".

If one adds many event handlers for the same event it's because it's clearer (more readable) than making a big (obfuscated) event handler method to handle many response-actions. (also the responds_to_event is a single action method in Apotomo and it's handier to map an event to one state of a widget than making proxy methods each time). But still, the event handlers for one event in an object context should be treated as ONE big SET of things to do IN RESPONSE to the event.

It's more like a unordered set of handlers than an array of ordered handlers.

Humanly speaking it' more like:

  1. When event :A is received I will do "all of this". Which is quite different than:
  2. When event :A is received I will do 'this" and then maybe do "that" etc...

1 is less error prone in coding. If you want to sync things and put conditions well do it yourself.

Now the #stop! command. In a "responder-chain-like" the objects participating to the chain have very few things to do. They do there stuff when the event occurs, and then if they want to propagate the event, they call super (which makes the event climb). In onfire, the "call-to-super" is implicit (I know onfire is not a class but a module), so there should be a way to prevent the climbing of event... it's the #stop! method.

If the #stop! command is not a climbing-ONLY flag it means that you are making decisions to impose an order in an unordered SET of handlers, which makes no sens and leads to confusion.

Also, if you wish to stay with this behavior then there should be some methods to insert new handlers at a particular index of the "array" of handlers. But i remember when you said ... "onfire should be dumb and small... that's what great tools are" so ...

Orion98MC avatar Aug 24 '10 13:08 Orion98MC

Interesting, in jQuery, there are two different stop methods: stopImmediatePropagation is our present #stop!, whereas stopPropagation is what you want.

apotonick avatar Aug 24 '10 23:08 apotonick

So I guess it's useful to have both :)

Orion98MC avatar Aug 27 '10 11:08 Orion98MC