vrimar / construct-ui

A Mithril.js UI library
https://vrimar.github.io/construct-ui
MIT License
287 stars 23 forks source link

Passing lifecycle event handlers to nested elements #3

Closed abidingotter closed 5 years ago

abidingotter commented 5 years ago

Our scenario is that we have a control that is in read-only mode by default, but when clicked will render a construct-ui Input component instead. We want to set the focus to the DOM input element inside the Input component (the Input renders a div at the top level).

Here is an example of what I mean (I have used a very very cut down version of the Input component to simplify matters) https://jsfiddle.net/wz1250ma/

It works, but the oncreate is fired for the div as well as for the input, since the Input component passes any unrecognised properties onto the input element.

I wonder if there is any 'clean' way of separating the lifecycle event handlers for the actual component vs the elements nested inside it? E.g. if I could specify oncreate handler for the Input separately from the input tag inside it. By 'clean' I mean some way that doesn't make the API ugly and hard to use.

vrimar commented 5 years ago

There are two solutions I can think of.

One is having an explicit inputAttrs prop that gets passed to the underlying input element. Not entirely ideal since it would be a breaking change. The current API however, is slightly confusing since it's not immediately clear which attrs are being passed to the container element vs the input element.

Second solution is simpler, just destructure all the life cycle methods in the Input component view like so:

public view({ attrs }: m.Vnode<IInputAttrs>) {
    const {
      basic,
      class: className,
      contentLeft,
      contentRight,
      disabled,
      fluid,
      intent,
      size,
      style,

      // Avoid calling lifecycle methods
      oninit,
      oncreate,
      onupdate,
      onbeforeupdate,
      onbeforeremove,
      onremove,

      ...htmlAttrs
    } = attrs;

This way the life cycle methods are only called on the underlying input element.

Again, this might be too confusing and an explicit inputAttrs prop might be better. What are your thoughts?

abidingotter commented 5 years ago

If I understand it correctly, the second solution would prevent the lifecycle callbacks from being passed to the actual input element though wouldn't it? I guess that is ok, as long the Input component triggers my oncreate handler, which it would, I can find the underlying input element in the vnode.dom.children and the event only happens once.
Maybe that is the preferable solution.

vrimar commented 5 years ago

Yeah, sorry for the confusion, I meant they aren't called on the underlying input element. I'll add this in the next version (v 0.1.11)

vrimar commented 5 years ago

Fixed in v0.1.11.