View events, keystroke handler, commands, composer – their place in the editor architecture

[Reinmar]

This is a followup of or concerns the following issues:

• https://github.com/ckeditor/ckeditor5-engine/issues/586 (topic starter – read if you have too much time)
• https://github.com/ckeditor/ckeditor5-core/issues/22
• https://github.com/ckeditor/ckeditor5-core/issues/20
• Features should set their keystrokes through the keystroke handler #339
• https://github.com/ckeditor/ckeditor5-list/issues/10

The problem

Most features have to react to user actions which often mean reacting to a certain keystroke. This sounds trivial, but it turns out that this concerns many layers of the editor architecture:

1. engine view,
2. keydown event (view observers),
3. keystroke handler (optionally – a feature may listen on the view directly),
4. command (optionally),
5. model composer (optionally),
6. engine model.

Thanks to the event-based architecture, a feature may interact with pretty much every layer here. This is, on one hand, great and gives amazing flexibility. But on the other hand, it's a curse of abundance when it comes to deciding where you want to hook in your code.

These were some of the problematic subjects:

• making actions extensible – one feature may want to extend the default action of another feature (e.g. Enter should behave differently when at the end of headings or in list items),
• making keystrokes configurable – and remembering that:
  • when you change configuration all behaviour should be rebound,
  • configuration is set in a declarative way and should not require too much "coding".
• defining granularity of "actions" – e.g. is "enter" only a key which triggers couple different actions (break block, outdent list) or is "enter" an action triggered by the Enter key?

As I mentioned, there are many possible solutions, so it's hard to explain why all of the alternatives are flawed (and that's why we discussed this for a month, changing opinions couple of times). Let me only describe the architecture which IMO works best.

Solution

Keystroke handler

The keystroke handler should be used to map keystrokes to commands. Imagine e.g. a graphical program like Photoshop which lets you define keystrokes. You have a list of commands and you can assign keystrokes to them. This is what keystroke handler should be used in CKEditor 5.

In the specific case of Enter, the action of "enter" is also called "enter" (enter command). The authors of the Editing spec (see my comment) tried to call this action "insertParagraph", but it's very weird that insertParagraph's default action is e.g. to create a new list item or outdent a list.

Anyway, the most important thing to remember is actions == commands.

There's a tricky thing here, though. If we'd use keystroke handler to assign such actions as "enter" or "delete" to keystrokes, then we wouldn't be able to switch to the beforeinput event once it's ready. Keydown is fired before beforeinput so by handling specific keys through the keystroke handler, we'd block firing beforeinput. The power of beforeinput is that it is far better integrated with the OS and the browser than we can be by listening to key events, so as soon as it's available we'd like to use it.

I thought that we could divide keys into two categories – those which will have specific beforeinput type and those which won't. But then I realised that most of the keystrokes will have assigned input types. So there's no other choice – at some point in the future, we'll have to remove keystroke bindings. However, things should work smoothly because it will still be possible to e.g. bind "moveToNextParagraph" command to Shift+Enter.

tl;dr: All features should use keystroke handler to bind commands to keys. Commands == actions.

Place of extension

Since keystrokes should be configurable, which means that you should be able to rebound any action (command) to any keystroke, the command should be atomic. E.g. enter should always execute the "enter" command and this command should take care of handling lists, headings, etc.

This means that it's the commands which should be extendable. For that we need smart beforeExecute and afterExecute events to be fired by them.

The composer

Composer should only contain action primitives – operations on the model. We've just made commands extensible and keystrokes configurable, but we also need to make those composer methods extensible as well. However, the set of methods should be closed and defined by the engine. There will be 4 methods:

• Insert data (behaves like "paste").
• Delete data (behaves like Delete or Ctrl+X).
• Copy data (behaves like "copy").
• Modify selection (behaves like arrow keys).

You should use those methods when dealing with unknown content of the model. E.g. the delete feature uses delete composer method in order to delete selected part of the content. The delete feature doesn't know about other features enabled in the editor, so it wouldn't know itself how to perform their deletion. Similarly, when typing into a non-collapsed selection, the typing feature must delete the selected content, prior to inserting content in it. In both cases, the composer's method should be used.

