Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Again) Concrete architecture for signals/events/connect/bindings #1583

Open
rickmcgeer opened this issue Jun 30, 2024 · 2 comments
Open

(Again) Concrete architecture for signals/events/connect/bindings #1583

rickmcgeer opened this issue Jun 30, 2024 · 2 comments
Labels
📟 support inquiry Questions that can be answered without changing code

Comments

@rickmcgeer
Copy link
Collaborator

What would you like to achieve?
This architecture (despite years of programming with it) is still not clear to me, and even if (as I suspect is the case) there is a clear architecture underneath, it isn't well-enough explained or articulated. I may not be the quickest or most acute study, but I believe I'm not the slowest, either, so if I'm having problems others will, too.

I just spent a couple of days debugging signals not being propagated from

$world.execCommand("open browser", {moduleName: "studio/inputs/slider.cp.js", packageName: "engageLively--galyleo-dashboard", codeEntity: "DoubleSliderModel"});

to

$world.execCommand("open browser", {moduleName: "studio/inputs/slider.cp.js", packageName: "engageLively--galyleo-dashboard", codeEntity: "DoubleSliderWithValuesModel"});

After much experimentation, it turned out that the fix was to add the rangeChanged event to the expose list in DoubleSliderModel.

But this still left me with questions:

  • In DoubleSliderWithValuesModel, using model in the binding for rangeChanged didn't work, but target did. Should I have used the model name and model? If so, what name should I use (since binding is by name, not object)
  • As I understand bindings, the fields are:
    -- model: model of the object generating the event
    -- target: name of the object generating the event
    -- handler: function to call to handle the event
    Is this correct?
  • Since bindings is a static property, how does one handle the case where there are multiple items which generate events which are added dynamically? A good example of this is a non-static list.
  • Similarly, how does one dynamically connect to an event (e.g., list selection)?

How are you trying to achieve that
The inputs and helpers classes in https://github.com/engageLively/galyleo-dashboard/tree/main/studio try basically all of the above, as well as connect (and I don't understand that one well, either). They're something of a mess; in part, that's me, but in part it's also due to the fact that the architecture here is really unclear to me. Filters, for example, don't use the view/model architecture but use Morphs, and so they use connect a lot. See:

$world.execCommand("open browser", {moduleName: "studio/filters.cp.js", packageName: "engageLively--galyleo-dashboard", codeEntity: "BooleanFilterMorph"});

Alternative solutions
This is primarily a plea for documentation/examples/cookbook, and a lot of those are already in Lively. What we (principally @merryman) did for Galyleo was to take the studio components from lively.ide and adapt them. This actually gives a pretty good start. Here is what I'd recommend:

  • The Galyleo UI objects subclassed Lively UI components that themselves changed dramatically. That was the risk of being an early adopter, and all in it paid off. In future, people (including me) should build on a standard, well-documented toolkit, which will have components like
    • Lists
    • Buttons (push, toggle, radio)
    • Menus (OK, really a list of buttons, but still...)
    • Sliders (realistically, a minor UI element, that I overuse)
    • TEXT!! In of course multiple forms
  • A standard, well-documented way to react (to coin a phrase) to UI events. And it should be based on high-level, semantic events associated with each widget, not roll-it-from-the-ground up mechanisms. I get signals are very general; but they are also very basic. Frankly, my list interface ought to be at the level of new selection; button, an action on press; etc.
  • One of the strengths of Lively is the flexibility of the low-level interface, so I'd recommend building on top of the existing mechanism, not replacing it. That way, when people want to dive deeper and do things that aren't easy with the high-level interface, they can.

Here's one idea:
Each UI element offers a set of events. An event is an object which offers the following methods:

  • subscribe(handler): A handler is an object with a handleEvent(obj) method. handler.handleEvent(event) gets called on each new event
  • unsubscribe(handler): remove handler from the list of handler objects for this event
  • subscribers: a read-only property (no set method) for the subscriber list
  • notifySubscribers(obj): call handler.handleEvent(obj) for each handler on subscribers

