Mijick / Popups

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

Caching data of previously presented screen #101

Closed jay-jay-lama closed 3 months ago

jay-jay-lama commented 6 months ago

Tap on different items opens screen with cached information To fix it is needed to use a pause with 0.4 sec before opening the new screen

Simulator Screen Recording - iPhone 12 - 2024-05-26 at 13 15 03

FulcrumOne commented 5 months ago

resolved

dentvii commented 5 months ago

I can confirm and was able to reproduce the issue at 2.4.2 Just add this to the demo app replacing the top of the ContentView.


import Combine

// Step 1: Define the global products dictionary
let MockProductDictionary: [Int: [String]] = [
    1: ["A", "B", "C"],
    2: ["D"],
    3: ["X", "Z"]
]

actor SubscriptionViewModel: ObservableObject {
    @MainActor @Published private(set) var selectedProducts: [String]?

    init() {
        Task{
            await getRandomProduct()
        }
    }

    private func getRandomProduct() async {
        if let randomKey = MockProductDictionary.keys.randomElement(),
           let values = MockProductDictionary[randomKey] {
          print("Random key is \(randomKey) so values are \(values)")
            await self.updateProduct2(values)
        }
    }

    @MainActor
    func updateProduct2(_ values: [String]) {
        self.selectedProducts = values
    }
}

struct SubscriptionView: View {
    @StateObject private var subVM : SubscriptionViewModel

    init() {
        self._subVM = StateObject(wrappedValue: SubscriptionViewModel())
    }

    var body: some View {
        VStack {
            Text("Selected Products:")
                .font(.headline)
            if let selectedProducts = subVM.selectedProducts {
                List(selectedProducts, id: \.self) { product in
                    Text(product)
                }
            }

        }
    }
}

struct BottomPopupSub_Fullscreen: BottomPopup {

    func configurePopup(popup: BottomPopupConfig) -> BottomPopupConfig {
        popup
            .contentFillsEntireScreen(true)
            .dragGestureEnabled(false)
    }
    func createContent() -> some View {
        VStack(spacing: 0) {
            Spacer.height(12)
            Text("Close")
                .onTapGesture {
                    PopupManager.dismissAll()
                }
            Spacer.height(16)
            SubscriptionView()
        }
        .frame(maxHeight: .infinity, alignment: .top)

    }
}

struct ContentView: View {
    var body: some View {
        ScrollView(showsIndicators: false, content: createContent).preferredColorScheme(.light)
    }
}

private extension ContentView {
    func createContent() -> some View {
        VStack(spacing: 44) {
            Text("Show Subs")
                .onTapGesture {
                    BottomPopupSub_Fullscreen()
                        .showAndStack()
                }
            createSection(Data.Bottom.self)
            createSection(Data.Centre.self)
            createSection(Data.Top.self)
        }
        .padding(.top, 40)
        .padding(.bottom, 32)
        .padding(.horizontal, 28)
    }
}

If you verify the video bellow, even thou we randomize and instaciate a new subscriptionview every time it is created, if the viewmodel is still around, a new one is not generated (as no print statement is provided). Unfortunatelly, this may not have to do with your awesome library, but with swift itself. If anyone has any suggestions please advise. ADding UUID does not work :/

https://github.com/Mijick/PopupView/assets/87960/60026ad0-708f-40ff-b146-2d2d005a3b46

dentvii commented 5 months ago

@FulcrumOne It kept in my mind that I hadn't tested throughly. I decided to compare against Apple FullScreen and when comparing with PopupView it does have an issue regarding some sortof cache issue. Bellow you can compare PopupView and Apples.

If possible I would re-open this issue.


import SwiftUI
import MijickPopupView

import Combine

// Step 1: Define the global products dictionary
let MockProductDictionary: [Int: [String]] = [
    1: ["A", "B", "C"],
    2: ["D"],
    3: ["X", "Z"]
]

actor SubscriptionViewModel: ObservableObject {
    @MainActor @Published private(set) var selectedProducts: [String]?

    init() {
        Task{
            await getRandomProduct()
        }
    }

    private func getRandomProduct() async {
        if let randomKey = MockProductDictionary.keys.randomElement(),
           let values = MockProductDictionary[randomKey] {
          print("Random key is \(randomKey) so values are \(values)")
            await self.updateProduct2(values)
        }
    }

    @MainActor
    func updateProduct2(_ values: [String]) {
        self.selectedProducts = values
    }
}

struct SubscriptionView: View {
    @StateObject private var subVM : SubscriptionViewModel

    init() {
        self._subVM = StateObject(wrappedValue: SubscriptionViewModel())
    }

    var body: some View {
        VStack {
            Text("Selected Products:")
                .font(.headline)
            if let selectedProducts = subVM.selectedProducts {
                List(selectedProducts, id: \.self) { product in
                    Text(product)
                }
            }

        }
    }
}

