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

Tweaks to appearence transitions handling on setCenterViewController* methods #139

Closed

Conversation

Air-Craft
Copy link

Relates to issue 138: #138

  • Fixes missing [oldCenterViewController willMoveToParentViewController:nil]
  • Fixes missing edge case on setCenterViewController:with(Full)CloseAnimation which resulted in view*Disappear methods not being called
  • Fixes edge case which resulted in view*Appear methods being called twice
  • Reorganises begin/endAppearenceTransition methods to have a more systematic calling order (begin's on both VCs called before end's)

Hari Karam Singh added 2 commits October 10, 2013 13:02
… methods

- Fixes missing `[oldCenterViewController willMoveToParentViewController:nil]`
- Fixes missing edge case on setCenterViewController:with(Full)CloseAnimation which resulted in view*Disappear methods not being called
- Fixes edge case which resulted in view*Appear methods being called twice
- Reorganises begin/endAppearenceTransition methods to have a more systematic calling order (begin's on both VCs called before end's)
Adds a check that the container is actually visible before calling the appearance transitions.  This prevents reduandant calls in the setCenterViewController* methods wrt to those called in the viewWillAppear, etc methods of MMDrawerController.
@kcharwood
Copy link
Contributor

Thanks for the pull request! I've been traveling for the last couple of weeks, but I'll have some time this week to take a look at this and get it pulled in.

@paiv
Copy link

paiv commented Oct 14, 2013

MMDrawerController (0.5.1)
I have an issue: when neither left nor right side is open, changing center view controller does not invoke viewWillAppear, viewDidAppear.
I hope this is addressed with this patch. Although I did not review it in full.

What I patched in my code for now -- call explicitly beginAppearanceTransition, endAppearanceTransition after changing center view controller.

@Air-Craft
Copy link
Author

I believe it will fix the issue. This was an edge case that was missed when the animated flag was YES, IIRC

@kcharwood
Copy link
Contributor

There are actually quite a few more changes in here than what I expected. I'm still trying to walk back up the tree of what the original problems were, and trying to isolate what parts of your pull request fix each issue you reported.

The appearance transition code is tricky, so I also want to make sure there are no regression issues here. Thanks again for the pull.

@kcharwood
Copy link
Contributor

Also, I'm not sure I am able to reproduce the fullCloseAnimated problem you mention above.

In the example, when I full close the center from a drawer, I get the following trace:

2013-10-17 08:47:51.092 MMDrawerControllerKitchenSink[48823:a0b] Center will appear (<MMExampleCenterTableViewController: 0xb96a650>)
2013-10-17 08:47:51.578 MMDrawerControllerKitchenSink[48823:a0b] Center did appear (<MMExampleCenterTableViewController: 0xb96a650>)
2013-10-17 08:47:52.406 MMDrawerControllerKitchenSink[48823:a0b] Left will appear
2013-10-17 08:47:52.744 MMDrawerControllerKitchenSink[48823:a0b] Left did appear
2013-10-17 08:47:56.429 MMDrawerControllerKitchenSink[48823:a0b] Center will disappear (<MMExampleCenterTableViewController: 0xb96a650>)
2013-10-17 08:47:56.590 MMDrawerControllerKitchenSink[48823:a0b] Center did disappear (<MMExampleCenterTableViewController: 0xb96a650>)
2013-10-17 08:47:56.590 MMDrawerControllerKitchenSink[48823:a0b] Left will disappear
2013-10-17 08:47:56.594 MMDrawerControllerKitchenSink[48823:a0b] Center will appear (<MMExampleCenterTableViewController: 0xb9956b0>)
2013-10-17 08:47:57.084 MMDrawerControllerKitchenSink[48823:a0b] Center did appear (<MMExampleCenterTableViewController: 0xb9956b0>)
2013-10-17 08:47:57.084 MMDrawerControllerKitchenSink[48823:a0b] Left did disappear

If I close with animated NO, I still get the same trace. What am I missing?

What changes to the current example could I make that replicate the problems you were seeing?

@kcharwood
Copy link
Contributor

Ping @Club15CC

@Air-Craft
Copy link
Author

Hi there,

Sorry for the delay. I’m just finishing up a contract and will attempt to isolate this better when I have a moment. The problem occurs when you call the methods with code rather than use the menu. It’s certain combinations of

  1. Drawer is currently Open / Closed
  2. You call the method with animated: YES / NO
  3. You call withFullCloseAnimation or plain vanilla withCloseAnimation

Particularly when you call with animated:YES but the drawer is already closed, I think, is when you get some problems.

Through observation of the app and also by following the logic of the code, I believe there is at least one of these cases that result in view transition methods being called more than once as well as a scenario where some are not called.

Hope this helps. Let me know…

HKS

On 28 Oct 2013, at 14:14, Kevin Harwood [email protected] wrote:

Ping @Club15CC


Reply to this email directly or view it on GitHub.

@kcharwood
Copy link
Contributor

@Club15CC I think have the scenario with changing out the center controller when the drawer is closed addressed in #212.

If you are able to reproduce other issues, please let me know! Thanks for taking the time to help debug this.

🍻

@kcharwood kcharwood closed this Jan 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants