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

"_lockWindow.rootViewController = _screenshotViewController" is set too early #1480

Closed
84798012 opened this issue Dec 2, 2019 · 14 comments
Closed
Assignees
Labels
bug

Comments

@84798012
Copy link

@84798012 84798012 commented Dec 2, 2019

_lockWindow.rootViewController = _screenshotViewController
this line code set too early, it caused our app UIWindow(should not be can rotated) changed to be can rotated.

Root cause from the code:

  • (UIInterfaceOrientationMask)supportedInterfaceOrientations {
    if (self.currentAppWindow == nil) {
    return UIInterfaceOrientationMaskAll;
    }

  • (BOOL)shouldAutorotate {
    if (self.currentAppWindow == nil) {
    return YES;
    }

Suggetion to fix :
only when present viewcontroller, set the value.
when dissmiss, _lockWindow.rootViewController = nil.

@joeljfischer joeljfischer added the bug label Dec 2, 2019
@joeljfischer joeljfischer mentioned this issue Dec 2, 2019
4 of 4 tasks complete
@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 2, 2019

@84798012 Can you test #1483 to see if it fixes your issue? If not, please provide additional information or a test project so that I can test your setup.

@84798012
Copy link
Author

@84798012 84798012 commented Dec 3, 2019

I have made a test, unfortunately it still have issue. Here following what I fixed code, you can reference.

@implementation SDLLockScreenPresenter

  • (instancetype)init {
    //_lockWindow.rootViewController = _screenshotViewController; // set too early, affect our app rotate, should remove
    }

  • (void)sdl_presentIOS13 {
    if (![windows containsObject:self.lockWindow]) {
    //self.lockWindow.rootViewController = self.screenshotViewController; // should remove
    }
    }

  • (void)sdl_presentWithAppWindow:(nullable UIWindow *)appWindow {
    self.lockWindow.frame = appWindow.bounds;
    self.lockWindow.rootViewController = self.screenshotViewController; // should set here
    }

  • (void)dismiss {
    weakself.lockWindow.rootViewController = nil; // should set to nil
    weakself.screenshotViewController = nil;
    }

@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 3, 2019

Hi @84798012, are you sure that you used the PR and branch I mentioned? Several of your suggested changes were already implemented. If you did use that branch and it's not working, I must unfortunately ask for you to create either (1) a PR yourself, or (2) a test project that reproduces your issue so that I can test if my code changes are fixing your issue.

(2) is preferred. Without that, I am unable to figure out what you're doing in your code that's causing your issue and I am unable to fix it. I will wait for your response, thank you!

@84798012
Copy link
Author

@84798012 84798012 commented Dec 4, 2019

@joeljfischer
Thanks for you reply. I'm sure I used #1483, I can see only my 4th suggestion were fixed in the code, other 3 need to change not did. You can just reference my suggestion, and you can decide how to fix the issue.
I have update a demo for you to reproduce the issue.
Here follows the demo link, I changed a little code base on your SDL demo. Detail please see that comments.
https://github.com/84798012/SDL_Window_Rotate_Bug_Reproduce_Demo

@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 4, 2019

@84798012 Thank you for creating the example app. However, in my testing with it I am not seeing any rotation. Was there supposed to be rotation? It appears that you modified the Obj-C example, so I ran that. I attempted to rotate, but nothing occurred. I then activated the lock screen and attempted to rotate, and nothing occurred. Could you please list out clearly the steps to see the bug you have reported in the example project you have sent?

@84798012
Copy link
Author

@84798012 84798012 commented Dec 4, 2019

@joeljfischer first, make sure your iphone enabled rotate. second, you just need in iAP page tap connect, then rotate your iphone, you will see that a rotation bar on the bellow.

@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 4, 2019

I did all of that and it did not rotate. I can triple check tomorrow if you’d like, but there must be a difference in something here.

@84798012
Copy link
Author

@84798012 84798012 commented Dec 4, 2019

I double check the project I uploaded, it can reproduce, please see the details on that links comments. And is it from 1483 code branch that I uploaded file?

@84798012
Copy link
Author

@84798012 84798012 commented Dec 5, 2019

@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 5, 2019

@84798012 Thank you for the video, I now understand the issue. I was looking for the entire VC to be rotating when it shouldn't. I have updated #1483, and it seems to resolve the issue on your test project. If you could please confirm that it fixes your issue, that would be great!

@84798012
Copy link
Author

@84798012 84798012 commented Dec 6, 2019

seems this issue been fixed, thanks.

@NicoleYarroch NicoleYarroch mentioned this issue Dec 12, 2019
7 of 7 tasks complete
@84798012
Copy link
Author

@84798012 84798012 commented Dec 18, 2019

fixed, so close it.

@84798012 84798012 closed this Dec 18, 2019
@joeljfischer joeljfischer reopened this Dec 18, 2019
@joeljfischer
Copy link
Member

@joeljfischer joeljfischer commented Dec 18, 2019

@84798012 Could you please re-test this bug using PR #1509? We ended up doing a large refactor of the lock screen code and would like to confirm that it still fixes all of your issues. Thank you!

@84798012
Copy link
Author

@84798012 84798012 commented Dec 23, 2019

I have test on 1509, the issue fixed. Thanks!

@joeljfischer joeljfischer mentioned this issue Jan 7, 2020
3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants