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

Coordinator viewmodel ownership #29

Merged
merged 7 commits into from Aug 25, 2017
Merged

Coordinator viewmodel ownership #29

merged 7 commits into from Aug 25, 2017

Conversation

mkj-is
Copy link
Contributor

@mkj-is mkj-is commented Aug 21, 2017

Update of the architecture. As we decided in the past, the ownership of the coordinator is tranferred from view controller to the view model.

Some more changes and improvements must have been done during the change:

  • Removal of Coordinated protocol
  • So the controller does not have the ability to access coordinator and view controllers directly, two protocols are used. The one for coordinator describes all navigation methods that the coordinator has. Coordinator input protocol has all methods for returning values and sending changes to view controller.
  • Forgotten navigate method was removed from coordinators.
  • Configuration block was changed to configure method which is much simpler to use. (No need to use [weak self] or [unowned self], lazy etc.)
  • One test with example scene and its navigation was created.
  • The view model is now created lazily by the child coordinator. It is no more created in the parent navigation coordinator, this prevents code repetition in navigation methods.

Copy link
Contributor

@zvonicek zvonicek left a comment

Choose a reason for hiding this comment

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

Mostly discussed in person.

@mkj-is mkj-is merged commit d010fd3 into futuredapp:coordinator-ownership Aug 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants