Move event handlers out of attributes

[mpacer]

The issue that came up around #69 is making me think. Would it make sense to modify the vdom spec to actually encode callbacks not as a type of attribute, but as a separate field entirely?

Event handlers (specifically their targets) do have a pretty specific purpose that isn't equivalent to a standard (static, serializable) attribute. Other than this all of the attributes that we passing up the wire providing are standard html/css attributes too.

The downside is it would require changing the message spec expected by front ends… of course that's why we're versioning our messaging schema… so maybe that's ok.

@gnestor @rgbkrk Do you have any thoughts?

[rgbkrk]

Would it make sense to modify the vdom spec to actually encode callbacks not as a type of attribute, but as a separate field entirely?

Oh my gosh that's a great idea.

Then we'll have a minor version change to the spec that can be ignored by current frontends and is adaptible by future implementations. Love it @mpacer!

[rgbkrk]

Do we want to call them dynamicAttributes or something about comm registration? I like that this will make it explicit which fields to ignore for the html representation.

[gnestor]

This sounds fine to me 👍

To clarify, would the user experience still be the same:

my_button = button('Click me', onClick=handle_click)

[rgbkrk]

It seems like something where we'd special case the handling in python to make the interface nice n' easy for pythonistas, while the internal storage and output format differ.

[gnestor]

Ok, I'm thinking through this and would appreciate some feedback:

Assuming that the UX is the same:

my_button = button('Click me', onClick=handle_click)

First, we would pop event handlers out of attributes. In self.encode_attributes, we are doing:

attributes = {}
for key, value in self.attributes.items():
    if callable(value):
        attributes[key] = create_event_handler(key, value)
    else:
        attributes[key] = value
return attributes

So, if an attribute is a function, we consider it an event handler. We could also create a whitelist of event handler attribute names and filter them out that way. I like the former because it's simpler. If vdom supports React components one day with arbitrary callback props like onActiveItemChange then this would still work.

This would probably go in from_dict because that's where we're doing the same from style:

attributes = value.get('attributes', {})
style = None
event_handlers = {}
if 'style' in attributes:
    # Make a copy of attributes, since we're gonna remove styles from it
    attributes = attributes.copy()
    style = attributes.pop('style')
for key, value in attributes.items():
    if callable(value):
        event_handlers.update(attributes.pop(key))
return cls(
    tag_name=value['tagName'],
    attributes=attributes,
    style=style,
    event_handlers=event_handlers,
    children=[VDOM.from_dict(c) if isinstance(c, dict) else c for c in value.get('children', [])],
    key=value.get('key')
)

Lastly, we need to handle event handlers in self.to_dict. For self.style, it's just merged back into attributes. Would we do the same for self.event_handlers? It sounds like we're talking about modifying the message spec in which case eventHandlers might be a top-level property:

{
  "tagName": {
    "type": "string" 
  },
  "children": {
    "$ref": "#/definitions/vdomNode"
  },
  "attributes": {
    "type": "object"   
  },
  "eventHandlers": {
    "type": "object"   
  },
  "key": {
    "oneOf": [{"type": "number"}, {"type": "string"}, {"type": "null"}]
  }
}

[rgbkrk]

eventHandlers would definitely be a top level property. The spec for an eventHandler though is just a mapping of string to string right? Since we're talking onClick -> target_name right?