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

Support storyboard embed modules #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RomanTruba
Copy link

This pull request support "Storyboard Embed" segue.
It will allow to automatically wire parent view-controller's presenter with embed view-controller's presenter, which were instantiated from a storyboard.

@RomanTruba
Copy link
Author

Please review

@Sapozhnik Sapozhnik self-assigned this Mar 2, 2017
@Sapozhnik Sapozhnik self-requested a review March 2, 2017 09:45
@kmordan kmordan self-requested a review March 7, 2017 07:56
Copy link

@kmordan kmordan left a comment

Choose a reason for hiding this comment

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

ViperMcFlurry allows us open modules using following methods: openModuleUsingSegue: and openModuleUsingFactory: which return RamblerViperOpenModulePromise.
RamblerViperOpenModulePromise has thenChainUsingBlock: method which receives RamblerViperModuleLinkBlock. This block is not only used for configuring newly initialized module (via moduleInput) but also for returning moduleOutput for this module. Returned module output is used in implementation of thenChainUsingBlock: and links to module which was previously initialised.

When I dive into your solution I discovered that with your changes moduleOutput will be set twice if we use openModuleUsingSegue: method:

  1. in RamblerViperPrepareForSegueSender;
  2. in RamblerViperOpenModulePromise`s thenChainUsingBlock: method.

Could you please fix that?

if ([targetModuleTransitionHandler respondsToSelector:@selector(moduleInput)]) {
moduleInput = [targetModuleTransitionHandler moduleInput];
} else {
return;
Copy link

Choose a reason for hiding this comment

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

I don't really sure that "return" is needed there.

In case sender is RamblerViperOpenModulePromise and targetModuleTransitionHandler does not respond to selector "moduleInput", variable moduleInput will be nil.
Value of this variable will be assigned to property moduleInput of RamblerViperOpenModulePromise instance and in setter (setModuleInput:) will be performed some inner setup which will allow to perform linkBlock but whith moduleInput == nil. If we do not call setModuleInput: inner setup won`t be performed and linkBlock will be never called too.

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.

3 participants