Note: based on these 4 methods we could implement the whole editor. We won't do that, because, in order to have more semantical operations on the model (in terms of OT), we often will use deltas directly, but the more feature use composer methods, the better.

Summary

I hope that the proposal looks good to you because I really don't want to discuss it further. I can clarify some things, but I don't want to explore other directions, because we've seen too many of them leading to a mess.

[Reinmar]

This ticket should not have any action. There will be other tickets, though, in which we'll link to this one.

[Reinmar]

PS. There's a case in which we totally change the meaning of a key – e.g. when enter is pressed while a widget is selected we open an "edit widget balloon". These are special cases where we could listen directly on the view (because the action isn't any more happening in the editable). However, we can also make it possible to bind more commands to a single keystroke (and prioritise them).

This, fortunately, it's a pretty rare case, so it should not be concerned by it.

[scofalik]

Okay, so I will just comment on what I don't like (and describe an alternative idea that we discussed). The whole idea is okay but there's one thing that just bugs me a lot... And I just can't stop myself from commenting on it. I know we have to accept something and finally code something but, no matter what we choose, I'd like to have my word here :)

You say that command == action but what does it really mean? By the way how you described it, the command is really a shortcut to what user did and does not have much to do with user intention (the original idea).

So your idea means that we have EnterCommand which doesn't have real specification of what it does, other than "it does whatever should happen when enter key is pressed". In the same vein, we could have SpaceCommand, TabCommand or EscapeCommand. This is far away from keystroke bindings in software and command is not an "action". It's like setting "on [Enter] do [Enter stuff]" in such software :P.

You also argue, that this makes re-binding easy, etc. But then you are able to rebind EnterCommand to space or TabCommand to enter and this is more WTF-ish than what I will propose. Also, rebinding can be done by high-level API, so we don't need to have an "anchor point" like a command.

Even more, you already provide a scenario when your idea is wrong (widgets). I'd argue, that outdenting on enter is also a case of "totally change the meaning of a key". On the other hand, I suppose that Enter is the only key with such broad meaning so we won't have those problems with other keystrokes.

So, my idea is that we do not treat commands as "whatever user did" but really "actions" or "intentions". Then we could have BreakElement command (or EnterCommand for historical reasons) and IndentCommand. Then, we bind both EnterCommand to Enter and following callback:

() => {
  if ( positionAtTheEndOfListItem ) {
    editor.execute( 'outdentList' );
  }
};

We could also make API a bit different and use three parameters for binding: keystroke, commandName and, optionally, callbackCheck. If callbackCheck is provided, it is fired and if it returns false, the key binding is skipped (command doesn't fire). This makes it less flexible but more tightened up so we don't use keystrokes to do everything and I like this idea too.

Apart from that we'd need some priorities, but this maybe coud be build on EmitterMixin and we need rebinding. I see it simply as keystrokeHandler.rebind( 'enter', 'shift+space' ). Rebinding should/could be done only after all features are loaded so we won't have the problem that we bind something to enter rebind it to space and then bind something again to enter expecting that it will be bound to space.

BTW. Note how "expanding keystroke" (using priorities) is similar to "expanding command" (using beforeExecute and afterExecute events) but is much better from semantic point of view :).

I am sorry I am discussing this again, but:

1. You described your ideas really well and I think the discussion can be much narrowed and more profitable,
2. I agreed with you yesterday (mosty because I was feeling hopeless :P) but today when I think of it, I am just sad and it hurts me.

All in all, I can accept going your way but I think it's not perfect and we can do better with only a bit more work.

[Reinmar]

I knew it... I should've blocked commenting :P.

Going into the direction that you proposed, when commands are immutable and all happens within keystroke handler is possible, but will require a lot of additions to the keystroke handler. Most likely, to make all this configurable, we'll have need to introduce a concept of "actions" there. This may be a way to go.

Especially that @fredck just brought to me a problem with my initial proposal. The extensions (like the enter to outdent) are based on existence of the base commands. If you won't load the enter feature, then it's not a big problem – you apparently don't want to handle enter. But if you'll load a replacement of the enter feature which binds Enter to a different command (e.g. myEnter), then you're screwed – the list feature won't add its behaviour to it.

