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

Add configuration to delay the first start of Push until Push.setEnabled(true) is called from JS #480

Merged
merged 9 commits into from
Dec 11, 2018

Conversation

guperrot
Copy link
Member

@guperrot guperrot commented Dec 8, 2018

Things to consider before you submit the PR:

  • Has CHANGELOG.md been updated?
  • Are tests passing locally?
  • Are the files formatted correctly?
  • Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?

Description

To delay start push with this change:

  • Add EnablePushInJavascript YES to AppCenter-Config.plist
  • Add "enable_push_in_javascript": true to appcenter-config.json
  • Call Push.setEnabled(true) from Javascript when developer wants to start Push.

Since this is only for the first time we want to delay Push, the code also remembers the first call to Push.setEnabled(true) and will start Push automatically the next time.

Documentation PR: https://github.com/MicrosoftDocs/appcenter-docs-pr/pull/1000

Requested in #287

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@dhei dhei left a comment

Choose a reason for hiding this comment

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

I am thinking maybe use this API in TestApp?

thyeggman
thyeggman previously approved these changes Dec 10, 2018
@dhei dhei merged commit e81de60 into develop Dec 11, 2018
@dhei dhei deleted the feature/delay-start-push branch December 11, 2018 22:00
@jdnichollsc
Copy link

jdnichollsc commented Jan 15, 2019

Hi guys,
EnablePushInJavascript is not working for iOS, the event onPushNotificationReceived is not executed in that platform

@guperrot
Copy link
Member Author

@jdnichollsc it works for us in TestApp from this repo but it looks like your issue is caused by something else since you commented on #505.

@jdnichollsc
Copy link

jdnichollsc commented Jan 15, 2019

@guperrot thanks, I'm reviewing the code of AppCenter but I can't see any issue

@guperrot
Copy link
Member Author

We will investigate and track in #505 even if not related (can still rename the title there when we find the cause).

Also you mentioned in #505 that version 1.8.0 of the SDK also has the issue so that means it's not caused by this new configuration feature and is a older issue.

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.

5 participants