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

State management breaks on quick opening/closing of the popup #29

Closed
vamsii777 opened this issue Dec 10, 2023 · 10 comments
Closed

State management breaks on quick opening/closing of the popup #29

vamsii777 opened this issue Dec 10, 2023 · 10 comments
Labels

Comments

@vamsii777
Copy link

Describe the Bug:

When interacting with the LNPopupUI in my SwiftUI application, specifically when the audio is playing and I either drag down the popup or click the close button on the popup, the tab view and popup view turn black. The audio continues to play as expected, but the visual elements of the popup and tab view do not render correctly.

To Reproduce:

Steps to reproduce the behavior:

  1. Start audio playback in the app using LNPopupUI.
  2. Interact with the popup by either dragging it down or clicking the close button.
  3. Observe that the tab view and popup view turn black while the audio continues to play.

Expected Behavior:

The popup and tab view should maintain their proper visual appearance when interacted with, without turning black, while allowing the audio to continue playing.

Screenshots:

RPReplay_Final1702224398.MP4

Additional Context:

  • The issue occurs in a SwiftUI application.
  • Using LNPopupUI for popup management.
  • The problem seems related to view rendering and state management in conjunction with LNPopupUI.
  • The issue is consistently reproducible under the described conditions.

Environment:

  • SwiftUI version: 5
  • LNPopupUI version: 1.7.0
  • iOS version: 17.1.2
  • Xcode version: 15.0.1 (15A507)
  • Device/Emulator type and iOS version: iPhone 11
@vamsii777 vamsii777 added the bug label Dec 10, 2023
@LeoNatan
Copy link
Owner

Hello,
This looks like a user state management bug. Please try to reproduce the issue on a clean example project (or the included one with this repo) and attach here.
Thanks

@LeoNatan
Copy link
Owner

Ping.

Thanks

@vamsii777
Copy link
Author

Yes, I will follow your instructions by replicating the issue on a clean sample project and will attach it.

@vamsii777
Copy link
Author

Hey @LeoNatan,

Attached is an example project LNPopupNetworkExample.zip
demonstrating the issue where dragging down the popup while playing on network audio stream causes it to vanish unexpectedly.

Thank you for looking into this.

@LeoNatan
Copy link
Owner

Hello @vamsii777
I took a short look at the demo project. It's not a small one. While I haven't investigated the ins and outs of it, and I am afraid I won't be able to, I do find that my instinct was correct; it is a state management issue. For example, removing the isPopupOpen: $audioPlaybackViewModel.isPopupOpen portion in ContentView.swift is enough to remove the issue (but might break your logic). I did notice, in your code you set isPopupOpen in multiple places. What I suspect that might be happening is that the underlying framework, LNPopupController, is not dealing with multiple changes in state as gracefully as it should, and something breaks. However, this is not a priority for the framework, and debugging these issues are very difficult, especially with a complex project as your example one. Before SwiftUI support was introduced with LNPopupUI, the LNPopupFramework framework considered state changes while transitioning to be user error. This was no longer possible in SwiftUI due to the nature of the beast, and I tried making everything as cancellable and reversible as possible, but it's not an easy task, and there seem to be some issues left.

I suggest leaving the popup open/close to the user, and at most, closing the popup when your playback ends. But it seems to me, at least from a cursory look, that you set it to closed and then open multiple times in very short times, creating the issue. If you must, use the onOpen and onClose to track the state of popup, and queue your state changes according to those, but, again, I do not suggest doing that (especially not in something like SwiftUI+LNPopupUI).

I will keep this issue open, but not sure if/when I will be able to improve it.

@LeoNatan LeoNatan changed the title Black Screen Issue on Popup Interaction During Audio Playback State management breaks on quick opening/closing of the popup Dec 22, 2023
@LeoNatan
Copy link
Owner

So, I've been looking at how SwiftUI solves UIKit transitions, because it seems UIKit has the same issues as LNPopupController. For example, attempting to present and dismiss a view controller right away causes a crash. Yet, in SwiftUI, the system finishes the presentation and then dismisses.

So that gave me an idea to implement a similar queueing system in LNPopupController (and thus in LNPopupUI).

For example, the following code:

- (void)_start
{
	__weak __typeof(self) weakSelf = self;
	
	[self _firstContentController];
	[self _presentBar];
	[self _dismissBar];
	[self _presentBar];
	[self _dismissBar];
	[self _presentBar];
	[self _openPopup];
	[self _secondContentController];
	[self _presentBar];
	[self _dismissBar];
	[self _presentBarAndOpen];
	[self _closePopup];
	[self _openPopup];
	[self _dismissBarCompletionHandler:^{
		[weakSelf _afterSecond:^{
			[weakSelf _start];
		}];
	}];
}

creates the following behavior:

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2023-12-24.at.23.53.24.mp4

That's not to say that this solves your issue, because if your code remains as it is, your users will see a lot of transitions they shouldn't, but I am happy with this result as a stop gap between nothing (current implementation) and a complete interruptibility (which I also tried, but so far haven't had a satisfactory result).

@vamsii777
Copy link
Author

Acknowledged the recent developments and insights on the state management issue with LNPopupController. The proposed queueing system, inspired by SwiftUI's handling of UIKit transitions, seems like a strategic solution to address the rapid state changes and their impact on rendering. This approach could provide a more stable and consistent user experience, effectively resolving the visual anomalies observed during popup interactions. Eager to see how this implementation enhances the framework's robustness in upcoming releases.

@LeoNatan
Copy link
Owner

I hope to release a new version soon. Want to test locally to try and catch any regressions that might have been introduced. Nothing so far.

@LeoNatan
Copy link
Owner

LeoNatan commented Jan 5, 2024

Hello,
I released LNPopupController 2.18.0 yesterday, which includes what we discussed before. Please update your dependencies, and try it out.
Thanks

@LeoNatan LeoNatan closed this as completed Jan 5, 2024
@LeoNatan
Copy link
Owner

LeoNatan commented Jan 7, 2024

Hello, when investigating #31 (which was a similar issue to this), I added an improvement on top of the previous release, which now makes things much better. Please try it out.

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

No branches or pull requests

2 participants