View: An utility to extend existing templates

[Reinmar]

Moved from ckeditor/ckeditor5-core#219

[oleq]

TL;DR: I decided to propose some general changes to Template–View interaction.

First of all, I think the decision we made that event listener declaration should be as brief as possible like

{
  click: 'clicked'
}

is oversimplification and mixes Template with View logic. I think it's dirty. It silently says that clicked will be fired on this (View instance), which is not always true because it could be a Model or other EmitterMixin instance instead.

The same thing happened to attribute bindings. Using View#attributeBinder.to() implicates direct binding between the Template instance and View#model. It should be up to the Template, which ObservableMixin "class" instance it listens to and what it does to the DOM.

Then, once separated View from Template and vice–versa, the extension of templates can be done easily via Template#extend. The basic, class inheritance-based template extension could look like this:

// IMO importing Template class in each View is not a huge cost.
import Template from './template.js';

class ButtonView extends View {
    constructor( model ) {
        super( model );

        this.template = new Template( {
            tag: 'button',

            attributes: {
                class: [
                    'ck-button',
                    // Move binding to Templates. It should be View-independent.
                    // It should allow binding to any model, not View#model only.
                    Template.bind.to( this.model, 'isEnabled', () => ... ) ),
                    Template.bind.if( any model, 'isOn', () => ...  )

                    // Alt syntax?
                    //      [ 'to', this.model, 'isEnabled', () => { ... } ]
                    //      [ 'if', any model, 'isOn', () => { ... } ]
                ]
            },

            on: {
                click: [
                    // Dispatching works for this View...
                    Template.dispatch( this, 'clicked' ),

                    // ...but also for any instance of EmitterMixin.
                    Template.dispatch( any emitter, 'execute' ),

                    () => { ... }

                    // Alt syntax?
                    // [ any emitter, 'clicked' ],
                ]
            }
        } );

        // I think this method name is confusing. Why not
        //      this.region( 'children', el => el );
        // or
        //      this.setRegion( 'children', el => el );
        // or
        //      this.registerRegion( 'children', el => el );
        // ?
        this.register( 'children', el => el );
    }
}

class MyButtonView extends ButtonView {
    constructor( model ) {
        super( model );

        // Extending works via existing Template instance created by the parent class.
        this.template.extend( {
            attributes: {
                class: [
                    Template.bind.to( any model, 'foo', () => { ... } )
                ],
                bar: 'value'
            },

            on: {
                click: [
                    () => { ... }
                ]
            }
        } );
    }
}

Template application to the existing DOM Element could look like this:

class NoConstructorTemplateView extends View {
    constructor( model ) {
        super( model );
    }

    init() {
        new Template( {
            attributes: {
                foo: Template.bind.to( this.model, 'isEnabled', () => ... ) )
                bar: 'value'
            },

            on: {
                click: Template.dispatch( this, 'clicked' ),
            }
        } ).apply( someDOMElement );
    }
}

[Reinmar]

Questions:

1. Why do you say that view's properties may need to me bound to other models that view.model? A view has just one model, hasn't it?
2. I guess that similar thing applies to events. Maybe it just makes sense to bind view events directly to model events, or to a simple callback?

[Reinmar]

As for regions, maybe this.createRegion() or this.addRegion()?

[Reinmar]

And generally, I like the idea about separating templates and views. I think that they should be totally separated up to a point when a template is being applied to a view.

The related changes like moving binding definitions to Template.bind make sense for me as well.

[oleq]

Why do you say that view's properties may need to me bound to other models that view.model? A view has just one model, hasn't it?

It has one model. But it may spawn sub-views in simple cases when controller is not necessary. Like IconView inside ButtonView, which has a separate VM. It may also happen that some component (like Dropdown) is a host to different kinds of children and wants to react to some changes in child VM directly. Besides, I see no reason to restrict ourselves.

I guess that similar thing applies to events. Maybe it just makes sense to bind view events directly to model events, or to a simple callback?

By binding View events to model events you make VM mandatory for event system to work. You cannot have a model–less View firing events, you got to pass an empty Model to do this. Not a good idea, IMO.

[Reinmar]

It has one model. But it may spawn sub-views in simple cases when controller is not necessary. Like IconView inside ButtonView, which has a separate VM. It may also happen that some component (like Dropdown) is a host to different kinds of children and wants to react to some changes in child VM directly. Besides, I see no reason to restrict ourselves.

But, for the sake of proper separation every sub-view should have its own view model. View's view model should be its only point of concern.

So the reason to restrict us is to keep the code clean. By forcing binding view's template to its model only (in both directions), we'll have a cleaner API (because you don't have to pass the model each time) and a more predictable code.

By binding View events to model events you make VM mandatory for event system to work. You cannot have a model–less View firing events, you got to pass an empty Model to do this. Not a good idea, IMO.

As I wrote above – I see this as a big advantage. Most likely views will be creating their view models (as mentioned in https://github.com/ckeditor/ckeditor5-ui/issues/26) and those models should be the only entry point to the views.

[oleq]

But if you use

foo: Template.bind.to( 'isEnabled', () => ... ) )

instead of

foo: Template.bind.to( this.model, 'isEnabled', () => ... ) )

then the View must make sure the whole thing is bound to View#model. It mixes the logic of View and Template and creates a 2-step process: create instance of Template first, then activate bindings by View, which is what we have now and I don't like it. Besides, in such case any further extension with template.extend() must go through the View at some point and activate new VM bindings. It's a mess.

I suppose you'd like to see something like that:

// In View#constructor()
this.template = {
  attributes: {
    foo: view.bind.to( 'isEnabled', () => ... ) )
  }
}

which then internally is processed into a Template:

// In View#init()
const tpl = new Template( this.template );

for ( let a in tpl.attributes ) {
  Template.bind.to( view.model, tpl.attributes[ a ] );
}

but it's only a syntax sugar. It creates additional layer of abstraction which I think is unnecessary.

[Reinmar]

I'm not sure I agree, so I'll write generally how I see it. Curious if that's doable.

So, I totally understand that template should be totally detached from the view. I support that.

However, template must know about the view model, right? You propose to pass the model explicitly to each bind and dispatch() (*). I'm proposing passing it to the template earlier and only once. Like:

constructor() {
    const bind = new Binding( this.model );

    // later in some template:
   foo: bind.to( 'foo' )
}

What's the difference between passing model once to the whole templating mechanism and passing to each of its methods?

(*) I don't like this name... Can't we also bind events? Like... property change trigger other property change (and we call this a binding) and an event fired on the view triggers event on a model (why not calling this a binding as well?).