The obj argument to handleEvent is event-specific; it captures data specific to the event. The existing evt construct, which we use for mouse events, might be re-used for this (in general, I'd like to add as little new as possible)
Implementing this on top of the existing signal architecture gives the following notional methods, based on a LivelyEvent class with the methods given above.

A simple example of this is given with the current action property of ButtonModel, and we have this method:

trigger () {
    try {
      signal(this.view, 'fire');
      typeof this.action === 'function' && this.action();
    } catch (err) {
      const w = this.world();
      if (w) w.logError(err);
      else console.error(err);
    }
  }

(see

$world.execCommand("open browser", {moduleName: "buttons.js", packageName: "lively.components", codeEntity: "trigger"});

Except with events, action is replaced with a list of subscribers

init() {
   ...
  this.events.action = LivelyEvent();
  '''
}

trigger() {
   signal(this.view, `fire`);
   this.events.action.notifySubscribers(null); // no data to pass for a button press
}

The defensive code is gone. Checking if there's really an action is gone, because the event and its associated subscriber list was done on init, and logging an exception in the handler.handleEvent code is done in notifySubscribers

Additional Resources
The Galyleo code could be a guinea pig for this. I'd do most of the implementation but I'd like to work with @linusha and @merryman as well, since they understand what exists much better and also for figuring out how we could turn this into a reusable toolkit

Version: Please paste the lively.next commit on which the problem occurred here (use the copy button of the Version Checker in the bottom left corner).
130b086

@rickmcgeer rickmcgeer added the 📟 support inquiry Questions that can be answered without changing code label Jun 30, 2024
@merryman
Copy link
Member

merryman commented Jul 1, 2024

This architecture (despite years of programming with it) is still not clear to me, and even if (as I suspect is the case) there is a clear architecture underneath, it isn't well-enough explained or articulated. I may not be the quickest or most acute study, but I believe I'm not the slowest, either, so if I'm having problems others will, too.

The architecture of bindings is still not final and changing, so we are thankful for any feedback like this one.
Now we are currently in the process of relaunching the lively.next website, and it will come with comprehensive explanations with regards to view models, bindings, the IDE and other stuff. So please be patient.

In DoubleSliderWithValuesModel, using model in the binding for rangeChanged didn't work, but target did. Should I have used the model name and model? If so, what name should I use (since binding is by name, not object)
As I understand bindings, the fields are:
-- model: model of the object generating the event
-- target: name of the object generating the event
-- handler: function to call to handle the event
Is this correct?

While this is correct, we have recently decided to deprecate the model and will soon drop support for it. The reason is that binding to models causes extremely confusing scenarios like the one you entered. I suppose the reason here was, that the signal itself was triggered on the view (and therefore target) and not on the model itself.
I also do not want to rule out typos that can happen easily, since this is a soft binding and will not throw errors if you are defining bindings based on nonexistent method or signal names.

Since bindings is a static property, how does one handle the case where there are multiple items which generate events which are added dynamically? A good example of this is a non-static list.

Good question. While the property itself is static, the underlying connections are dynamically instantiated and also updated if submorph structure changes. If you want to define a binding for multiple morphs, you can use regular expressions for the name of the target which will apply the binding to all morphs where the name matches the regular expression. For instance if I had multiple morphs which all should receive a certain binding, I would 1.) ensure that the names follow a certain scheme like: "item 1", "item 2", "item 3" ... 2.) define the binding like this: { target: /item/, signal: 'position', handler: 'reactToMove' }.

Similarly, how does one dynamically connect to an event (e.g., list selection)?

Not entirely sure what you mean by that, but since the bindings are dynamically updated this should be covered?

About the rest of your proposal, this seems more like a revision to lively.bindings? So you would revise lively.bindings in a way where the information about signals is explicitly modeled via objects instead of the implicit signal.
We can certainly think about that, but to me that is a separte issue to the bindings problem earlier in the ticket.
Nice thing about this is:

  • The explicit modeling of actions/signals allows them to be dynamically checked easily.
  • Also simplifies building tooling for visual connection definitions since tooling can dynamically check the presence of actions inside the event object.

Bad things:

  • Requires an explicit preparation for signals within the code, for instance connect can also connect to method calls that where not specially prepared in that way.
  • Overall is more verbose than just using signal() and connect().

@rickmcgeer
Copy link
Collaborator Author

rickmcgeer commented Jul 1, 2024

first, thanks for the detailed and thoughtful response. It looks like I'll be changing my code yet again :-). (This is really no problem as long as I understand the architecture!)

I also do not want to rule out typos that can happen easily, since this is a soft binding and will not throw errors if you are defining bindings based on nonexistent method or signal names.

Right, that's possible (and it's very hard to debug!). This is why bindings based on object references are more robust, particularly when there is a discovery mechanism (e.g., the subscribers property I proposed). The graphic connection lines from the previous sidebar were an enormous help.
In general, binding by name in all circumstances is challenging for debugging, because the name resolution happens through an opaque process at runtime. I appreciate it also adds dynamism, because changing a name changes a target

Not entirely sure what you mean by that, but since the bindings are dynamically updated this should be covered?

yes, bindings updated dynamically covers this.

About the rest of your proposal, this seems more like a revision to lively.bindings?

No, the pub/sub architecture isn't a revision to lively.bindings, or to any existing Lively code! It is an overlying convenience architecture on bindings/signals/connections, which would be implemented using the existing architecture (as the code snippets I put in were). In fact, Lively already uses a stub of this architecture in Button, where there is an action property; where the action is non-null and a function, it's called simultaneously with the issuance of the fire signal.
The generalizations to action are:

  1. Used in multiple UI objects, not just buttons
  2. Multiple potential handlers, not simply a single function embedded in a property
  3. Since the handler is an object (and not a function) the closure around the function is given

But notice that action doesn't change signal() and connect(), and the signal/connect interface is available to programmers. I'll give you another "bad thing" about the pub/sub architecture I proposed: it isn't as general and flexible as signal() and connect(), and sometimes that flexibility and generality is required. The nice thing is that we can have both, and the pub/sub architecture provides:

  1. A familiar interface to UI designers and programmers generally; lots of UI toolkits work this way
  2. A transparent connection and debugging mechanism
  3. A way to illustrate/provide a gentle intro to the connect/signal architecture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📟 support inquiry Questions that can be answered without changing code
Projects
None yet
Development

No branches or pull requests

2 participants