Mijick / Popups

Popups, popovers, sheets, alerts, toasts, banners, (...) presentation made simple. Written with and for SwiftUI.
Apache License 2.0
1.38k stars 59 forks source link

[BUG] 3.0.0 crash in sheet View #147

Closed zjinhu closed 5 days ago

FulcrumOne commented 1 month ago

Hey,

Could you send me the details about the crash (what exactly crashed and how to reproduce it)

Thanks, Tomasz

FulcrumOne commented 1 month ago

Hey @zjinhu,

Sorry for interrupting, but any further description would be extremely helpful - I tried to debug your problem, but unfortunately no crash appeared.

zjinhu commented 1 month ago

crash

Carlegk commented 1 month ago
image

Hey, When a sheet is open, the app crashes. However, on pages without a sheet, there are no issues.

FulcrumOne commented 1 month ago

thanks. I have a suspicion - I know I made a mistake and all the calculations are done in the same thread (which I plan to fix early next month); it is possible that in this case the function calculating the height of the popup overloads the thread by looping. And this is my guess (however I didn't observe this behavior when developing the update). To confirm this, I would have to ask you to produce a runable code that I can use to observe exactly the same behavior.

Thank you again for your report and have a good day, Tomasz

Carlegk commented 1 month ago

Hey, This is the error demonstration after I simplified my code. I may not have written it correctly. Thank you.

import SwiftUI
import MijickPopups

struct ContentView: View {
    @StateObject private var viewModel: ViewModel = ViewModel()

    var body: some View {
        VStack {
            Image(systemName: "globe")
                .imageScale(.large)
                .foregroundStyle(.tint)
            Text("Hello, world!")
        }.onAppear{
            viewModel.testError()
        }
    }
}

extension ContentView{
    class ViewModel: ObservableObject{
        @Published var errorManager: ErrorManager = ErrorManager()

        func testError(){
            // no crash
            // ErrorPopupView().present()

            // no crash
            // errorManager.triggerError()

            // crash
            Task {
                errorManager.triggerError()
            }
        }
    }
}

class ErrorManager: ObservableObject {
    func triggerError() {
        ErrorPopupView().present()
    }
}

struct ErrorPopupView: CentrePopup {
    var body: some View {
        createContent()
    }

    func createContent() -> some View {
        Button(action: {
            dismissAllPopups()
        }){
            Text("dismiss")
        }
    }
}
FulcrumOne commented 1 month ago

Thanks @Carlegk,

At first glance, it seems that this is indeed a problem with the main thread; I'll investigate this more thoroughly over the weekend. But if my suspicions prove true, a fix for this bug is on my schedule for early November (super sorry for that, but I'm busy with other things right now)

FulcrumOne commented 2 weeks ago

Hey! Here's a quick update about the problem:

As I suspected, there is a problem with threads; since Xcode 16, the View protocol has gained an @MainActor attribute by default, therefore all methods declared in the View are @MainActor.

Popup conforms to View, which means that all its methods (including .present()) are @MainActor, and should therefore be called from the main thread, hence this crash.

The solution is kinda easy - just make sure that these methods are called from the main thread, a.k.a. use DispatchQueue.main.async methods, BUT I would rather advise you to wait a few days and I will provide a more complex solution to this problem (so eventually you won't have to change anything in your code).

Thanks for your patience, Tomasz

Kejiasir commented 2 weeks ago

@FulcrumOne Hello, I have the same problem. After I updated to version 3.0.2, the app crashed immediately when the pop-up view appeared. 20241105142404

FulcrumOne commented 2 weeks ago

@FulcrumOne Hello, I have the same problem. After I updated to version 3.0.2, the app crashed immediately when the pop-up view appeared.

20241105142404

Hey, see the previous post in which I explained everything 😉

fayharinn commented 2 weeks ago

Hi, I'm using DispatchQueue.main.async to call MyView.present(), there is still a crash Thread 1: Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value updatePopupAction(popup.settingHeight(newHeight))

To reproduce :

bukira commented 2 weeks ago

I have the same crash but iOS 15 and 16 only, working fine in iOS 17, and 18

FulcrumOne commented 2 weeks ago

Hi,

I'm currently working on this and it should be ready within a few days. I'll keep you in the loop.

Have a nice day, Tomasz

FulcrumOne commented 1 week ago

Hey!

A brief update on the situation - I have completed most of the work on the upcoming update (4.0.0) and you can test its beta version by switching to the branch patch-4.0.0 in your projects.

To solve the problem, I had to make sure that the present() and dismiss() methods are called from the main thread - to do this I unfortunately had to mark them as async, which results in you having to present the popup with code like:

Task { 
    await YourPopup().present()
}

I am very sorry about this, but alas, it was the only solution. I confirmed it with the code delivered by @Carlegk and it works (btw. I am very grateful for this example as it has shortened my work considerably).

Let me know if it resolves your problems and have a nice day, Tomasz

PS. I plan to publish this version by the end of this week, preferably on Saturday.