Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[url_launcher] Update readme for URL schemes on iOS #3252

Merged

Conversation

TahaTesser
Copy link
Member

Description

Add instructions for iOS, for targeting iOS 9.0 and higher.
Note:
All new projects 1.22+ targets iOS 9.0 by default
Screenshot 2020-11-06 at 4 50 28 PM
97959488-6d67bd00-1dea-11eb-9cdf-a06ea55738d5

Related Issues

Related to flutter/flutter#65936

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@@ -2,7 +2,7 @@ name: url_launcher
description: Flutter plugin for launching a URL on Android and iOS. Supports
web, phone, SMS, and email schemes.
homepage: https://github.com/flutter/plugins/tree/master/packages/url_launcher/url_launcher
version: 5.7.10
version: 5.7.10+1
Copy link
Member

@hamdikahloun hamdikahloun Nov 6, 2020

Choose a reason for hiding this comment

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

The next PATCH version must be 5.7.11

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@TahaTesser

For package versions > 1.0.0, we shouldn't do +z .

Details in https://dart.dev/tools/pub/versioning#semantic-versions

Running version check for changed packages
packages/url_launcher/url_launcher/pubspec.yaml incorrectly updated version.
HEAD: 5.7.10+1, master: 5.7.10.
Allowed versions: {6.0.0: NextVersionType.BREAKING_MAJOR, 6.0.0-nullsafety: NextVersionType.MAJOR_NULLSAFETY_PRE_RELEASE, 5.8.0-nullsafety: NextVersionType.MINOR_NULLSAFETY_PRE_RELEASE, 5.8.0: NextVersionType.MINOR, 5.7.11: NextVersionType.PATCH}

Copy link
Member Author

Choose a reason for hiding this comment

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

That's interesting, made the changes required.
Thank you


```
<key>LSApplicationQueriesSchemes</key>
<array>
Copy link
Member

Choose a reason for hiding this comment

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

Does this cover other Schemes like mailto, tel and sms, or we must add new items for this property?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those schemes are supported without adding this so no new items are required

@TahaTesser
Copy link
Member Author

CC: @cyanglaz

@TahaTesser
Copy link
Member Author

@hamdikahloun
Is there anything we need to merge this?
Thank you

@hamdikahloun
Copy link
Member

hi @TahaTesser
Last time i checked it, the branch has conflicts that must be resolved .
Please fix the conflicts before merging .
Thank you

Copy link
Member

@hamdikahloun hamdikahloun left a comment

Choose a reason for hiding this comment

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

LGTM

@TahaTesser TahaTesser force-pushed the url_launcher_readme_update branch 2 times, most recently from a9ab05d to e17673e Compare February 3, 2021 12:09
@TahaTesser
Copy link
Member Author

TahaTesser commented Feb 3, 2021

@hamdikahloun
Just rebased and bumped version, update changelog
I've removed the When targeting iOS 9.0+, new default from Flutter 1.22+. line since this version of stable has been around for months
Can you please review the current updated PR?
Thank you

@hamdikahloun
Copy link
Member

@hamdikahloun
Just rebased and bumped version, update changelog
I've removed the When targeting iOS 9.0+, new default from Flutter 1.22+. line since this version of stable has been around for months
Can you please review that current updated PR?
Thank you

LGTM

@darshankawar
Copy link
Member

LGTM


### iOS

Add the following keys to your _Info.plist_ file, located in `<project root>/ios/Runner/Info.plist`:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not an accurate summary of the requirement, and may still cause confusion. The correct description is that the scheme of any URL they pass to canLaunch should be listed, and the README should say that. You can give the below as an example, but you should make clear that it's an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Made the changes, thanks!

@TahaTesser TahaTesser changed the title [url_launcher] Update readme for iOS targeting iOS 9.0 and higher [url_launcher] Update readme for URL schemes on iOS Feb 11, 2021
## Installation

### iOS
Add scheme for any URL passed on `canLaunch` to your _Info.plist_ file.
Copy link
Member

Choose a reason for hiding this comment

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

May be "Add any URL scheme passed to canLaunch in your Info.plist file."

@TahaTesser TahaTesser force-pushed the url_launcher_readme_update branch from 002d571 to b3ba69b Compare February 16, 2021 10:33
@darshankawar
Copy link
Member

LGTM

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with nits

## Installation

### iOS
Add any URL scheme passed to canLaunch in your Info.plist file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nits:

  • Add `s around canLaunch
  • s/in your/as LSApplicationQueriesSchemes entries in your/

</array>
```

See [canOpenURL](https://developer.apple.com/documentation/uikit/uiapplication/1622952-canopenurl) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the standard way to refer to an OjbC method without context includes the class, and the full signature (which includes the - and the :), like: -[UIApplication canOpenURL:]

See [`-[UIApplication canOpenURL:]`](...) for more details.

@TahaTesser TahaTesser force-pushed the url_launcher_readme_update branch from b3ba69b to 2781fd9 Compare March 2, 2021 13:48
@TahaTesser
Copy link
Member Author

@stuartmorgan
Sorry for the delay, made the changes.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM!

@stuartmorgan-g
Copy link
Contributor

I've restarted the failed test, since it was a network issue.

@stuartmorgan-g stuartmorgan-g added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 23, 2021
@fluttergithubbot fluttergithubbot merged commit 7173380 into flutter:master Mar 23, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 23, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 23, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 24, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 24, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: url_launcher waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants