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

UIButton action in bar controller is ignored in 2.10.0 #397

Closed
MacMark opened this issue Aug 18, 2020 · 42 comments
Closed

UIButton action in bar controller is ignored in 2.10.0 #397

MacMark opened this issue Aug 18, 2020 · 42 comments

Comments

@MacMark
Copy link

MacMark commented Aug 18, 2020

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
Copy link
Owner

Hmm

Thanks, will have a look soon!

@LeoNatan LeoNatan added the bug label Aug 18, 2020
@LeoNatan
Copy link
Owner

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
Copy link
Author

MacMark commented Aug 19, 2020

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
Copy link
Owner

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
Copy link
Owner

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

@MacMark
Copy link
Author

MacMark commented Aug 26, 2020

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
Copy link
Owner

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
Copy link
Author

MacMark commented Aug 26, 2020

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
Copy link
Owner

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

@LeoNatan
Copy link
Owner

LeoNatan commented Aug 26, 2020

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
Copy link
Author

MacMark commented Aug 27, 2020

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

  • it first goes to - (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldReceiveTouch:(UITouch *)touch in LNPopupOpenTapGesutreRecognizer.m
  • and then into my wantsDefaultTapGestureRecognizer

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Author

MacMark commented Aug 28, 2020

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
Copy link
Contributor

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

@MacMark
Copy link
Author

MacMark commented Aug 28, 2020

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 #397 (comment) when i use 2.10.x.

@MacMark
Copy link
Author

MacMark commented Aug 28, 2020

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
Copy link
Contributor

@MacMark ,
Have you tested without LOTAnimationView?

@MacMark
Copy link
Author

MacMark commented Aug 28, 2020

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
Copy link
Owner

LeoNatan commented Aug 28, 2020

The custom bar controller is added directly to the popup up bar content view:

[self.contentView addSubview:_customBarViewController.view];

@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
Copy link
Author

MacMark commented Aug 28, 2020

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
Copy link
Owner

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
Copy link
Contributor

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

@MacMark
Copy link
Author

MacMark commented Aug 29, 2020

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
Copy link
Author

MacMark commented Aug 30, 2020

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
Copy link
Author

MacMark commented Aug 30, 2020

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
Copy link
Owner

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

@LeoNatan
Copy link
Owner

Check what size is returned from your calculation.

@MacMark
Copy link
Author

MacMark commented Aug 31, 2020

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
Copy link
Author

MacMark commented Aug 31, 2020

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
Copy link
Contributor

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
Copy link
Author

MacMark commented Aug 31, 2020

@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
Copy link
Contributor

iDevelopper commented Aug 31, 2020

@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
Copy link
Owner

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

@iDevelopper
Copy link
Contributor

Ah and what is the reason, please?

@LeoNatan
Copy link
Owner

LeoNatan commented Sep 1, 2020

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
Copy link
Owner

LeoNatan commented Sep 1, 2020

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
Copy link
Owner

LeoNatan commented Sep 1, 2020

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
Copy link
Owner

LeoNatan commented Sep 1, 2020

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
Copy link
Author

MacMark commented Sep 2, 2020

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
Copy link
Owner

LeoNatan commented Sep 2, 2020

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

@MacMark
Copy link
Author

MacMark commented Sep 2, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants