search

ckeditor / ckeditor5-design

☠ Early discussions about CKEditor 5's architecture. Closed now. Go to https://github.com/ckeditor/ckeditor5 ☠
58 stars 12 forks source link

MV* prototype #10

Closed gregpabian closed 7 years ago

gregpabian commented 10 years ago

On branch mvc-samples I added some usage examples of the MV* framework prototype I've been working on recently. This is an "umbrella" issue to discuss them and the whole idea.

jodator commented 10 years ago

So I'll drop few lines after quick lookup.

For now I really like how models work - I don't know how other MV* frameworks does it (besides Backbone) but not having verbose setters and getter is very nice.

On the other hand templates look weird to me. Maybe because I'm used to string templates and this is something different? I don't feel how they should work and how to build something more complex, ie multiple nested divs with some content (ie image with caption and some overlay span with data).

Anyway the the model-view cooperation is working great plus great to have two-way binding.

mlewand commented 10 years ago

Some notes:

Preventing Events

Is there any way to cancel the events right now?

Suggestion: I think that we might consider using in CKE something like preventDefault in DOM for our events, which would mean that developer wants CKEditor to not fire default handling for the event.

Reasoning: I'm missing it a lot in CKE4, because now you have two solutions:

Common scenarios:

Overall Impression

A very good step forward, I can see that this proposition gives some powerful features and it's looking promising.

I would love to get much deeper into it, but some parts requires deep analysis and I already gave it considerable time, while A11ychecker timer is ticking. :P

I think we should prepare an integration sample, with Editor object utilizing PluginManager. It would load like 3 plugins, and then we would see how does it works, and most importantly "feels". : )

@jodator Templates are looking weird for me but after inspecting the internals I feel like they are very flexible, so i'm excited about it. I'm not used to such syntax, but all what worries me at this point. : D

gregpabian commented 10 years ago

@jodator, @mlewand regarding a View's template - it's something more than just a DOM builder - a template property allows you to define a DOM structure but also to define all the bindings in one place. The problem with string-based templates is HTML parsing, which takes some time. While working with Backbone I had to do a lot of hacking to build a complex HTML structure, render it and bind all the events in a reasonable period of frame and I hope that working on DOM elements should help us avoid this problem. And regarding the syntax - it's inspired by JsonML a bit, but with several modifications.

@mlewand

bob.helpers.bindProp - is there any reason to reduce mutator only to object members? It seems to limit the functionality, I would allow a function to be passed to.

Sounds reasonable, I'll add this feature.

what does the bob stands for?

I was thinking how to name it properly since it's not just a DOM builder, nor it is a data binder, so bob stands for Binder & Builder ^_^ (I was also considering "bnb", but "bob" sounds better). Still, ideas are welcome.

I don't like the fact that types are packed so tightly :c eg. MVC.Model, MVC.Space, MVC.PluginManagerMixin, MVC.Application specified in a single file doesn't sound like any fun to me.

It could be moved to separate files - no problem with that - just my laziness ^_^;

isn't change event supposed to occur after the change: event?

I agree, we should start from specific handlers and then move to general ones.

Is there any way to cancel the events right now?

It's not possible right now, but could you give me a real life example for that? (I don't know CKE4 good enough to name some problems with events in the current version...)

gregpabian commented 10 years ago

@mlewand The PluginManager is another big thing we're working on right now, but it's gonna take some time to ship it since it has a lot of impact on how the whole architecture and build process works.

fredck commented 10 years ago

@jodator (https://github.com/jodator), @mlewand (https://github.com/mlewand) regarding a View's template - it's something more than just a DOM builder - a template property allows you to define a DOM structure but also to define all the bindings in one place. The problem with string-based templates is HTML parsing, which takes some time. While working with Backbone I had to do a lot of hacking to build a complex HTML structure, render it and bind all the events in a reasonable period of frame and I hope that working on DOM elements should help us avoid this problem. And regarding the syntax - it's inspired by JsonML a bit, but with several modifications.

Let me add that the object model of the template makes it easy to make modifications to it, like changing element names or adding attributes to them. See my recent changes to 5.js for an example of an extension to an existing template.

what does the bob stands for?

I was thinking how to name it properly since it's not just a DOM builder, nor it is a data binder, so bob stands for Binder & Builder ^_^ (I was also considering "bnb", but "bob" sounds better). Still, ideas are welcome.

bob is a funny name… we can justify it as “Bindings on Builder” ;)

I don't like the fact that types are packed so tightly :c eg. MVC.Model, MVC.Space, MVC.PluginManagerMixin, MVC.Application specified in a single file doesn't sound like any fun to me.

It could be moved to separate files - no problem with that - just my laziness ^_^;

True. We may have a single “mvc” folder with each class separated.

jodator commented 10 years ago

@gregpabian Thanks for rationale - I suspected that speed was the case for this. Anyway I'd like to see more complex examples of DOM structure to see how they can work

Also it can be Bob the DOM builder ;-)

@mlewand @fredck @gregpabian I like the sexual innuendo with bob, so it could stay ;)

pjasiun commented 10 years ago

Cool. Binding is cool. And is seems to be not over-complicated (yet). But I am very curious how it will work in the real application, I mean many concepts are cool when you have 1000 LOC and no real problems.

The one think I do not like is an array as an template and ending up with:

IconLinkButton.prototype.template[ 1 ].children.unshift( //...

As I understand the first element is the type of the element and the second are properties. Why name can not be one of the properties?

gregpabian commented 10 years ago

Why name can not be one of the properties?

It's a matter of readability - compare:

template: [
    'a', {
        className: Button.bindProp( 'model.active', 'getActiveClass' ),
        onclick: 'click',
        href: 'javascript:;',
        children: [
            [ 'span', {
                text: Button.bindProp( 'model.text' ) 
            } ]
        ]
    }
]

with this:

template: {
    tag: 'a',
    className: Button.bindProp( 'model.active', 'getActiveClass' ),
    onclick: 'click',
    href: 'javascript:;',
    children: [
        {
            tag: 'span',
            text: Button.bindProp( 'model.text' )
        }
    ]
}

In the first example it's easier to tell what element you're in, besides it allows us to use some shorthands, e.g.:

[ 'span', Button.bindProp( 'model.text' ) ]

instead of:

[ 'span', {
    text: Button.bindProp( 'model.text' ) 
} ]
pjasiun commented 10 years ago

I still prefer second version. Especially when you will need to access some properties:

template.children[ 0 ].tag

is in my opinion more readable then:

template[ 1 ].children[ 0 ][ 0 ]

And I think we will access/merge templates a lot.

adelura commented 10 years ago

True. We may have a single “mvc” folder with each class separated.

I suggest to make standard for it. Each namespace should be represented by directory with the same name as namespace (so here we create MVC directory). And each constructor should be represented as single file with the same name (so in MVC directory we got file Model.js).

While current template implementation is a bit tricky. I suggest to make some improvements (this is just prototype):

https://gist.github.com/adelura/dd86f474697626ee4f26

What I did:

Pros:

Cons:

Reinmar commented 10 years ago

##################################################

I wrote an email to the team few months ago comparing the DOM builder and HTML templates. Some of you couldn't read it then, so quoting myself:

##################################################

Hi,

I made a research and have some I think very interesting findings.

Web

First of all, some blog posts.

I don't encourage you to read all that. I haven't done it myself (except the last one). Just scanned for some most important notes. It doesn't make sense to rely on these articles, because our case is significantly different than this of single page apps. We build DOM once, we keep templates inside JS, so we use JS logic (e.g. loops and conditions), we build UI and besides that create many small elements across the entire code base, etc.

Testing

I found many innerHTML vs DOM builders performance tests, but they were poor implementations and were missing the point (especially, our point).

First I implemented this test: http://jsperf.com/dom-builder-vs-innerhtml/2 One uses HTML string (taken directly from our UI), but no logic and no concatenation, second uses simple and naive DOM builder. There are two pairs of tests in fact - one checks 10x repeats of the template second floods DOM with single instances of layout. You'll see in the results why I separated these cases.

Then I implemented an improved version: http://jsperf.com/dom-builder-tpl-vs-innerhtml/2 I wanted to focus on the "repeating" and "templating" aspects. In the DOM builder I create a template DOM structure which is then cloned and data is filled in remembered places. Cloning is much faster than building from scratch and you can see that in the results.

As you can see in the results (if you can't see use FF instead of Chrome, sic!) the second implementation is incomparable with the naive one. Also, the loop case gives different results than the single element creation case. This means that innerHTML has long starting time, most likely because of the parser initialization. I'll get to that later.

I also created a version of test which is more close to createFromHtml vs not using it - http://jsperf.com/dom-builder-vs-innerhtml/3

What's our case?

Then I asked myself - what we actually do now.

Advantages from using DOM builder

Doubts

Disadvantages

Conclusions

tl;dr I'm for using DOM builder in CKE5. It has many advantages over HTML strings, it may improve performance in many cases and should not break it in others.

  1. DOM builder may be optimized (e.g. className&style instead of setAttr), it may be used differently, because we may attach listeners differently, we may not need some ids, etc. When innerHTML usage can't really be improved.
  2. Modern browsers (IE11 doesn't count apparently) have faster DOM implementations than parsers. We can expect that IE will join to this group. Additionally, I expect that DOM API performance will improve better with time than parser performance.
  3. I chose a template with dozen of attributes and listeners. In all cases when we create simple node with few of properties, DOM builder gains.
  4. Parsing HTML does not take now a lot of time, so the decision whether we should using builder or HTML strings will not affect performance significantly. However, it may affect architecture and in this case it will make it better.
  5. It may be possible to write builder templates in a way that they will be well minificable. No spaces between atts, no closing tags, tags name may be replaced by function names, so will be minified too.
  6. Using DOM builder opens many new possibilities, like partial undoing (important for MathJax like cases).

Sh**y conclusion about performance testing

CKEditor loads on Chrome so well comparing to IE and Firefox, that the results I get from its awesome dev tools are completely useless for general view. I was trying to use FF's profiler, but it doesn't seem to work at all. We'll have to check if Firebug does any better. And also check IE.

Reinmar commented 10 years ago

High-level stuff

Disclaimer: Since I didn't work on the prototype and I don't know the things that you discussed but which aren't reflected in the prototype's code, my comments may explain what you already know and/or skipped intentionally. Also, I know that you might be focused on the data binding and templates only, so my comments about MV* in general may exceed the scope of this prototype. However, to prevent the situation that something hasn't been said and then it's too late, I'm writing everything down.

The current prototype does not meet the two important requirements for the MV* architecture that we need:

Why isn't the prototype meeting these requirement?

  1. The view contains application logic. It acts on UI events and modifies the model.
  2. Everything in the model is fully visible. There's no encapsulation at all.
  3. The controller only initializes the view and model. The model is totally empty. That means that all the rest is implemented by the view.
  4. The API which view must implement is not defined (because the view does everything).
  5. The APIs which view may use are well defined - it's the model attributes. That's good, because it's clear how other views can be implemented, but since the model API is defined by the specified attributes... the view can actually do everything (encapsulation again).

How to solve these issues? The first thing to notice is that the model proposed in the prototype is actually only a bidirectional data binding. So this is the view model (VM) from the MVVM pattern which Fred was exploring. VM should not contain any logic and this is true now - it is just a set of attributes with plain setters, getters and events when something changes.

Besides the VM a component must have a model (M). The model implements the tricky logic which assures the validity and integrity of the data. Perfect example of the need of having a well defined model was the model in the autocomplete plugin which:

All properties are readonly and can only be changed by the methods what clearly defines the API. There are, however, examples of very simple models with no logic like this. In such cases there should be an easy way to define all those setters. For instance, we could have a generic setter and a list of model properties which may be changed by it:

Model.prototype = {
    defineProperty: function( name ) {
        Object.defineProperty( this, name, {
            enumerable: true,

            get: function() {
                return this._properties[ name ];
            },

            set: function() {
                throw new Error( 'Violation!' );
            }
        } );
    },

    set: function( name, value ) {
        if ( name in this.setters )
            this._properties[ name ] = value;
    }
};

SomeModel.prototype = {
    setters: { title: 1, height: 1 },

    setQuery: function( queryText, queryRange ) {
        this._properties.queryText = queryText;
        this._properties.queryRange = queryRange;
    }
};

Although, if we find this too complicated to use this._properties.propName we could use this._propName or resign from the violation error at all and base this restriction on the documentation (I'm OK with using the @readonly tag).

The last two pieces of the puzzle are what the view and the controller do. Obviously, the view does not see the model. However, it can either user VM for everything or fire some events on itself. Additionally, does the view have methods or not? To answer that we have to understand the scope of communication. The types of situations we have:

Back to the question - should the view implement methods which the controller can use and should the view fire events which the controller/model can listen to? Or can we use the VM for everything? I don't have a strong opinion on that but I would use the VM for everything, so on the data changes (the first two points of the above list) and for the single actions (3rd and 4th points). Although, I'm OK with the alternative option too.

To wrap up:

Random stuff

  1. Model#attributes will lead to name collissions. Better call it _attributes. (BTW. attributes or properties?)
  2. Model#set collide with the ability to set value directly what may be confusing. Couldn't it be just Model#add/define with just one task - defining the property?
    • Other than that, #set actually can't be called more than once for the same attribute now.
  3. If anyone else was wondering too - getters and setters on some browsers might be few orders of magnitude slower than conventional solutions, but it's not a significant difference for us, since we will never use them more than few thousand times per sec (where they would make a difference).
  4. I like the convention that where we can we use the native DOM elements and if we created an instance of our class for them we keep them separately. This way in many places we can work on native DOM elements which may be enough for us and we can avoid creating our instances at all. For example, this may be important in the data processors.
  5. Missing - events delegation in the view. It should be possible to easily define a listener on e.g. the view.el, which will only be executed when the event was targeted on specific set of elements inside the view.el. We could use CSS selectors for that (like Backbone).
  6. (5.js) getActiveClass - I was reading the method without checking the template and I was confused what the value is. Direct access to the VM would be much clearer, but I guess that we want to keep that sugar there. Can we make something to make it more obvious? Naming the argument after the VM's attribute name would help I think.
  7. I think that we need a better classes binding. Imagine a pretty common case with multiple classes bound to the VM in different ways. Generating all the classes every time something changes is not cool.
  8. In the autocompleter we had a case when the view needs a caret position on screen (because it's positioned relatively to it. The caret position is theoretically the EditorModel's property (let's say the EditorModel is all the models outside of the current component). However, if, in the controller I make a binding between EditorModel.caretPosition and AutocompleteViewModel.caretPosition, then if an alternative view implementation doesn't need the caret position it will be unnecessarily computed. To avoid that I put the code retrieving caret position in the view (because my view didn't have access to other models). Can it be solved in a better way? Do we care at all?
  9. (5.js) We may need some additional tools for modifying the templates. Currently we have the same kind of situation in the dialogs and I found it rather hard to work on those templates. You need examine the existing template, find indexes, modify it and hope that the original template will not be modified. Also, the resulting code is unclear and always needs comments.

  10. The templates structure is OK. I would only consider extracting children array to the 3rd item of the parent array. Then it will be always ([] around names means that this item is optional):

    [ name, [attributesAndBindingObj], [childrenArray] ]

// Examples:

[ 'span' ]
[ 'span', [ 'a' ] ]
[ 'span', { className: 'foo' } ]
[ 'span', { className: 'foo' }, [ 'a' ] ]

  11. Bindings are really cool (although I sometimes have problems with reading them quickly). I see that the methods don't need a context so we can make the code shorter:

    var prop = mvc.View.bindProp;

var Button = mvc.View.extend( {
    template: [
        'button', {
            onclick: 'click',
            className: prop( 'model.active', 'getActiveClass' ),
            text: prop( 'model.text' )
        }
    ],
  12. I haven't checked yet the Spaces, SpaceManager, Application and Commands classes. It would be cool if we could see a sample of them.
  13. A thing that I already miss is a short version for repetitive and ugly Object.defineProperty :D. I never use it myself because it's so ugly.
mlewand commented 10 years ago

I suggest to make standard for it. Each namespace should be represented by directory with the same name as (...)

I totally agree I'd even presume that it's very obvious. It makes finding anything so much easier.

stands for Binder & Builder

Alright, now I get it. Eventually can't we name it simply a builder, and in it's docs mention that builder is also responsible for binding? Anyway as a results it's a just detail...

keep template in diffrent file

I love keeping markup in separate files, but given example, adds just soo much complexity and overhead. I'm fine with proposed template it's already an improvement opposed to the current templates, as they offer this ability to binding other props.

It's not possible right now, but could you give me a real life example for that? (I don't know CKE4 good enough to name some problems with events in the current version...)

I left it to the end, though it's the most important thing, just because it has more text.

I already gave some scenarios that I find common, but I didn't elaborate on them, as I want to save time in this thread.

  1. We have some action going on if user will perform dblclick on an element. For links CKEditor opens link dialog, for Widgets it's gonna show widget edit dialog. Developer might want to prevent these actions, but still not to break any further dblclick listeners, that he's not aware of.
  2. Requesting for resources. Lets say that we're handling custom classes (standalone, loaded on demand) requesting or mediaembed requestes. Not going into details: developer might want to implement fetching these resources in his custom way, without using CKEDITOR.scriptLoader and other things.
gregpabian commented 10 years ago

OK, I'm gonna answer starting from the end :)

Model#attributes will lead to name collissions. Better call it _attributes.

I'd go with _attributes. BTW. have we considered using _ at the beginning of private properties/methods names?

(BTW. attributes or properties?)

TBD, but I'm following this convention: https://github.com/Reinmar/v5-mvc-research/blob/master/api/core.js#L42

Model#set collide with the ability to set value directly what may be confusing. Couldn't it be just Model#add/define with just one task - defining the property?

I'd go with Model#define(name, [value], [options]) API.

Other than that, #set actually can't be called more than once for the same attribute now.

That's right, by default a porperty's configurable flag is set to false.

Missing - events delegation in the view. It should be possible to easily define a listener on e.g. the view.el, which will only be executed when the event was targeted on specific set of elements inside the view.el. We could use CSS selectors for that (like Backbone).

You want something like events property in Backbone's View? How is it gonna associate with the existing bindings established using template property?

Naming the argument after the VM's attribute name would help I think.

Yup, something like this would make more sense:

getActiveClass: function( active ) {
    return active ? 'active' : '';
}

I think that we need a better classes binding. Imagine a pretty common case with multiple classes bound to the VM in different ways. Generating all the classes every time something changes is not cool.

I agree, e.g. a togglable class should care about itself only and not affect other classes.

However, if, in the controller I make a binding between EditorModel.caretPosition and AutocompleteViewModel.caretPosition, then if an alternative view implementation doesn't need the caret position it will be unnecessarily computed.

If the View is not accessing the ViewModel.caretPosition then why would it be computed?

We may need some additional tools for modifying the templates.

True, the current solution gives us full control, but also makes some simple modification pretty tough - we have to think of some helpers here.

The templates structure is OK. I would only consider extracting children array to the 3rd item of the parent array. Then it will be always ([] around names means that this item is optional):

:+1: - this is almost the same syntax that JSONML uses and I'm all for following an existing convention

I haven't checked yet the Spaces, SpaceManager, Application and Commands classes. It would be cool if we could see a sample of them.

This wasn't the main goal of the prototype so not much to see there yet, besides, incoming changes in Views, Models and VMs will most likely have a lot of impact on their shapes.

A thing that I already miss is a short version for repetitive and ugly Object.defineProperty :D. I never use it myself because it's so ugly.

Again - we can introduce something like Model#define(prop, value, options) to make it shorter.

gregpabian commented 10 years ago

OK, second bunch of comments.

The view contains application logic. It acts on UI events and modifies the model.

Application logic? Where? Alright, the name "View" here might be a bit misleading. It is not the exact representation of a View in MV* - it's more of a Presenter from MVP mixed with a View's concerns and that could be separated.

Everything in the model is fully visible. There's no encapsulation at all.

I haven't heard of such requirement when I started prototyping, besides it's hard to get a true encapsulation in JS and I can't tell if existing frameworks care about this much...

The controller only initializes the view and model. The model is totally empty. That means that all the rest is implemented by the view.

Hmm, where exactly?

The API which view must implement is not defined

What do you mean? What are your expectations on this?

The first thing to notice is that the model proposed in the prototype is actually only a bidirectional data binding. So this is the view model (VM) from the MVVM pattern which Fred was exploring.

It's the simplest Model implementation I could imagine, nothing stands in the way of adding some logic to it. (OK, when you were writing it, the Model missed 2 things I added later - extend API and handling of properties object used to extend the instance so this might change the perception of a Model now).

VM should not contain any logic

Really? What about exposing methods used by a View, what about e.g. translating a Model's properties to a form understandable by Views? ViewModel is a middleman here and may need some logic.

The model implements the tricky logic which assures the validity and integrity of the data. Perfect example of the need of having a well defined model was the model in the autocomplete plugin which:

has two properties queryText and queryRange which always go together (so there's one method setQuery to set them both), has a data property which is reset by setQuery and then set asynchronously or synchronously (it depends on the implementation) when the suggestion items are retrieved (e.g. from the server), has a selectedItemId which is reset when query changes (so by the setQuery method), has a set of methods like selectItem, selectFirstItem, selectNextItem which handle cases like the end of the list of items or an empty list.

Most of this can be achieved using the existing Model implementation, short example:

var CustomModel = mvc.Model.extend({
    initialize: function (attributes, properties) {
        mvc.Model.prototype.initialze.call(this, attributes, properties);

        this.on('change:query', function () {
            this.selectedItemId = null;
            this.data = null;
            // ...
        }, this );
    },

    selectItem: function () {
        // ...
    },

    setQuery: function () {
        // ...
    }
});

Although, if we find this too complicated to use this._properties.propName we could use this._propName or resign from the violation error at all and base this restriction on the documentation (I'm OK with using the @readonly tag).

Let's avoid over-engineering things. Properties can still be accessed and modified using this._properties[name] so it guarantees nothing but additional complexity. Having well written documentation and standardized way of doing "things" should be enough.

The Model must change name to ViewModel.

I won't be so sure - first of all - I never said this is a MVVM pattern implementation... :) Besides, mvc.Model already does a model's job, but I agree it could to this better (e.g. better way to define attributes, computed attributes maybe?).

We need UI events -> VM events binding. This should include events delegation (listening on root for events on the children)

I'd love to get more suggestions on this piece.

it'd be good to have easy way to call preventDefault and/or stopPropagation.

preventDefault - fine, as long as we know what we're doing, but I'm still against stopPropagation...

We need a completely new Model class.

I don't agree, for the above reasons. What we really need is separation of Presenter's/Controller's concerns from the View and/or completely new class - ViewModel (but I'm still not sure how it would look like...)

We need a Controller class with tools for making M <-> VM bindings.

I don't think we need a Controller for this - a ViewModel could take care of the binding with its model.

gregpabian commented 10 years ago

I did some changes in the mvc-samples branch:

gregpabian commented 10 years ago

I think that we need a better classes binding. Imagine a pretty common case with multiple classes bound to the VM in different ways. Generating all the classes every time something changes is not cool. I agree, e.g. a togglable class should care about itself only and not affect other classes.

I was thinking about this issue and found following use cases:

All of the above, except the last one is doable now. What we have to think about now is how to allow defining a mix of those case.

I'm thinking of using something like this:

...
template: [ 'a', {
    classes: [
        'foo',
        'bar',
        bindClassValue( 'model.foo' /*, mutatorFunction */ ),
        bindClassToggle( 'hidden', 'model.foo' /*, conditionFunction */ )
    ]
}]
...

(Side note: I think that className property should be still usable apart from the classes.)

Now, while implementing bindClassToggle helper seems to be pretty simple using Element.classList, the other one with values might be tricky and require some additional effort (e.g. storing previous value to remove it later on change). So, what do you think?

adelura commented 10 years ago

Does bindClassValue have to be responsible only for classes? I think that it might have name bindValue and be responsible for other attiributes like href, src etc. We we might end up with somethink like this:

template: [ 'a', {
    classes: [
        'foo',
        'bar',
        bindValue( 'model.foo' /*, mutatorFunction */ ),
        bindToggle( 'hidden', 'model.foo' /*, conditionFunction */ )
    ],
    href: bindValue('model.link')
}]

(Side note: I think that className property should be still usable apart from the classes.)

Isn't class more natural and more clear?

gregpabian commented 10 years ago

Does bindClassValue have to be responsible only for classes?

Yes, it uses Element.classList token list to add/remove classes so it won't work for other properties.

Isn't class more natural and more clear?

Yes, it is, but class is a reserved word in JavaScript.

gregpabian commented 10 years ago

On second thought, it's possible to merge bindClassValue with bindProp and so I did, now you can do the following:

classes: [ 'foo', 'bar', bindProp( 'model.baz' ) ]

I also modified bindClassToggle API to allow negating property value to toggle class by adding ! at the beginning of the property, example:

classes: [ 'foo', 'bar', bindClassToggle( 'hidden', '!model.visible' ) ]

Now if model.visible is truthy, the hidden class will be removed and added if the property is falsy.

fredck commented 9 years ago

I think that, at the current stage, most of the details about the MVC API will be developed once we'll have a better understanding on how we gonna organize the whole UI creation and handling.

We should have our ideas about MVC already in place, therefore we must proceed with other design topics and adapt our MVC infrastructure to them. I doubt we'll ever reach a final API right now.

Therefore, I've ported our previous generic documentation about MVC into our wiki. This is a first step, to make our MVC intentions official.

Reinmar commented 9 years ago

(Side note: I think that className property should be still usable apart from the classes.)

If you meant that both should be usable at the same time, then TBH I would avoid that. Either you use className or classes, your choice. I can't imagine how they could be synchronised. Unless, that's exactly what you meant :).

  • a mix of above

All of the above, except the last one is doable now

A mix of above can be achieved by bindProp() with a mutator. If the mutator returns a falsy value, then the class is removed. So with a mutator you can do everything...

... except binding one class (or actually anything) to two Model properties at the same time (the thing should be updated whenever any of these two properties change). It resembles merging two streams in the reactive paradigm. I think that in this case it could be achieved by the view defining a new Model property which will be a merge of two or more other properties. It does not even have to have a value in some cases - it can be a virtual property which has a corresponding change: event which is fired whenever any of the source properties change.

PS. I've just realised that in the Model you have attributes, while the function is called bindProp() (although, I don't know to what "prop" refers in this case) and we all call them properties (you even wrote Model#define( prop )). So maybe the answer to:

(BTW. attributes or properties?)

Is "properties" because this seems to be more intuitive. HTML tags have attributes, classes, objects and all the rest have properties.

gregpabian commented 9 years ago

If you meant that both should be usable at the same time, then TBH I would avoid that. Either you use className or classes, your choice. I can't imagine how they could be synchronised. Unless, that's exactly what you meant :).

Yup, you can use one or another but not both at the same time - this would produce too many weird scenarios to cover.

A mix of above can be achieved by bindProp() with a mutator. If the mutator returns a falsy value, then the class is removed. So with a mutator you can do everything...

Yup, that's true for the current version.

... except binding one class (or actually anything) to two Model properties at the same time (the thing should be updated whenever any of these two properties change). It resembles merging two streams in the reactive paradigm. I think that in this case it could be achieved by the view defining a new Model property which will be a merge of two or more other properties. It does not even have to have a value in some cases - it can be a virtual property which has a corresponding change: event which is fired whenever any of the source properties change.

Or even something like this:

var CustomView = mvc.View.extend( {
    initialize: function () {
        mvc.View.prototype.initialize.apply( this, arguments );

        this.listenTo( this.model, 'change:foo', this._fooBarChanged, this );
        this.listenTo( this.model, 'change:bar', this._fooBarChanged, this );
    },

    template: [
        'div', {
            // a View already inherits from the Emitter so we can do the following
            className: mvc.View.bindProp( 'fooBar', function ( model, value ) {
                // do magic here...
            } )
        }
    ],

    _fooBarChanged: function ( model, newValue, oldValue ) {
        this.trigger( 'change:fooBar', model, newValue, oldValue );
    }
} );

Written from my head, might not work :) Anyway, it's doable.

fredck commented 9 years ago

I've enlarged the docs to describe what we should expect to have in the model's state: https://github.com/ckeditor/ckeditor5-design/wiki/MVC#the-models-state

Reinmar commented 7 years ago

The implementation on MV* was refactored a couple of times but we finally came to a point where we're comfortable with the result. It can be now found in https://github.com/ckeditor/ckeditor5-ui.