Mvvmcross: MvxModalNavSupportIosViewPresenter does not use CurrentTopViewController

[Cybrosys]

Steps to reproduce

1. Use MvxModalNavSupportIosViewPresenter
2. Show a MvxViewController that inherits from IMvxModalIosView
3. Show a MvxViewController without IMvxModalIosView
   

   Expected behavior

The final show should use the NavigationController that contains the IMvxModalIosView to Push/Show the normal MvxViewController (CurrentTopViewController), not the MasterNavigationController. A push modal should allow for a new horizontal navigation. Preferable a way to close the modal as well, collapsing the whole navigation chain.

Presenters in MvvmCross: Nested Modal Navigation in iOS

Actual behavior

No navigation happens after showing a IMvxModalIosView and trying to Push/Show a normal MvxViewController.

Configuration

Version: 4.1.0

Platform: iOS

[PelleRavn]

I agree with you, it lacks some "native behavior" you would except from iOS.

I've been using a custom presenter based on the NestedModalPresenter class you linked to (with a few bug fixes and improvements). Can make a PR with it, if people seem interested in it.

[martijn00]

@PelleRavn We would appreciate a PR!

[PelleRavn]

@martijn00 Cool, will go ahead and make that.

[PelleRavn]

In my opinion, we could easily merge MvxModalNavSupportIosViewPresenter and MvxModalSupportIosViewPresenter (maybe even the MvxIosViewPresenter as well) into one single presenter. It seems like extra code to maintain, and doesn't really have much differences between them.

[martijn00]

@PelleRavn I agree with that. Could you look into combining those?

[PelleRavn]

In that case, the presenter I added in the PR is my suggestion for a common one. If people agree that it's functionality is good enough, then I can make a PR with a merged presenter.

Maybe @Cybrosys could have a look, since he already looked into this?

[Cybrosys]

I have some requests for the new Presenter, I've implemented some of them in my own but they're based off of the same "source" as the presenter the PR is of. I'll just attach the source to this post for @PelleRavn to take a look and maybe incorporate some of the changes; if it's not asking too much?

I've added virtual Show and Close methods that open up the ability for MvxPresentationHint to control if a Presentation or Dismissal should be animated (helps if you want a Hint that pops all the modals but you don't want to animate them all).

There's also a custom NavigationController that inherits from the UINavigationController which enables users to control rotation again in their ViewControllers (the whole implementation was changed in some iOS version and basically stopped working unless you, as I did, created your own NavigationController). In order to control rotation you then just override GetSupportedInterfaceOrientations and PreferredInterfaceOrientationForPresentation in your ViewControler, as you used to do in previous iOS versions.

The final request, is one I haven't implemented myself yet, it's building on the IMvxModalIosView type check in Show (there's some commented out code for testing, it just looks for a specific IMvxModalIosView and skips wrapping it inside a NavigationController). The user should be able to choose whether or not to have their UIViewController wrapped inside a NavigationController. The reason for this has to do with presenting views that have transparent parts modally over an existing view; it just doesn't work if you wrap it inside a NavigationController. Maybe a new interface called IMvxNavModalIosView could be added and checked against, or add a property to the IMvxModalIosView interface that returns whether or not the view should be wrapped?

IosViewPresenter.zip

[PelleRavn]

@cybrosys I get what you mean, and why you want features like this. I removed some of the features from the presenter I made, that I normally have in there when I extend the default presenter. But the question is, if it's "in the spirit of MvvmCross"?
Should the presenter be straight forward and simple to use/understand, or it should be full featured with everything that we can pack in it?

[Cybrosys]

@PelleRavn The usage doesn't differ with the added features, as I see it. The animations are in the current presenter hard coded to true, which is a miss/bad design in my opinion and should have already been exposed as method parameters from the start.

The IMvxModalIosView interface already exists to control if a view should be presented Modally or not. The original problem was that the view did not get wrapped inside a UINavigationController in one of the presenters but did in the other; but was not used as the main UINavigationController which made it completely pointless.

It's better if the user can decide for themselves by a added interface (IMvxNavModalIosView) if they want their modal view to be wrapped or not. The new interface won't break the current usage of IMvxModalIosView by adding a new property/method.

If we always force a modal view to be wrapped inside a UINavigationController we're only creating a problem for when people don't want that to happen; they'll have to write their own presenter again (which is probably why there were two presenters originally).

All of the above added features are there to cover more of the iOS navigation scenarios and fixing the rotation redesign. It doesn't change the usage of the Presenter.

If we don't want to add a MvxNavigationController then the Presenter needs a method for the user to override to give the Presenter the type of UINavigationController that they want to be used when the presenter wraps views, otherwise they'll be forced to write their own Presenter to deal with it.

The added features add value, they do not change how the presenter is used. I don't see a reason why they should not be added. The features are native to iOS already (animation control etc.), it's not like we're adding bells and whistles, we're just adding what should have already been there from the start.

Sorry if I repeated myself, the post became longer than I thought :smile:

[Cybrosys]

What are your thoughts on the matter @martijn00 ?

[martijn00]

I think i would rather have 1 presenter handling most use cases then multiple for separate cases. So i would like to see all of your changes applied to one presenter which will be default after that.

[PelleRavn]

@Cybrosys I agree with you, would much rather have all the possible features for the platform, in the default presenter. Just wasn't sure if it was in the spirit of "You can extend it yourself" or "If you have the code, we would love to do everything".

I will look further into this. But I think, for starters, I will merge the other presenters into one single presenter to resolve this issue. And then afterwords look at a new presenter (in a new issue) with all (or most) the features that we've talked about.

But on a sidenode, this will be breaking changes for users using the default presenters. Maybe they made some fixing, or expect the presenter to behave as it does right now.

[martijn00]

I think we can make this breaking change, but we should make it clear in the blog what changed so users know about it.

[promontis]

+1 for fixing this, even if it introduces a breaking change

[martijn00]

I am thinking to make sure it doesn't break many people's apps, we could add a new presenter doing all this stuff, and make it the default one. That way others can still use the old presenters.

[PelleRavn]

@martijn00 I think that's a good idea.

I have something working at the moment, and is currently testing it out, while figuring out to set it up in the MvvmCross solution to make sure it works there as well.

[martijn00]

@PelleRavn Shall i close #1297 for now, and you open a new PR with the new presenter when you have it ready?

[PelleRavn]

@martijn00 Yes, please do.

[PelleRavn]

I have to realize I can't finish the task of merging the iOS presenters right now. So I wanted to share my progress so far, if anyone wanna pick it up before I get time to finish it.

https://gist.github.com/PelleRavn/dcd62b133107233a1af157612165ecca

Finished so far:

• Made interfaces to present a View as Modal with Navigation Controller (IMvxNavModalIosView) or without Navigation Controller (IMvxModalIosView)
• Updated Navigation Controller to forward Interface Orientation from TopViewController if assigned (as @Cybrosys' code suggested)
• Presenter handles to close the Modal if it's the top View Controller without the need of "view just disappeared by itself"-exception
• Made interfaces to display a modal in different styles (Cross Dissolve, Flip Horizontal, Form Sheet, Popover)

Known things that's missing:

• New implementation of ClearTop (should properly have same API as Android to make them behave somewhat identical). This have some challenges, because you need to dismiss the whole view hierarchy, and then present a new View Controller (with/without animation, with animation is the biggest challenge) as root ViewController on the first Navigation Controller

I've been running with this implementation on some of my own apps, and have been working without issues.