search

jordanbyron / react-native-quick-actions

A react-native interface for Touch 3D home screen quick actions
MIT License
1.06k stars 93 forks source link

fix: 3D Touch will be responsed twice when the app is cold-launched #62

Closed greedbell closed 5 years ago

greedbell commented 6 years ago

like this:

QuickActions.popInitialAction().then(quickActionsHandler);
DeviceEventEmitter.addListener('quickActionShortcut', quickActionsHandler);

quickActionsHandler will be called twice

greedbell commented 5 years ago

@jordanbyron take a look at application:didFinishLaunchingWithOptions:

I think if application:didFinishLaunchingWithOptions: return NO, we should not handle the quick action. so we do not need to handle the quick action from the method application:didFinishLaunchingWithOptions:.

NO if the app cannot handle the URL resource or continue a user activity, otherwise return YES. The return value is ignored if the app is launched as a result of a remote notification.

jordanbyron commented 5 years ago

@greedbell I agree that your proposed workflow of not handling the initial launch action would probably work, however there is a reason Apple has made the distinction between a cold start launch value vs a warm running app receiving and receiving a shortcut action. This choice was intentional and I don't think it's our place to break that behavior. For example, an app may have a deep stack of view layers that a shortcut action bypasses. The cold boot launch option allows the app to deep link directly into the desired screen without first displaying it's normal default view.

I think it makes the most sense if this library matches the same patterns that the underlying API provides and doesn't try to be smart or clever and change the way things are handled. So I think that means we should probably just close this PR and create a ticket to return the correct value (Which I think is NO from the didFinishLaunchingWithOptions method when a shortcut item is present at first launch. That should fix any double calls that we are seeing. What do you think?

greedbell commented 5 years ago

@jordanbyron How about add a new method getQuickActionShortcut to get shortcut action from application:performActionForShortcutItem:completionHandler, and the original method get shortcut action from application:didFinishLaunchingWithOptions:.

to skip quickActionsHandler be called twice, use it as follow 

QuickActions.getQuickActionShortcut().then(quickActionsHandler);
DeviceEventEmitter.addListener('quickActionShortcut', quickActionsHandler);
jordanbyron commented 5 years ago

Isn’t that what popInitialAction already does? On Tue, Oct 9, 2018 at 7:12 AM Bell notifications@github.com wrote:

@jordanbyron https://github.com/jordanbyron How about add a new method getQuickActionShortcut to get shortcut action from application:performActionForShortcutItem:completionHandler, and the original method get shortcut action from application:didFinishLaunchingWithOptions:.

to skip quickActionsHandler be called twice, use it as follow

QuickActions.getQuickActionShortcut().then(quickActionsHandler); DeviceEventEmitter.addListener('quickActionShortcut', quickActionsHandler);

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/jordanbyron/react-native-quick-actions/pull/62#issuecomment-428153058, or mute the thread https://github.com/notifications/unsubscribe-auth/AACe1ggaNYSRbNmECgMH086TkQzIVQ7Dks5ujISigaJpZM4Tp_zV .

-- Jordan

greedbell commented 5 years ago

@jordanbyron popInitialAction can get shortcut action from both application:performActionForShortcutItem:completionHandler: and application:didFinishLaunchingWithOptions:. If that, call 

QuickActions.popInitialAction().then(quickActionsHandler);
DeviceEventEmitter.addListener('quickActionShortcut', quickActionsHandler);

between application:didFinishLaunchingWithOptions: and application:performActionForShortcutItem:completionHandler:, quickActionsHandler will be called twie

jordanbyron commented 5 years ago

@greedbell I spent a little time building a demo app and I don't think there is actually anything wrong with the library. The code is here, but you can see it at the URL below.

https://vimeo.com/294963456

The cold start pop action does not trigger the event listener. Everything works as expected. I'm going to close this PR now but if you can produce a counter example that recreates the double fire on launch feel free to open an issue and I'll review it. Thanks!