Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ObservableMixin.decorate() #4958

Closed
Reinmar opened this issue Jun 7, 2017 · 0 comments · Fixed by ckeditor/ckeditor5-utils#163
Closed

Implement ObservableMixin.decorate() #4958

Reinmar opened this issue Jun 7, 2017 · 0 comments · Fixed by ckeditor/ckeditor5-utils#163
Assignees
Labels
package:utils type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Jun 7, 2017

A common pattern which we use is that in order to make some component's action overridable we implement it using an event. The general flow is like this:

  1. The method does only one thing – fires an event (called like that method).
  2. The default action of that method is implemented in form of a listener with a default priority.

One can then easily listen to that event, modify arguments, cancel it, perform additional actions, etc.

We always did this manually which means adding a bit of ugly code and is not DRY. When talking about https://github.com/ckeditor/ckeditor5-core/issues/88 we thought that execute() could be implemented in the same way but we can't have even more code duplication and the commands' code should be as clean as possible.

Hence, the idea to implement OM.decorate() (big simplification of the decorators idea). It will take care of replacing the original method with a method which fires and event and plugs a listener.

EventInfo#return

Part two – implement EventInfo#return so you can return from fire(). This will be needed for https://github.com/ckeditor/ckeditor5-engine/issues/963 and it makes no sense to work on these two things separately.

@Reinmar Reinmar self-assigned this Jun 7, 2017
szymonkups referenced this issue in ckeditor/ckeditor5-utils Jun 13, 2017
Feature: Introduced `ObservableMixin#decorate()` and support for setting `EmitterMixin#fire()`'s return value by listeners. Closes #162.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-utils Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:utils labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:utils type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
2 participants