bambuser / BambuserPlayerSDK-iOS

SDK of Bambusers live video shopping player for iOS
7 stars 1 forks source link

Close button behaviour complications #11

Open joshuapoq opened 3 months ago

joshuapoq commented 3 months ago

Description

UPDATE: The close button logic is misleading but the button does work when PIP is disabled. However the current way the close button works is not really enough for users to be able to close the view without us adding our own way to close it. See comment below.

Using the closeButton: .visible UI configuration shows a down chevron in the top right corner. ~Tapping that chevron does nothing (no event is fired)~. It's not clear if the chevron relates to PiP or is meant as a modal dismiss close button.

Also the API to handle close is a bit tricky from the event closure. It might be better to switch to delegates like you are doing for the new API. See example below where I need to capture a closure to handle close (UIKit).

Versions

Code

class BambuserFactory {
    func makeBambuserViewController(showId: String) -> UIViewController {
        var close: (() -> Void)?
        let viewController = BambuserPlayerViewController(showId: showId, config: .init(uiConfig: .init(closeButton: .visible))) { event in
            switch event {
            case .close:
                close?()

            default:
                print(event) // No event occurs when tapping the chevron.
            }
        }
        close = { [weak viewController] in viewController?.close() }
        return viewController
    }
}
saeidbasirnia commented 3 months ago

Hi,

chevron only is shown if PiP is enabled in configuration setup: PlayerConfiguration.pipConfig.isEnabled == true. Please note that PIP is not working in iPhone simulator at this point, you either need to test it on iPad or a real device In this case, closing this view, should be handled by the developer. if PlayerConfiguration.pipConfig.isEnabled == false, then there will be a X, at the top right corner. Also thanks for your suggestion regarding the API and we'll look into this.

joshuapoq commented 3 months ago

Thanks for the explanation, that does make sense! I will test this out.

saeidbasirnia commented 1 month ago

I'll close this issue but feel free to open a new one if you have any issues.

joshuapoq commented 1 month ago

Hey @saeidbasirnia, I finally tested this out and it does work when disabling Picture in Picture but it's not a great experience.

There's no way to close the modal presentation when PIP is enabled when presenting fullscreen so we must either add our own close button over the PIP button or present within a navigation stack.

This problem is a bit more frustrating when it comes to safeArea. I'm not sure if it's the same for SwiftUI but using UIKit there's no way to present the video fullscreen but have the controls within the safe area. The uiConfig for ignoresSafeArea seems to do nothing.

There is also no close button on the end curtain screen after the video is finished.

saeidbasirnia commented 1 month ago

Hi @joshuapoq, Thanks for reporting this issue, I reported this to our team and we'll look into. I will get back to you soon.

saeidbasirnia commented 2 weeks ago

Hi @joshuapoq, These issues are resolved in release 1.5.1. Please check and let me know if there's more issues.

joshuapoq commented 1 week ago

Works much better now, great job, thanks! ❤️

joshuapoq commented 1 week ago

Would suggest maybe having the UI always abide by the safeArea so you don't have to add magic number padding (as the track at the bottom is now quite far away from the bottom and the header is still slightly too close to the safe area) but instead only use ignoreSystemInsets for the video layer but not the UI.

joshuapoq commented 1 week ago
Apologies @saeidbasirnia whilst the close button is now working on the video and curtain. It's still not showing for every state which could lead to users needing to force close the app (the video is an upcoming show): Issue (upcoming show) Fullscreen (for reference - working / looks good)
BambuserMissingClose Fullscreen
saeidbasirnia commented 1 week ago

Hi @joshuapoq, Thanks for your feedback and screenshots. We have some improvements and bug fixes coming up this week and it should cover these issues.

Beside the upcoming show view, did you face other places with similar issue?

saeidbasirnia commented 1 week ago

Hi @joshuapoq, BambuserPlayerSDK 1.5.3 is now available and includes fixes for these issues.