SideMenu icon indicating copy to clipboard operation
SideMenu copied to clipboard

App behaves weird in iPad due to transition

Open kirthika opened this issue 4 years ago • 13 comments

I have read the guidelines for contributing and I understand

  • [yes] My issue is happening in the latest version of SideMenu (older versions are no longer maintained).
  • [yes] My issue was not solved in the README.
  • [yes] My issue can not be answered on stackoverflow.com.
  • [yes] My issue is not a request for new functionality that I am unwilling to build and contribute with a pull request.
  • [yes] My issue is reproducible in the demo project.

Describe the bug App goes to background/when we try to open an external link and then when we come back to our app, the app transits to landscape in iPad.

To Reproduce Create an option under Push View Controller of Left Menu that takes to an external link.
Click on the above option.
Come back to Side Menu app.

Expected behavior Side Menu does not transit to landscape in iPhone and behaves as expected. However, it transits in iPad.

Screenshots Simulator Screen Shot - iPad (7th generation) - 2020-10-21 at 20 30 17

Video Sidemenu_6.5_fix.mov.zip

Modified sample code SideMenu-6.5.0.zip

kirthika avatar Oct 21 '20 15:10 kirthika

I've tried reproducing this using your modified project using iPad Pro (iOS 14) and have not been successful. Please provide the following:

  • Clear written steps to reproduce the problem.
  • iOS Version you are seeing the problem on.

jonkykong avatar Oct 21 '20 22:10 jonkykong

@kirthika

jonkykong avatar Oct 22 '20 21:10 jonkykong

@jonkykong On steps to reproduce : you've got to click "Open link" that opens google in safari and then come back to the application. May be trying it multiple times you could reproduce it. Also, please try as I did in the video attached. iOS Version : 13.7 iPad(7th generation). Thanks.

kirthika avatar Oct 27 '20 05:10 kirthika

I could reproduce the issue as well but it does not happen every time. I need to try the following steps around 5 times to reproduce the issue.

  1. open the app
  2. open the sideDrawer
  3. send the app to background (no force quit)
  4. re-open the app, if issue didn't occur, start with step 2.

Here is a video that shows how i reproduced the issue with the sample code @kirthika provided.
ScreenRecord.mov.zip

At #648 I posted another video showing the same issue but with my production codebase.

2h4u avatar Oct 27 '20 08:10 2h4u

Hi, I'm experiencing the same issues too in my App. Did you have any workaround to avoid it?

Thanks

DiegoArenaO avatar Oct 29 '20 14:10 DiegoArenaO

@DegenDao try out my PR i made a while ago and give us feedback if it helps. For me at least it fixed all issues https://github.com/jonkykong/SideMenu/pull/650

2h4u avatar Oct 29 '20 14:10 2h4u

@2h4u This change fix the issue only if the device is not rotated, but it's a good approach to fix the rotation issue. Thanks

DiegoArenaO avatar Oct 29 '20 15:10 DiegoArenaO

@2h4u Adding the inactive case to your fix I was able to fix the weird transition when rotates the device in background too. I just left a comment in the PR #650 with the code.

Thanks.

DiegoArenaO avatar Oct 29 '20 15:10 DiegoArenaO

Can someone please give me clear repro steps, even if it's not consistent? I want to understand the problem better before I take anymore changes to this issue. @DiegoArenaO the main fix seems to be checking for the app's inactive state -- I'd prefer to keep the changes around the forced unwraps out and the fatal errors aren't very actionable for a developer to troubleshoot their own code since it's deep within SideMenu's code.

jonkykong avatar Nov 02 '20 19:11 jonkykong

Can someone please give me clear repro steps, even if it's not consistent? I want to understand the problem better before I take anymore changes to this issue.

Which information is my post (https://github.com/jonkykong/SideMenu/issues/656#issuecomment-717080584) missing to make it a clear step-by-step guide for you?

@DiegoArenaO the main fix seems to be checking for the app's inactive state -- I'd prefer to keep the changes around the forced unwraps out and the fatal errors aren't very actionable for a developer to troubleshoot their own code since it's deep within SideMenu's code.

The fatalError() calls fulfill more of a documentary purpose. And if everything behaves as indented the devs which use the SideMenu lib should never notice them because there should always be a non-nil presentationController within the SideMenuAnimationController if a method like SideMenuAnimationController.transitionWillBegin() is called. The benefit of this is that everybody that takes a look at the code of SideMenu immediately knows that presentationController should not be nil at this point. Maybe there should be also some documentation which describes why presentationController needs to be present and what happens if it is not.

The main problem with the current implementation is that it says that presentationController == nil is a valid state, which it is clearly not. So it is better that there is a crash with a descriptive error message than misbehaving code which is very hard to debug. (Here is a quick read for everyone which wonders why this might be a good idea: https://www.martinfowler.com/ieeeSoftware/failFast.pdf)

2h4u avatar Nov 03 '20 09:11 2h4u

Hi @jonkykong in my case I was able to reproduce it consistently in my app, the steps are the same of the @2h4u in the first comment but the steps are the following: 1 - Show menu panel 2 - Sent the app to background 3 - Open the app 4 - Open the menu panel 5 - The detail view is rotated 90°

DiegoArenaO avatar Nov 03 '20 14:11 DiegoArenaO

@DiegoArenaO that's right.. @jonkykong please try with the same steps..

kirthika avatar Nov 03 '20 16:11 kirthika

Hi @jonkykong Any news with this bug? we are still facing it.

DiegoArenaO avatar Dec 03 '20 15:12 DiegoArenaO