struct SubscriptionAppleView: View {
    @StateObject private var subVM : SubscriptionViewModel
    @Binding private var showFullScreen : Bool

    init(showFullScreen : Binding<Bool>) {
        self._subVM = StateObject(wrappedValue: SubscriptionViewModel())
        self._showFullScreen = showFullScreen
    }

    var body: some View {
        VStack(spacing: 0) {
            Spacer.height(12)
            Text("Close")
                .onTapGesture {
                    self.showFullScreen = false
                }
            Spacer.height(16)
            VStack {
                Text("Selected Products:")
                    .font(.headline)
                if let selectedProducts = subVM.selectedProducts {
                    List(selectedProducts, id: \.self) { product in
                        Text(product)
                    }
                }

            }
        }
        .frame(maxHeight: .infinity, alignment: .top)
    }
}

struct BottomPopupSub_Fullscreen: BottomPopup {

    func configurePopup(popup: BottomPopupConfig) -> BottomPopupConfig {
        popup
            .contentFillsEntireScreen(true)
            .dragGestureEnabled(false)
    }
    func createContent() -> some View {
        VStack(spacing: 0) {
            Spacer.height(12)
            Text("Close")
                .onTapGesture {
                    PopupManager.dismissAll()
                }
            Spacer.height(16)
            SubscriptionView()        }
        .frame(maxHeight: .infinity, alignment: .top)

    }
}

struct ContentView: View {
    @State private var showFullScreen : Bool = false

    var body: some View {
        ScrollView(showsIndicators: false, content: createContent).preferredColorScheme(.light)
    }
}

private extension ContentView {
    func createContent() -> some View {
        VStack(spacing: 44) {
            Text("Show Subs")
                .onTapGesture {
                    BottomPopupSub_Fullscreen()
                        .showAndStack()
                }

            Text("Show Apple Subs")
                .onTapGesture {
                    self.showFullScreen = true
                }
            createSection(Data.Bottom.self)
            createSection(Data.Centre.self)
            createSection(Data.Top.self)
        }
        .padding(.top, 40)
        .padding(.bottom, 32)
        .padding(.horizontal, 28)
        .fullScreenCover(isPresented: $showFullScreen, content: {
            SubscriptionAppleView(showFullScreen : $showFullScreen)
        })
    }
}
FulcrumOne commented 5 months ago

@dentvii, thanks.

We discussed this problem with @jay-jay-lama and it is quite a complicated issue; @State needs about 0.48 seconds to release memory from a previously closed view. In previous versions we solved this problem by speeding up the closing animation, however, this resulted in an animation that was not very appealing. From version 2.2.0, we decided to sacrifice a bit of safety in favour of appearance, because we found that the problem would only occur if the user tried to open the same popup within 0.3 seconds after closing it.

Nevertheless, I agree with you that we should reopen this Issue, as we are planning to refactor the code in version 3.0.0; therefore maybe we can find a better solution to this problem (if you have any ideas, feel free to contribute).

Thanks again and have a great day, T.K.

dentvii commented 5 months ago

Yeah, sure it is a dev focused on testing type indeed.

My best guess for a workaround would be adding a date to the string that identifies the popup, but this might conflict with dismissing popups of a certain type.

I will try latter and return Thanks!

‘’’ public extension Popup { var id: String { “.init(describing: Self.self)(Date())” } var body: V { createContent() }

func configurePopup(popup: Config) -> Config { popup }

} ‘’’

Get Outlook for iOShttps://aka.ms/o0ukef


From: Zaphod Beeblebrox @.> Sent: Thursday, June 13, 2024 4:13:19 PM To: Mijick/PopupView @.> Cc: Felipe Veiga @.>; Mention @.> Subject: Re: [Mijick/PopupView] Caching data of previously presented screen (Issue #101)

@dentviihttps://github.com/dentvii, thanks.

We discussed this problem with @jay-jay-lamahttps://github.com/jay-jay-lama and it is quite a complicated issue; @State needs about 0.48 seconds to release memory from a previously closed view. In previous versions we solved this problem by speeding up the closing animation, however, this resulted in an animation that was not very appealing. From version 2.2.0, we decided to sacrifice a bit of safety in favour of appearance, because we found that the problem would only occur if the user tried to open the same popup within 0.3 seconds after closing it.

Nevertheless, I agree with you that we should reopen this Issue, as we are planning to refactor the code in version 3.0.0; therefore maybe we can find a better solution to this problem (if you have any ideas, feel free to contribute).

Thanks again and have a great day, T.K.

— Reply to this email directly, view it on GitHubhttps://github.com/Mijick/PopupView/issues/101#issuecomment-2166589020, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAVPGB7RRU5WPHJ2HCMG4LZHHVM7AVCNFSM6AAAAABIJWTWVSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGU4DSMBSGA. You are receiving this because you were mentioned.Message ID: @.***>

FulcrumOne commented 3 months ago

@dentvii, Should be fixed in the patch-2.5.1 branch