nteract / vdom

🎄 Virtual DOM for Python
https://github.com/nteract/vdom/blob/master/docs/mimetype-spec.md
BSD 3-Clause "New" or "Revised" License
222 stars 35 forks source link

Updating the Event Spec #97

Closed rmorshea closed 5 years ago

rmorshea commented 5 years ago

Right now event handler information is serialized into an object of the form:

{
    "{target_name}": "{hash}_{event_name}"
}

However encoding all the event information into a string is a bit limiting and probably leftover from when event handlers were part of the attributes field rather than eventHandlers. To improve this situation it seems like it would be better for event information like the hash to be expressed in an object:

{
    "{target_name}": {
        "hash": string,
        "eventName": string,
    }
}

This is more straightforward to parse on the client-side, and also enables extra options to be passed. For example, adding the ability to prevent the default event action or stop event propagation:

{
    "{target_name}": {
        "hash": string,
        "eventName": string,
        "stopPropagation": boolean,
        "preventDefault": boolean
    }
}

Additionally the "target_name" is described as being the ID of the comm channel the client should connect to when responding with events. However right now vdom and Nteract's vdom-transform actually treat this field as if it were an event name.

Should the documentation be updated to reflect this? If so, we could further simplify the event spec as follows:

{
    "{eventName}": {
        "hash": string,
        "stopPropagation": boolean,
        "preventDefault": boolean
    }
}

And if there is a need for a target name at some point down the road we can always add a targetName field to the event handler object.

rmorshea commented 5 years ago

cc: @gnestor @rgbkrk

gnestor commented 5 years ago

Actually event handlers look like:

{
  "{event_type}": "{hash}_{event_type}"
}

So a vdom object with an event handler might look like:

{
  "tagName": "input",
  "attributes": {
    "placeholder": "type something",
    "type": "text"
  },
  "eventHandlers": {
    "onChange": "282935650_onChange"
  },
  "children": []
}

The value (or the {hash}_{event_type}) is used as the comm target name. It doesn't include any other metadata because there hasn't been a need for it. It's true that it's not possible for a vdom (Python) user to call any methods on an event object, like event. preventDefault because they only receive the serialized event data from vdom. We could enable this, but what would the vdom user interface for this look like?

button('click me', onChange=handle_event, preventDefault=True)

I'm happy to explore how we can make the interactive part of vdom (vdom widgets) more powerful, but I also want to think carefully about how we change vdom's user interface. What's really nice about vdom UI is that it resembles the DOM's API about as closely as possible (given that it's operating in Python and has no handle on the user's current DOM). If you haven't played with Pyodide yet, I highly suggest that you do because in Pyodide you get a full handle on the DOM and can call any method you want on event as well as document, window, etc.

https://alpha.iodide.io/notebooks/2569/

rmorshea commented 5 years ago

Looks like the spec just needs to be updated to reflect that events should actually be of the form you described. Right now event-spec.md shows the {target_name} being the key for the event handler mapping.

As far as how to incorporating other things into the event spec without breaking the user interface I'd suggest adding an EventHandler object which can be used via a decorator:

@event_handler(preventDefault=True)
def my_handler(event):
    # do stuff...

button('click me', onChange=my_handler)

The decorator would wrap the function in an EventHandler object so that when you're creating the VDOM you just need to check whether the callback is a simple function or if it's an EventHandler.

gnestor commented 5 years ago

Looks like the spec just needs to be updated to reflect that events should actually be of the form you described. Right now event-spec.md shows the {target_name} being the key for the event handler mapping.

Ah, I see. I will submit a PR to fix that now.

As far as how to incorporating other things into the event spec without breaking the user interface I'd suggest adding an EventHandler object which can be used via a decorator:

We could certainly add something like that. That doesn't complicate the API, it just makes it possible to do more sophisticated event handling. Are there any other event properties of than preventDefault and stopPropagation that you would like to see this support?