Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[location] remove sticky notification on app killing #11775

Conversation

zaguiini
Copy link
Contributor

Why

Sticky notification was not being removed when the app was killed.

Some folks are having this problem.

How

I'm not so good at Android development so I'll try my best to explain it. Basically I added a listener to the service stop which allowed to remove the sticky notification from the taskbar.

Test Plan

Try starting background location and killing the app from the task manager. The notification should be destroyed. It doesn't happen like that on the latest published version.

Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution 🥇 LGTM 💪
Please add the changelog entry to the expo-notification package.

@wylanosorio
Copy link

Thanks for your contribution 🥇 LGTM 💪
Please add the changelog entry to the expo-notification package.

@lukmccall I think this is expo-location package.

@lukmccall
Copy link
Contributor

Yes, of course 😅 Sorry my bad

@zaguiini
Copy link
Contributor Author

Done @lukmccall

Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT 💯

@lukmccall lukmccall merged commit 9610aee into expo:master Feb 19, 2021
@mccooll
Copy link

mccooll commented Mar 4, 2021

Does this apply to swiping the app closed?

@0x2539
Copy link

0x2539 commented Aug 2, 2021

@zaguiini @lukmccall

This PR looks like it is braking the background location updates while app is closed/killed. My app was working perfectly fine before this PR (Aug 2020), but after updating expo-location (Aug 2021), background location stopped working. The location updates still work perfectly fine while the app is in background, but not when it is closed/killed.

My guess is that the sticky notification was the one that kept the service "alive" even when the app was closed (and by "alive" I mean that location updates keep coming even after onTaskRemoved is called).

I can add a PR pretty quickly if you give me a green light.
Wdyt?

@lukmccall
Copy link
Contributor

@alexbuicescu, you're right. We shouldn't remove the notification. However, I think we should add an option to choose if the notification (and service) will be automatically closed with the app.

@clmoreno
Copy link

I agree with this, the purpose of the stiky notification is keeping the foreground service alive when app is killed so the app can keep receiving location updates, the stinky notification requirement for background location updates was introduced few years ago in Android and it is must if the app requires location updates in background.

@Yandamuri
Copy link

@zaguiini In my case notification is being removed as soon as I killed the app. I want sticky notification. What should I do to achieve sticky notification?

I am using "expo-location": "12.0.4"

@islamouzou
Copy link
Contributor

Is this supposed to be a fix ? this is bad . how would we get location updates when app is killed ?????????

@zaguiini zaguiini deleted the remove-location-foreground-notification-on-app-killing branch December 18, 2021 01:00
lukmccall pushed a commit that referenced this pull request Jan 3, 2022
…15633)

# Why

In order to make background location more felxible we should give a choice to developers whether or not the foreground service should be killed when the app is killed . i personally found this a show stopper . i for example have a cab booking system where i must show the driver that i'm using his location even when he kills his app .

Following this PR #11775 i was able to make this optional .


# How


I've added a boolean option "killServiceOnDestroy" to LocationTaskServiceOptions  which will be passed to the LocationTaskService Intent through extra params , if it's true the foreground service will dissapear on app killing .


# Test Plan


Try starting background location and killing the app from the task manager.  if killServiceOnDestroy is true The notification should be destroyed , otherwise it'll stick .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants