LeoNatan / LNPopupController

A framework for presenting view controllers as popups of other view controllers, much like the Apple Music and Podcasts apps.
MIT License
3.04k stars 342 forks source link

iOS 13 Support #334

Closed JacobSyndeo closed 5 years ago

JacobSyndeo commented 5 years ago

Description

When presenting a popupController on iOS 13, the application crashes. The device log says:

Terminating app due to uncaught exception 'NSGenericException', reason: 'Access to _UIBarBackground's _shadowView ivar is prohibited. This is an application bug'

An exception breakpoint reveals the issue to be caused in LNPopupController.m:924:

self.popupBar.systemShadowColor = [_bottomBar valueForKeyPath:str2];

So apparently we can't set that property anymore.

Steps to Reproduce

Present a popupBar on iOS 13

Device, OS and Xcode Versions

iPhone Xs iOS 13 beta 1 Xcode 11 beta 1

LeoNatan commented 5 years ago

Thanks I will sherlock your thread for the entire initial iOS 13 support tracking.

I will start looking at this soon, once the iOS beta and Xcode are a bit more stable.

MacMark commented 5 years ago

I have this issue too:

[General] CRASH: Access to _UIBarBackground's _shadowView ivar is prohibited. This is an application bug

8   LNPopupController                   0x00000001032d6d20 -[LNPopupController _configurePopupBarFromBottomBar] + 1396
9   LNPopupController                   0x00000001032d9a18 -[LNPopupController presentPopupBarAnimated:openPopup:completion:] + 1168
10  LNPopupController                   0x00000001032ebac8 -[UIViewController(LNPopupSupport) presentPopupBarWithContentViewController:openPopup:animated:completion:] + 284
11  LNPopupController                   0x00000001032ebb94 -[UIViewController(LNPopupSupport) presentPopupBarWithContentViewController:animated:completion:] + 144

It seems to be an already known issue with previous iOS versions because the code obfuscates the acccess to the shadowView ivar here in _configurePopupBarFromBottomBar:


#ifndef LNPopupControllerEnforceStrictClean
    //backgroundView
    static NSString* const bV = @"X2JhY2tncm91bmRWaWV3";
    //backgroundView.shadowView.backgroundColor
    static NSString* const bVsVbC = @"YmFja2dyb3VuZFZpZXcuc2hhZG93Vmlldy5iYWNrZ3JvdW5kQ29sb3I=";

    NSString* str1 = _LNPopupDecodeBase64String(bV);

    if([_bottomBar respondsToSelector:NSSelectorFromString(str1)])
    {
        NSString* str2 = _LNPopupDecodeBase64String(bVsVbC);

        if([[NSProcessInfo processInfo] operatingSystemVersion].majorVersion >= 10)
        {
            self.popupBar.systemShadowColor = [_bottomBar valueForKeyPath:str2];
        }
        else
        {
            self.popupBar.systemShadowColor = [UIColor lightGrayColor];
        }
    }
#endif

Looks like iOS 13 cannot be tricked by this anymore ;-)

Possible workarounds:

LeoNatan commented 5 years ago

There is always object_getIvar(). 😂

LeoNatan commented 5 years ago

I will look into this soon.

MacMark commented 5 years ago

iOS 13 has the card appearance style as a default for modal presentations. Maybe an option to configure the appearance style of the popup would be useful to enforce the "old" look of the popup?

LeoNatan commented 5 years ago

The popup does not currently use the normal presentation methods of iOS (e.g., UIPresentationController), so it shouldn't trigger that card presentation. Since I haven't looked at the iOS 13 runtime at all to see how this card presentation is achieved, I cannot say whether it is possible to use for the popup in a way that does not require rewriting everything.

In any way, SwiftUI support seems out of the question, as they completely abstract view controllers from the user, despite using them heavily themselves.

iDevelopper commented 5 years ago

@MacMark , Actually the framework does not use presentViewController:animated method for presenting the popup content view controller.

MacMark commented 5 years ago

Yes, you're right, the popup is not affected by the card stuff and looks the same like before. My mistake.

iDevelopper commented 5 years ago

@LeoNatan , they just added a new value for the modalPresentationStyle property (UIModalPresentationAutomatic).

A presentation style chosen by the system. And this is the default value.

LeoNatan commented 5 years ago

@iDevelopper So there is no way to reproduce this manually using some UIPresentationController subclass? It's all private?

iDevelopper commented 5 years ago

@LeoNatan , no way. It is like it was before except this new value for modalPresentationStyle property.

LeoNatan commented 5 years ago

Thanks

iDevelopper commented 5 years ago

iOS 13 beta: Also, It seems that Apple has removed the swipe gesture to present the maxi player in Music App. But she was not very good in fact ... A lot of glitch effects...

LeoNatan commented 5 years ago

You mean drag?

iDevelopper commented 5 years ago

Yes from the popup bar to open the popup content view controller by dragging up.

iDevelopper commented 5 years ago

But it is the first beta and in my implementation, the drag does not work properly when modalPresentationStyle is set to the default value (UIModalPresentationAutomatic) for the presenting view controller. They will surely correct...

LeoNatan commented 5 years ago

I've found the issue and will push a fix soon. I've also noticed another issue, where trying to dismiss the popup actually dismisses the container controller itself, due to the new presentation stuff by Apple.

LeoNatan commented 5 years ago

iOS 13 is presenting … challenges. 🙃

LeoNatan commented 5 years ago

Dark mode support will be coming later, as there are currently bugs in the system: https://twitter.com/smileyborg/status/1137511042949365761

LeoNatan commented 5 years ago

Pushed a fix on master with support for iOS 13.

