Skip to content

Add iOS only setLaunchURLsInApp function #542

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

Merged
merged 1 commit into from
Mar 10, 2022
Merged

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Mar 10, 2022

Description

One Line Summary

Adds the setLaunchURLsInApp function that is for iOS only.

Details

The function, previously only available via the native iOS SDK, does the following:

This method can be used to set if launch URLs should be opened in Safari or within the application. The default value is true which opens an in-app browser. Setting this to false will open in Safari.

For Android, just log that it is unsupported and don't add to plugin.

  • Add setLaunchURLsInApp method to OneSignal iOS plugin
  • Add setLaunchURLsInApp bridge method to onesignal_flutter.dart
  • Add setLaunchURLsInApp method to the example app with false

Test

  • iOS: Tested on iPhone 13 using the example app calling setLaunchURLsInApp with true and false. Tested with OneSignal-iOS-SDK 3.10.1 that remedied the problem with calling after init.
  • Android: Tested on emulator that no error occurred and logged the statement "setLaunchURLsInApp: this function is not supported on Android".
  • Did not add unit testing as this method was not added to the Android plugin and the mock channel was not being called as a result, leading to failing unit test.

This change is Reviewable

@nan-li nan-li requested review from a team, emawby, jkasten2 and Jeasmine and removed request for a team March 10, 2022 19:43
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

One nit on a log level, feel free to fix and merge without another approval from my end.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @emawby, @Jeasmine, and @nan-li)


lib/onesignal_flutter.dart, line 176 at r1 (raw file):

        "OneSignal#setLaunchURLsInApp", {'isEnabled': isEnabled});
    } else {
      _onesignalLog(OSLogLevel.verbose,

nit, I would say this should be info instead of verbose.

@nan-li nan-li force-pushed the add_setLaunchURLsInApp branch from 9f9a0e1 to 841c204 Compare March 10, 2022 21:09
@nan-li nan-li merged commit e1344d4 into main Mar 10, 2022
@nan-li nan-li deleted the add_setLaunchURLsInApp branch March 10, 2022 21:10
nan-li added a commit that referenced this pull request Mar 10, 2022
@nan-li nan-li mentioned this pull request Mar 10, 2022
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.

2 participants