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

Make events consistent by introducing data.ready and plugins.ready #1477

Closed
f1ames opened this issue Jan 22, 2019 · 2 comments
Closed

Make events consistent by introducing data.ready and plugins.ready #1477

f1ames opened this issue Jan 22, 2019 · 2 comments
Assignees
Labels
package:core package:engine type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@f1ames
Copy link
Contributor

f1ames commented Jan 22, 2019

Extracted from #1449.

In #1449 the editor.uiReady event was moved from Editor to EditorUI and renamed to ready which means editor.uiReady was replaced with editor.ui.ready event.

To improve the API and keep it consistent, same could be done with dataReady and pluginsReady events (first is fired by specific editor creators and second by the Editor instance).

The dataReady should become editor.data.ready (fired by DataController class) and pluginsReady should become editor.plugins.ready (fired by PluginCollection class).

@f1ames f1ames added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". package:core package:engine type:improvement This issue reports a possible enhancement of an existing feature. and removed type:task This issue reports a chore (non-production change) and other types of "todos". labels Jan 22, 2019
@f1ames f1ames self-assigned this Jan 23, 2019
@f1ames
Copy link
Contributor Author

f1ames commented Jan 23, 2019

When it comes to dataReady event and moving it to DataController class - it should be fired on the end of the init method (somewhere here probably). Now, there is another event already used in DataController called init created by decorating the init() method.

This means there will be two events - init and ready fired in the exact same moment (btw. ready could be also implemented using decoratedelegate). IMHO it doesn't make sense, because these two events just indicates the same thing (data initialization has finished). Should we get rid of init event?

The second thing, is it possible to have editor creator which does not use data.init method (such create() method is used in Editor class)? In such cases, data#ready event will not be fired. Should it be mandatory to use data.init() method or fire data#ready event manually when implementing editor creator (maybe it already is)?

cc @pjasiun @Reinmar

@pjasiun
Copy link

pjasiun commented Jan 23, 2019

When it comes to dataReady event and moving it to DataController class - it should be fired on the end of the init method (somewhere here probably). Now, there is another event already used in DataController called init created by decorating the init() method.

init event is needed to make it possible to overwrite this method. However, we could fire ready in the init listener with the LOWEST priority. It makes sense that at the end of the initialization data controller says it is ready. It would also work with custom listeners which adds additional functionality to the init method.

The second thing, is it possible to have editor creator which does not use data.init method (such create() method is used in Editor class)?

I think each editor should init engine using data.init. We should remove this Editor.create method since it is simply incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:core package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

3 participants