LeoNatan commented 5 years ago

Added support for iOS 13 dark mode. There is one issue that I know of that I need to fix, and that's UITabBarController+UINavigationController pop animation.

LeoNatan commented 5 years ago

5DA55869-A29B-4725-9CBE-D4C4264176F8

Quick GIF from the phone running beta 2.

LeoNatan commented 5 years ago

v2.7.2 is out with initial support for iOS 13 (up to and including beta 3) Please test and report back how it works for you. Thanks

funkenstrahlen commented 5 years ago

I am running 2.7.3 for some time now in my app and it runs perfectly fine without any issues. Thanks and thumbs up from my side! 😊

LeoNatan commented 5 years ago

Thank you!

LeoNatan commented 5 years ago

Heh

Screen Shot 2019-08-21 at 20 43 29
iDevelopper commented 5 years ago

About this screenshot. What is it for?

LeoNatan commented 5 years ago

https://www.apple.com/macos/catalina-preview/

iDevelopper commented 5 years ago

Do not understand, missing something?

funkenstrahlen commented 5 years ago

Do not understand, missing something?

He is happy this code works on Catalina via Catalyst.

LeoNatan commented 5 years ago

I worked on it further. It works great now on macOS. I don't believe it's a good ux for a Mac app, but if someone wants to use it, they can.

iDevelopper commented 5 years ago

Sorry, I have not looked enough. I did not notice the three buttons on the Mac interface. But this this thread concerns iOS 13 however. So, bravo! I'll have a look with a Mac updated with the Catalina beta. It will be better with a splitViewController, yes.

What are the things to modify so that the interfarce user also appears on Mac OS X while the framework was developed for iOS?

LeoNatan commented 5 years ago

You enable macOS as a platform in the project. Then your iOS app runs on the Mac like it’s an iPad app. It doesn’t look like a Mac app but runs.

LeoNatan commented 5 years ago

I suggest you look at WWDC videos on the subject.

iDevelopper commented 5 years ago

Ah ok, thanks!

It is like an iPhone application can be run on a iPad in a emulate mode?

iDevelopper commented 5 years ago

Hi @LeoNatan , I don't understand what is for these delegate methods (never called):

- (nullable UIContextMenuConfiguration *)tableView:(UITableView *)tableView contextMenuConfigurationForRowAtIndexPath:(NSIndexPath *)indexPath point:(CGPoint)point API_AVAILABLE(ios(13.0))
{
    return [UIContextMenuConfiguration configurationWithIdentifier:@"Preview" previewProvider:^ UIViewController* {
        NSString* cellIdentifier = [tableView cellForRowAtIndexPath:indexPath].reuseIdentifier;
        id segueTemplate = [self _segueTemplateWithIdentifier:cellIdentifier];
        return [segueTemplate instantiateOrFindDestinationViewControllerWithSender:self];;
    } actionProvider:nil];
}

- (void)tableView:(UITableView *)tableView willPerformPreviewActionForMenuWithConfiguration:(UIContextMenuConfiguration *)configuration animator:(id<UIContextMenuInteractionCommitAnimating>)animator API_AVAILABLE(ios(13.0))
{
    UIViewController* vc = animator.previewViewController;

    [animator addCompletion:^{
        [self presentViewController:vc animated:YES completion:nil];
    }];
}
LeoNatan commented 5 years ago

Again, please watch the WWDC videos describing new iOS 13 features. UIContextMenuConfiguration is used in the demo project as an example for users, and is most definitely called on iOS 13.

iDevelopper commented 5 years ago

I wanted to say that in your sample, these two UITableView delegate methods are never called (of course I test with iOS 13).

The delegate methods of UIContextMenuInteractionDelegate are called.

LeoNatan commented 5 years ago

Try long pressing a cell, then tap the preview.

iDevelopper commented 5 years ago

Ok, my apologies! I was expecting after a long pressing on a music cell...

LeoNatan commented 5 years ago

But it's not in the same class. :) The music app sits in Swift land.

iDevelopper commented 5 years ago

Yes sorry, and? We can't do it in the Music app in Swift?

LeoNatan commented 5 years ago

What? Why not? You pointed to code in an Objective C class unrelated to Music, and claimed it was never called.

iDevelopper commented 5 years ago

Yes you are right but I think the best place for this demo should be in the music demo, no?

LeoNatan commented 5 years ago

The whole point why I added that was because in the storyboard, I have the old peek/pop API (3D touch), but there is no such API currently for context menus. So I wanted to have the same functionality as before.

I also added the context menu in the normal stuff.

In iOS music app, 3D touching a song displays an activity view controller, not a preview.

iDevelopper commented 5 years ago

Hi @LeoNatan ,

Nothing major, but I wanted to show it to you (Mac OS Dark Mode):

Capture d’écran 2019-09-08 à 06 58 20 Capture d’écran 2019-09-08 à 07 07 15
iDevelopper commented 5 years ago

Also your navigation controllers are gone:

Capture d’écran 2019-09-08 à 07 21 30
LeoNatan commented 5 years ago

I don't know why it looks like that on your end on macOS. It's not how it looks for me. Make sure you are running on the latest version of macOS Catalina and compiling with the latest version of Xcode.

The navigation bar is not gone, it's the new navigation appearance stuff they introduced. Please read the docs: https://developer.apple.com/documentation/uikit/uinavigationitem?changes=latest_minor&language=objc

Image-2

LeoNatan commented 5 years ago

Hmm just reproduced the color issue on Catalina. Will fix @iDevelopper. Thanks!

LeoNatan commented 5 years ago

This seems like a bug in either Xcode or macOS. Wasn't there when I was working on v3 two weeks ago...