Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

t/95: Remove the Controller class #100

Merged
merged 24 commits into from
Nov 8, 2016
Merged

t/95: Remove the Controller class #100

merged 24 commits into from
Nov 8, 2016

Conversation

oleq
Copy link
Member

@oleq oleq commented Nov 2, 2016

@@ -22,10 +22,14 @@ export default class View {
/**
* Creates an instance of the {@link ui.View} class.
*
* TODO: A simple example how to create one.
*
* @param {utils.Locale} [locale] The {@link core.editor.Editor#locale editor's locale} instance.
*/
constructor( locale ) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember we talked about View taking attributes object as a second parameter, so one can easily initialize multiple observable creating a view class. I'm not sure do we need to make it generic, so should such parameter be in the base View class or in specific views, if needed.

@Reinmar Reinmar self-assigned this Nov 8, 2016
@Reinmar
Copy link
Member

Reinmar commented Nov 8, 2016

I decided to merge all this code to master and review it there because we risk having too many conflicts too quickly and blocking ourselves. 3 of you worked on these changes, all tests pass, CC is 100%, so let's not keep the world (:D) waiting.

@Reinmar Reinmar merged commit 0c093dd into master Nov 8, 2016
@Reinmar Reinmar deleted the t/95 branch November 8, 2016 15:30
@Reinmar Reinmar added this to the iteration 5 milestone Nov 8, 2016
@Reinmar Reinmar removed this from the iteration 5 milestone Nov 8, 2016
@Reinmar Reinmar removed the type:task label Nov 8, 2016
@Reinmar
Copy link
Member

Reinmar commented Nov 9, 2016

I finished the review. I haven't found many things, which is great taken the size of the refactoring. I wrote down everything in https://github.com/ckeditor/ckeditor5-ui/issues/103.

Kudos guys!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the Controller class
3 participants