delba / Permission

A unified API to ask for permissions on iOS
MIT License
2.91k stars 323 forks source link

Notifications request callback not called #13

Closed hartbit closed 8 years ago

hartbit commented 8 years ago

It seems like the implementation of notification request callbacks is missing:

// Notifications.swift
38:     func requestNotifications(callback: Callback) {
39:        // TODO: pass callback to finishedRequestingNotifications

On a second node, it seems like the only correct way to implement this behaviour is with a UIApplicationDelegate class, and not with an NSTimer like it's done on line 41.

delba commented 8 years ago

No, the callbacks are called here. That's just a note for a potential refactoring.

On a second node, it seems like the only correct way to implement this behaviour is with a UIApplicationDelegate class, and not with an NSTimer like it's done on line 41.

What do you mean?

hartbit commented 8 years ago

Sorry, I misunderstood where the problem was coming from.

1) After Application.registerUserNotificationSettings is called and the system alert appears on screen, the UIApplicationWillResignActiveNotification notification is dispatched. 2) NSNotificationCenter then calls requestingNotifications. 3) requestingNotifications then invalidates the notificationTimer 4) finishedRequestingNotifications is never called because the timer is invalidated and because UIApplicationDidBecomeActiveNotification is never dispatched (this I don't know why).

Trying to detect when the user has accepted/refused notifications by catching UIApplicationWillResignActiveNotification and UIApplicationDidBecomeActiveNotification seems brittle. It must have broken for it not to work for me. The solution I propose:

1) Implement a UIApplicationDelegate class than implements application(_: didRegisterUserNotificationSettings:). 2) Send a custom notification or call finishedRequestingNotifications from that method. 3) Ask users who want a 100% stable solution that they need their application delegate to subclass Permission's application delegate.

delba commented 8 years ago

UIApplicationDidBecomeActiveNotification should be dispatched after the alert is dismissed and then finishedRequestingNotifications should be called. We are using the same technique for PermissionAlert (see here).

I'm gonna look into it and get back to you soon.

Subclassing UIApplicationDelegate is a bit too much to ask. At the very most, swizzling application(_: didRegisterUserNotificationSettings:) but I'd prefer to stay away from that and to find a better solution.

hartbit commented 8 years ago

Is it never working for you or just very inconsistently (which is by no means less problematic) ?

Never.

Do you have the same issue with other permission types ?

Haven't tried other but could give it a try.

Are you testing on a real device or in the simulator ?

Real device with debugger attached.

delba commented 8 years ago

Which version / commit are you using please?

hartbit commented 8 years ago

Cocoapods version 1.2

delba commented 8 years ago

I identified the bug. I'm working on a fix.

delba commented 8 years ago

Fixed in https://github.com/delba/Permission/commit/b743cdc77b38a25b664c0ae1e59102ffba796c85.

The Permission instance was released from memory and thus finishedRequestingNotifications was never called.

hartbit commented 8 years ago

Great!