LeoNatan / LNPopupUI

A SwiftUI library for presenting views as popups, much like the Apple Music and Podcasts apps.
MIT License
320 stars 29 forks source link

Remove keyboard observer when object was deallocated #19

Closed aidewoode closed 1 year ago

aidewoode commented 1 year ago

Hi, I have got an issue when I am using LNPopupUI. And I found out that's because LNPopupUICustomPopupBarController add keyboard observers on init. But when LNPopupUICustomPopupBarController was deallocated, those observers didn't remove. So I will get fatal error when I click some input when LNPopupUI didn't appear on view. To resolve this, I add some code to remove those observers when object was deallocated.

LeoNatan commented 1 year ago

This should happen automatically in UIKit. There might be a different issue.

aidewoode commented 1 year ago

I created a simple demo to address this issue, you can check this out https://github.com/aidewoode/LNPopupUI_Demo

Steps to reproduce the issue:

After have clicked the email text field, demo will crash. Follow these error messages:

Fatal error: Attempted to read an unowned reference but the object was already deallocated2022-12-09 09:58:24.172798+0800 LNPopupUI_Demo[54178:2319437] Fatal error: Attempted to read an unowned reference but the object was already deallocated

#7  0x000000010298e0e4 in closure #1 in LNPopupUICustomPopupBarController.init(anyView:) at /Users/ed/Library/Developer/Xcode/DerivedData/LNPopupUI_Demo-frpevnqqbxddeucynhpxdtksxvpb/SourcePackages/checkouts/LNPopupUI/Sources/LNPopupUI/Private/LNPopupUICustomPopupBarController.swift:99
LeoNatan commented 1 year ago

Thanks, will take a look.

LeoNatan commented 1 year ago

Oh, I see now. So block observers behave differently than object observers.

https://developer.apple.com/documentation/foundation/notificationcenter/1411723-addobserver

Return Value

An opaque object to act as the observer. Notification center strongly holds this return value until you remove the observer registration.

LeoNatan commented 1 year ago

Merged. Thank you! I’ll create a release when I get home.

LeoNatan commented 1 year ago

LNPopupUI 1.4.3 is out with your fix. Thanks!