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

UIButton action in bar controller is ignored in 2.10.0 #397

Closed MacMark closed 4 years ago

MacMark commented 4 years ago

Description

UIButton action not triggered anymore in CustomBarViewController. It worked in 2.8.5 but not in 2.10.0 anymore.

Steps to Reproduce

Use an UIButton in the UIViewController that you use for setCustomBarViewController. Touch the button. The action of that button is not triggered. Instead the full screen controller is presented.

Device, OS and Xcode Versions

iPhone X, iOS 14, Xcode 12 beta 4

LeoNatan commented 4 years ago

Hmm

Thanks, will have a look soon!

LeoNatan commented 4 years ago

Sorry, I can't reproduce.

In the demo app, I added the following code to the custom bar controller's viewDidLoad() method:

if #available(iOS 14.0, *) {
    let button = UIButton(type: .system, primaryAction: UIAction.init(handler: { action in
        print("tapped")
    }))
    button.setTitle("BUTTON", for: .normal)
    button.sizeToFit()
    view.addSubview(button)
}

The button can be tapped.

MacMark commented 4 years ago

When visually debugging the view hierarchy i see an additional empty UIView in front of the custom bar controller with 2.10.x. Could this be an issue?

The working version that i used before i updated to 2.10.x was v2.8.5.

LeoNatan commented 4 years ago

Could you post a view hierarchy dump from Xcode 12? File -> Export View Hierarchy, and post the pointer of the offending view.

But I am not seeing this behavior in the demo app, so I'm not sure if it's something in the framework.

LeoNatan commented 4 years ago

Please provide the hierarchy with the latest release of LNPopupController. Thanks

MacMark commented 4 years ago

Sorry for my late answer, i was busy updating some apps for iOS 14.

The address of the view in the attached hierarchy is 0x7fe1adff7c00. With 2.8.5 this view is not there. Popup.viewhierarchy.zip

popup

Behind that view is the play button which cannot be activated in my case with 2.10.1.

LeoNatan commented 4 years ago

Thank you! I made some changes with later releases. Do you mind providing the same with the latest version? Will help me debug the issue better. Thanks

MacMark commented 4 years ago

This is the same situation recorded with 2.10.5, the play button is not getting the user's touch. Something is intercepting it since 2.10.x (i did not try 2.9.x). The button works when i use to 2.8.5. Printing description of $19: <_LNPopupBarShadowView: 0x7ff83bb294b0; frame = (0 0; 828 61); alpha = 0; autoresize = W+H; userInteractionEnabled = NO; layer = <CALayer: 0x60000267b5e0>> Popup-2-10-5.viewhierarchy.zip

popup-2-10-5

But there's also good news: The layout issues i experienced with 2.10.1 are fixed :-)

LeoNatan commented 4 years ago

Please notice that that view has User Interaction Enabled Off. That is not the view stealing your touches.

LeoNatan commented 4 years ago

I don't see anything in the framework causing this. The demo project works as expected. Try debugging the touch handling to see who finally takes it.

MacMark commented 4 years ago

When i break in LNPopupOpenTapGesutreRecognizer.m at the line if([touch.view isKindOfClass:[UIControl class]]) of this method