So this is a strong argument for keeping commands immutable and the whole configuration outside. But as I said – it's not very clear how to do this in the keystroke handler to ensure that it's still configurable in a declarative way (through the config). We could perhaps also make some things configurable using the config object and some through editor.keystrokes – more complex scenarios would be handled programmatically.

Another thing to keep in mind is being able to retrieve the keystrokes => actions pairs from the keystroke handler so you can create a keystrokes help (in CKE4 it's done pretty automatically IIRC). That's another argument for introducing the concept of "actions" to the keystroke handler.

There could be a predefined set of actions, so features would know to which actions they can listen. This, of course, can be implemented using events. The feature would then define the default keystroke which triggers certain actions – e.g. that Enter triggers "enter" action (and some keystrokes could be predefined).

Then, in the keystroke handler configuration you'd simply pass pairs of "keystroke => action" or "keystroke => null" to unbind some defaults.

If this sounds great to you (it sounds ok to me), then we can investigate this idea further. But please – do not underestimate our tricky cases. I'm really unsure how all the feature will integrate with each other. Also, we'll have to decide whether headings feature extends the enter command or enter action. The latter sounds cleaner (because we'd make commands immutable), but it's harder to implement in the heading feature (because it must borrow part of enter feature behaviour)... And last but not least – how can then a developer reproduce the behaviour of "enter"? The keystroke handler would always be required then – which sounds fine, I guess, because it's a part of base Editor class.

Anyway, we should be careful.

PS. One thing that we could think through is whether "key actions" could not be generalised to "user actions" and action handler. Perhaps it'd be useful in other cases as well.

Let's talk about this F2F.

[Reinmar]

We've talked with @fredck... and we'll go with the initial proposal I made in this ticket and we may make some improvements (mainly to the keystroke handler) in the future :D. Things that we agreed on:

1. It's correct that features like list or heading modify the enter command behaviour. You expect to trigger their specific behaviours when you execute this command. The same should happen if e.g. someone will like to change the bold's behaviour in, let's say, headings.
2. Things like "on enter when widget is selected, we should open the edit balloon" needs to be resolved in the keystroke handler. So in a near future, we'll need to make it possible to add multiple listeners to one keys. Most likely, the way to go will be to fire events on the keystroke handler. It'd be useful if they were named like the keystrokes. E.g.: key:ctrl+b.

[Reinmar]

Things like "on enter when widget is selected, we should open the edit balloon" needs to be resolved in the keystroke handler. So in a near future, we'll need to make it possible to add multiple listeners to one keys. Most likely, the way to go will be to fire events on the keystroke handler. It'd be useful if they were named like the keystrokes. E.g.: key:ctrl+b.

As for the event names – if we'll allow things like optional modifiers (see https://github.com/ckeditor/ckeditor5-core/issues/20), then it will be hard to come up with some events... We should be careful here.

[Reinmar]

There's yet another topic that I wanted to mention. There's a difference between how we define commands in CKEditor 4 and 5. In CKEditor 4, the link command will open the link dialog. In CKEditor 5 the link command applies specified link to the current selection.

This is an important shift. In CKEditor 4 there's no way to reuse functionality of the dialogs (which, on "ok" insert or change stuff in the editable). That's why the link command in CKEditor 5 is focused on the engine.

However, it's not clear now how you can open the link balloon. You'd like to do that if you used external buttons which should interact with the feature's UI (external link button should be able to open the link balloon).

I proposed two solutions in https://github.com/ckeditor/ckeditor5-link/issues/45. Either we expose more commands (like showLinkBalloon), or we expose these things like feature methods. Commands would be more useful, because in some cases they will need to have state and other features could interact with them (e.g. disabling them). WDYT?

[fredck]

As for the event names – if we'll allow things like optional modifiers (see ckeditor/ckeditor5-core#20), then it will be hard to come up with some events... We should be careful here.

The final event names can be numbers, like key:1020, as long as I'm able to create the event listener with a tool that accepts modifiers.

[fredck]

WDYT?

I think we're mostly talking about being able to execute a feature by invoking the UI connected to it. Some ideas:

• feature.exec(): call UI for UI-features or command for UI-less-features.
• command.exec( showDefaultUI ): inspired by execCommand.
• uiCommand.exec(), as you proposed.