- (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldReceiveTouch:(UITouch *)touch
{
    if([touch.view isKindOfClass:[UIControl class]])
    {
        return NO;
    }

    if([self.forwardedDelegate respondsToSelector:_cmd])
    {
        return [self.forwardedDelegate gestureRecognizer:gestureRecognizer shouldReceiveTouch:touch];
    }

    return YES;
}

Then the touch.view does not hold my UIButton with 2.10.5: po touch.view <_LNPopupBarContentView: 0x7fd2e260f100; frame = (0 0; 414 61); layer = <CALayer: 0x6000006b68a0>>

Even if i change that method and return NO the button is not activated.

Do i have to configure the PopUpController differently in 2.10.x than in 2.8.x?

[self.popupBar setCustomBarViewController:self.stickyPlayer];
[self presentPopupBarWithContentViewController:self.fullScreenPlayer animated:animated completion:nil];
self.popupContentView.popupInteractionGestureRecognizer.delegate = self;

But the touch.view does hold my UIButton with 2.8.5: po touch.view <PlayPauseButton: 0x7f8844573670; baseClass = UIButton; frame = (364 9; 40 40); opaque = NO; autoresize = RM+BM; layer = <CALayer: 0x6000003ba880>>

I added

- (BOOL)wantsDefaultTapGestureRecognizer {
    return NO;
}

to my subclass of LNPopupCustomBarViewController and when i try to touch the UIButton

The button is still not activated.

I guess the handling of custom LNPopupCustomBarViewController subclasses is broken in 2.10.x. The demo app does not use a custom LNPopupCustomBarViewController, right?

LeoNatan commented 4 years ago

It does use it. There is a “maps” demo scene which has a custom bar controller. This is where I added a button to test. My money is on something broken on your end. Nothing changed with regards to bar controllers.

iDevelopper commented 4 years ago

Hi @MacMark , I tested too, added a button to the Maps Demo Scene custom bar controller. LNPopupControllerExample[66172].viewhierarchy.zip

Simulator Screen Shot - iPhone 11 Pro Max - 2020-08-27 at 17 55 50

MacMark commented 4 years ago

I do not understand yet what might be wrong in my controller because it works with 2.8.x and just by replacing 2.8.x with 2.10.x it gets into this issue.

I will compare the sample custom bar controller with my controller.

iDevelopper commented 4 years ago

@MacMark , What about LOTAnimationView? Can you provide here the source code?

MacMark commented 4 years ago

Hi @iDevelopper, LOTAnimationView comes from https://github.com/airbnb/lottie-ios/releases/tag/2.5.3

I will also try it with a pure UIButton but for me it looks like the _LNPopupBarContentView does not hold the view of my controller at all in https://github.com/LeoNatan/LNPopupController/issues/397#issuecomment-681822376 when i use 2.10.x.

MacMark commented 4 years ago

I have to find why the UIButton is not in touch.view with 2.10.5 although it is with 2.8.5 and also with the 2.10.5 sample code:

Screenshot 2020-08-28 at 16 34 11 Screenshot 2020-08-28 at 16 30 56 Screenshot 2020-08-28 at 16 27 36
iDevelopper commented 4 years ago

@MacMark , Have you tested without LOTAnimationView?

MacMark commented 4 years ago

When i put a 2nd button in there that does nothing, my PlayPauseButton works and the touch.view contains my PlayPauseButton on the above break point just like with 2.8.5. How is such a side effect possible?

image
LeoNatan commented 4 years ago

The custom bar controller is added directly to the popup up bar content view: https://github.com/LeoNatan/LNPopupController/blob/139a067cba92a3a1fff25c5e5c59c15591e314f0/LNPopupController/LNPopupController/Private/LNPopupBar.m#L1162

@MacMark Let's try an experiment. Could you create a new subclass of the custom bar controller, with no logic in it, just a button? Let's see if that button works.

MacMark commented 4 years ago

As long as the custom controller has also a normal UIButton (it might even be hidden) the PlayPauseButton (subclass of LottieButton which is in turn a subclass of UIButton) works as normal (receives the touch because it is in touch.view). I gave the normal button now an action which just removes the normal button from the view hierarchy.

image

Now the fun part: The PlayPauseButton works as often as i like until i use the normal button: The normal button is removed and now the PlayPauseButton is not recognized anymore. The touch.view is only correct (i. e. contains the touched control, not the _LNPopupBarContentView) as long as i have a direct (not subclassed) UIButton in my controller's view hierarchy.

@LeoNatan To answer your question: This means a normal button works in my controller.

LeoNatan commented 4 years ago

WTF is going on. I can't explain it. Can you check and see if your special button (not the normal one) has a gesture recognizer? Have you added a gesture recognizer elsewhere in your bar controller's view hierarchy?

iDevelopper commented 4 years ago

@MacMark , Can you provide a small project reproducing this issue (with the lottieButton in it)?

MacMark commented 4 years ago

I paused the app via Xcode, selected the PlayPauseButton and then printed its description

image

Printing description of $43: <PlayPauseButton: 0x7fc4f6c152c0; baseClass = UIButton; frame = (364 9; 40 40); opaque = NO; autoresize = RM+BM; layer = <CALayer: 0x600000652bc0>>

(lldb) po 0x7fc4f6c152c0 <PlayPauseButton: 0x7fc4f6c152c0; baseClass = UIButton; frame = (364 9; 40 40); opaque = NO; autoresize = RM+BM; layer = <CALayer: 0x600000652bc0>>

(lldb) po [(id)(0x7fc4f6c152c0) class] PlayPauseButton

(lldb) po [(id)(0x7fc4f6c152c0) superclass] LottieButton

(lldb) po [[(id)(0x7fc4f6c152c0) superclass] superclass] UIButton

@LeoNatan This should be the view hierarchy according to Apple's docs:

(lldb) po [(id)(0x7fc4f6c152c0) recursiveDescription]
<PlayPauseButton: 0x7fc4f6c152c0; baseClass = UIButton; frame = (364 9; 40 40); opaque = NO; autoresize = RM+BM; layer = <CALayer: 0x600000652bc0>>
   | <UIImageView: 0x7fc4f6d09b50; frame = (2 2; 36 36); clipsToBounds = YES; hidden = YES; opaque = NO; userInteractionEnabled = NO; layer = <CALayer: 0x600000603060>>
   | <LOTAnimationView: 0x7fc4f6fef550; frame = (2 2; 36 36); clipsToBounds = YES; userInteractionEnabled = NO; layer = <CALayer: 0x6000007f6800>>
   |    | <LOTCompositionContainer: 0x600003218cc0> (layer)
   |    |    | <CALayer: 0x6000007ef200> (layer)
   |    |    |    | <LOTLayerContainer: 0x600003518870> (layer)
   |    |    |    |    | <CALayer: 0x6000007e8d60> (layer)
   |    |    |    |    |    | <CALayer: 0x6000007eb960> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007d65e0> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007d42e0> (layer)
   |    |    |    | <LOTLayerContainer: 0x600003518900> (layer)
   |    |    |    |    | <CALayer: 0x6000007d7460> (layer)
   |    |    |    |    |    | <CALayer: 0x6000007d43e0> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007d7fa0> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007d6be0> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007d70a0> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007d7b40> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007d6540> (layer)
   |    |    |    | <LOTLayerContainer: 0x600003518990> (layer)
   |    |    |    |    | <CALayer: 0x6000007d06c0> (layer)
   |    |    |    |    |    | <CALayer: 0x6000007defe0> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007dc440> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007dc500> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007debe0> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007defc0> (layer)
   |    |    |    | <LOTLayerContainer: 0x600003518a20> (layer)
   |    |    |    |    | <CALayer: 0x6000007dee20> (layer)
   |    |    |    |    |    | <CALayer: 0x6000007db660> (layer)
   |    |    |    |    |    |    | <CALayer: 0x6000007da240> (layer)
   |    |    |    |    |    |    |    | <CAShapeLayer: 0x6000007da4e0> (layer)

@iDevelopper I will try and provide a small sample reproducing it.

MacMark commented 4 years ago

The problem seems to be that the height of the view of the custom controller shrinks to zero after removing the normal button from superview. It seems to be associated with setPreferredContentSize in LNPopupCustomBarViewController.

image
MacMark commented 4 years ago

This is a minimal project showing my problem. The popup and the play button work fine (it logs a debug output). As soon as the blue button is hit, it removes itself from superview and the play button stops working because the setPreferredContentSize in LNPopupCustomBarViewController seems to go to zero height. This was not the case with 2.8.5. Proof: Replace 2.10.5 in the Podfile with 2.8.5 and run "pod install". Run the project again. And it will work fine. PopupMinTest.zip

image
LeoNatan commented 4 years ago

Do you correctly set the preferredContentSize property of your view controller properly?

LeoNatan commented 4 years ago

Check what size is returned from your calculation.

MacMark commented 4 years ago

It is (lldb) po size (width = 444, height = 61) on iPhone 11 and i set po self.preferredContentSize (width = -1, height = 61) which is UIViewNoIntrinsicMetric x 61.

image

I made a 2nd run and debugged the view height before touching the blue button:

image

And after touching the blue button:

image

The height of the view of the custom controller went to 0 according to the debug view although it is still visible with correct height. But i cannot touch the play button anymore.

MacMark commented 4 years ago

I guess i found the piece of code that sets the frame's height to zero: frame.size.height = state < _LNPopupPresentationStateTransitioning ? _LNPopupBarHeightForBarStyle(_LNPopupResolveBarStyleFromBarStyle(self.popupBar.barStyle), self.popupBar.customBarViewController) : 0.0;

image
iDevelopper commented 4 years ago

The problem seems to be there (LNPopupBar.m line 1161):

Replace:

    _customBarViewController.view.translatesAutoresizingMaskIntoConstraints = NO;
    [self.contentView addSubview:_customBarViewController.view];
    [NSLayoutConstraint activateConstraints:@[
        [self.contentView.leadingAnchor constraintEqualToAnchor:_customBarViewController.view.leadingAnchor],
        [self.contentView.trailingAnchor constraintEqualToAnchor:_customBarViewController.view.trailingAnchor],
        [self.contentView.centerXAnchor constraintEqualToAnchor:_customBarViewController.view.centerXAnchor],
    ]];

with:

    _customBarViewController.view.translatesAutoresizingMaskIntoConstraints = NO;
    [self.contentView addSubview:_customBarViewController.view];
    [NSLayoutConstraint activateConstraints:@[
        [self.contentView.leadingAnchor constraintEqualToAnchor:_customBarViewController.view.leadingAnchor],
        [self.contentView.trailingAnchor constraintEqualToAnchor:_customBarViewController.view.trailingAnchor],
        //[self.contentView.centerXAnchor constraintEqualToAnchor:_customBarViewController.view.centerXAnchor],
        [self.contentView.topAnchor constraintEqualToAnchor:_customBarViewController.view.topAnchor],
        [self.contentView.bottomAnchor constraintEqualToAnchor:_customBarViewController.view.bottomAnchor]
    ]];
MacMark commented 4 years ago

@iDevelopper I replaced the code like you suggested and it works with the demo project and also with my original app 👍 ✌️🎉 (And thus my previous comment was wrong).

iDevelopper commented 4 years ago

@LeoNatan , The Maps Demo Scene works but there is a constraint issue (look at the debug view hierarchy).

However, with a tab bar controller, it not works. The vertical constraints of the custom bar view controller' view are ambiguous, and its height is equal to zero. This is why the button is misplaced (I have set some constraints to it) and is not triggered.

Here is a very small sample project that demonstrates the issue. LNCustomBarTest.zip

LeoNatan commented 4 years ago

It's like this for a reason. The fix should be different. I will take a look soon.

iDevelopper commented 4 years ago

Ah and what is the reason, please?

LeoNatan commented 4 years ago

There were multiple popup bar layout issues without this. I need to understand why the layout is failing in this case, but not in the example project. Could still be a user error.

LeoNatan commented 4 years ago

I think I'm starting to see where the issue is. In my demo scene, the controller has constraints that express its size, where as in the example, this isn't the case.

LeoNatan commented 4 years ago

I'm not sure adding a constraint is the right thing to do. It might just be the case that if you are using auto layout, you should have all the constraints yourself. If I add a height constraint in the framework, it breaks genuine cases where one might want to change the height of the bar using a constraint, and then rely on systemLayoutSizeFittingSize: to calculate the preferredContentSize property.

LeoNatan commented 4 years ago

OK, I've made an improvement. Please test.

The system now takes into account translatesAutoresizingMaskIntoConstraints of the controller's view and lays it in the popup bar accordingly.

MacMark commented 4 years ago

When using 2.10.6 with my app the play button works again ✌️😍

But i'm having again some layout problems like with earlier version of 2.10.x (before 2.10.5): The bottom of the view controller behind the popup bar is covered by the popup bar when the navigation bar on top is hidden. In this screenshot 1 and a half row of the table are hidden by the popup bar which was not the case with 2.10.5

This is 2.10.6:

image

And that is 2.10.5:

image

I would like to have the correct layout of 2.10.5 and the working play button of 2.10.6 😎 @LeoNatan Should i open a new bug?

With the suggested change from @iDevelopper i run into a similar layout problem.

LeoNatan commented 4 years ago

Just to make sure, the issue is that the bottom inset is not correctly set? Please open an issue.

MacMark commented 4 years ago

Yes, it is the bottom inset. Thx, i will open a new issue.