-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add deep linking page #5244
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 deep linking page #5244
Conversation
| | iOS (not launched) | App gets initialRoute ("/") and a short time after gets a pushRoute | App gets initialRoute ("/") and a short time after uses the RouteInformationParser to parse the route and call RouterDelegate.setNewRoutePath, which configures the Navigator with the corresponding Page. | | ||
| | Android - (not launched) | App gets initialRoute containing the full route ("http:/deeplink/") | App gets initialRoute ("/deeplink") and passes it to the RouteInformationParser to parse the route and call RouterDelegate.setNewRoutePath, which configures the Navigator with the corresponding Pages. | | ||
| | iOS (launched) | pushRoute is called | Path is parsed, and the Navigator is configured with a new set of Pages. | | ||
| | Android (launched) | pushRoute is called | Path is parsed, and the Navigator is configured with a new set of Pages. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This table currently doesn't look very good, maybe there's a better way to format this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. This table doesn't look good. See dart.dev for a pretty table, and see its [implementation](## Keywords) (search for ## Keywords).
sfshaza2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits. When did you want this to be merged? As soon as it's ready? Later?
| description: Navigate to routes when the app receives a new URL | ||
| --- | ||
|
|
||
| Flutter now supports deep linking on iOS and Android. Any link matching the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"now"? As of when? I'd either specify a release or remove "now".
| --- | ||
|
|
||
| Flutter now supports deep linking on iOS and Android. Any link matching the | ||
| configured scheme will be handled by the Navigator. By following these steps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this page reflect original navigator or Navigator 2? I think we should be specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to refer to Navigator as the Navigator and Navigator 2 as the Router widget for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make this code fonted since it's referring to a specific class.
|
|
||
| Flutter now supports deep linking on iOS and Android. Any link matching the | ||
| configured scheme will be handled by the Navigator. By following these steps, | ||
| your app will be able to launch and display routes via named routes (either with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your app will be able to launch => Your app can launch and...
| configured scheme will be handled by the Navigator. By following these steps, | ||
| your app will be able to launch and display routes via named routes (either with | ||
| the [`routes`][routes] parameter or [`onGenerateRoute`][onGenerateRoute]), or by | ||
| using the [Router][Router] widget. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's redundant to put the identical text in both square brackets. In this case, the second pair of square brackets should be empty.
|
|
||
| A full restart is required to apply these changes. | ||
|
|
||
| ## Test on Android Emulator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android Emulator => Android emulator
| </array> | ||
| ``` | ||
|
|
||
| The CFBundleURLName is a unique URL used to distinguish your app from others |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the class names should be code fonted.
|
|
||
| A full restart is required to apply these changes. | ||
|
|
||
| ## Test on iOS Simulator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS Simulator => iOS simulator
| ## Migrating from plugin-based deep linking | ||
|
|
||
| If you have written a plugin to handle deep links, as described in [this | ||
| article][plugin-linking], it will continue to work until you opt-in to this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, it is Very Bad Form to link on "this". You've done that twice in this page. Use the name of the article or at least some descriptive text in the link text.
| | iOS (not launched) | App gets initialRoute ("/") and a short time after gets a pushRoute | App gets initialRoute ("/") and a short time after uses the RouteInformationParser to parse the route and call RouterDelegate.setNewRoutePath, which configures the Navigator with the corresponding Page. | | ||
| | Android - (not launched) | App gets initialRoute containing the full route ("http:/deeplink/") | App gets initialRoute ("/deeplink") and passes it to the RouteInformationParser to parse the route and call RouterDelegate.setNewRoutePath, which configures the Navigator with the corresponding Pages. | | ||
| | iOS (launched) | pushRoute is called | Path is parsed, and the Navigator is configured with a new set of Pages. | | ||
| | Android (launched) | pushRoute is called | Path is parsed, and the Navigator is configured with a new set of Pages. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. This table doesn't look good. See dart.dev for a pretty table, and see its [implementation](## Keywords) (search for ## Keywords).
|
Since this is currently on the dev channel, could we merge it with note at the top of the page? (waiting is okay, too) |
|
Whaaaaaaa?! But Flutter already has a Router class, right? This is
confusing.
…On Tue, Feb 2, 2021 at 4:47 PM John Ryan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/docs/development/ui/navigation/deep-linking.md
<#5244 (comment)>:
> @@ -0,0 +1,116 @@
+---
+title: Deep linking
+description: Navigate to routes when the app receives a new URL
+---
+
+Flutter now supports deep linking on iOS and Android. Any link matching the
+configured scheme will be handled by the Navigator. By following these steps,
We're going to refer to Navigator as the Navigator and Navigator 2 as the
Router widget for now.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#5244 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKS4PKOPR5JOQBX2DBDXKSTS5CMLPANCNFSM4W5W4T5A>
.
|
|
Yes, I am definitely ok with adding a note about this being on the dev channel. On flutter.dev/docs, we say: The documentation on this site reflects the latest stable release of Flutter. So it's well and good to add a warning and publish anyway. |
- Use code font for class names - Avoid "this" in link names - Use title case - Improve table formatting - Add note that this is only available on the dev or master channels
sfshaza2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're getting there. :)
| --- | ||
|
|
||
| {{site.alert.note}} | ||
| This feature is only available on the dev or master channel. See [Switching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor nit, but we indent notes by 2 spaces at the beginning of each line, but not the site.alert.tag tag itself.
| Flutter channels][switching-channels] for more information. | ||
| {{site.alert.end}} | ||
|
|
||
| Flutter supports deep linking on iOS and Android in the dev channel. Any link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It supports it on web, too. And desktop? If I read that first sentence, I might stop reading there if I'm not programming specifically for mobile. I'd like to minimize changes that we might have to. make later, so please address web and desktop, if possible, now. The first paragraph should indicate whether they need to keep reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add web to this sentence. The second paragraph discusses web behavior.
| {{site.alert.end}} | ||
|
|
||
| Flutter supports deep linking on iOS and Android in the dev channel. Any link | ||
| matching the configured scheme will be handled by the `Navigator`. By following |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Any link matching the configured scheme will be handled by the Navigator."
That sentence is fairly confusing, and it's in passive tense. Do you mean:
"The Navigator handles deep linking." ?? But this doesn't really seem right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to replace this sentence with a softer explanation " Users opening a URL will display a specific screen in your app."
| A full restart is required to apply these changes. | ||
|
|
||
| ## Test on Android emulator | ||
| To test with an Android emulator, the `adb` command can be given an intent. The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"can be given an intent" => To test..., give the adb command an intent where the host name matches the name defined in AndroidManifest.xml. For example:
| A full restart is required to apply these changes. | ||
|
|
||
| ## Test on iOS simulator | ||
| The `xcrun` command can be used to test on the iOS Simulator: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, very passive wording.
=> "Use the xcrun command to test on the ..."
| matching the configured scheme will be handled by the `Navigator`. By following | ||
| these steps, your can launch and display routes via named routes (either with | ||
| the [`routes`][routes] parameter or [`onGenerateRoute`][onGenerateRoute]), or by | ||
| using the [Router][] widget. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we've publicly called the new mechanism Navigator 2, we need to be clear what we are talking about here. This needs another note. (I can't remember exactly what you said about the naming, but we need to be clear and I haven't seen this said anywhere else.) Something along the lines of.
{{site.alert.secondary}}
**Version note:** You might be familiar with [Navigator 2.0][].
When referring to the updated navigator mechanism, we now
call it..."
{{site.alert.end}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will try to get an answer on this.
|
This is almost ready for another look - I'll spend some time figuring out what the team would like to call Navigator 2 going forward. |
|
Ready for another look. |
sfshaza2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, if you accept the suggestions.
| [switching-channels]: /docs/development/tools/sdk/upgrading#switching-flutter-channels | ||
| [routes]: https://api.flutter.dev/flutter/material/MaterialApp/routes.html | ||
| [onGenerateRoute]: https://api.flutter.dev/flutter/material/MaterialApp/onGenerateRoute.html | ||
| [Router]: https://api.flutter.dev/flutter/widgets/Router-class.html | ||
| [Navigator 2.0]: https://medium.com/flutter/learning-flutters-new-navigation-and-routing-system-7c9068155ade | ||
| [intent filter]: https://developer.android.com/guide/components/intents-filters | ||
| [plugin-linking]: https://medium.com/flutter-community/deep-links-and-flutter-applications-how-to-handle-them-properly-8c9865af9283 | ||
| [verify-android-links]: https://developer.android.com/training/app-links/verify-site-associations | ||
| [router-sample]: https://github.com/flutter/samples/blob/master/navigation_and_routing/lib/nav_2/router.dart No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| [switching-channels]: /docs/development/tools/sdk/upgrading#switching-flutter-channels | |
| [routes]: https://api.flutter.dev/flutter/material/MaterialApp/routes.html | |
| [onGenerateRoute]: https://api.flutter.dev/flutter/material/MaterialApp/onGenerateRoute.html | |
| [Router]: https://api.flutter.dev/flutter/widgets/Router-class.html | |
| [Navigator 2.0]: https://medium.com/flutter/learning-flutters-new-navigation-and-routing-system-7c9068155ade | |
| [intent filter]: https://developer.android.com/guide/components/intents-filters | |
| [plugin-linking]: https://medium.com/flutter-community/deep-links-and-flutter-applications-how-to-handle-them-properly-8c9865af9283 | |
| [verify-android-links]: https://developer.android.com/training/app-links/verify-site-associations | |
| [router-sample]: https://github.com/flutter/samples/blob/master/navigation_and_routing/lib/nav_2/router.dart | |
| [switching-channels]: /docs/development/tools/sdk/upgrading#switching-flutter-channels | |
| [routes]: {{site.api}}/flutter/material/MaterialApp/routes.html | |
| [onGenerateRoute]: {{site.api}}/flutter/material/MaterialApp/onGenerateRoute.html | |
| [Router]: {{site.api}}/flutter/widgets/Router-class.html | |
| [Navigator 2.0]: {{site.flutter-medium}}/flutter/learning-flutters-new-navigation-and-routing-system-7c9068155ade | |
| [intent filter]: https://developer.android.com/guide/components/intents-filters | |
| [plugin-linking]: {{site.medium}}/flutter-community/deep-links-and-flutter-applications-how-to-handle-them-properly-8c9865af9283 | |
| [verify-android-links]: https://developer.android.com/training/app-links/verify-site-associations | |
| [router-sample]: {{site.github}}/flutter/samples/blob/master/navigation_and_routing/lib/nav_2/router.dart |
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
|
Whoops, this is LGTM'd already. Merging... |
* add deep linking article, move navigation.md to navigation/index.md * Change table title * add deep linking page to sidenav * fix sidenav bug * specify deep linking is supported in the dev channel * "be able to launch" -> "can launch" * simplify reference-style links * Address review comments - Use code font for class names - Avoid "this" in link names - Use title case - Improve table formatting - Add note that this is only available on the dev or master channels * indent note * add web browsers to supported platforms in first paragraph * simplify introduction * active voice * active voice * add Navigator 2.0 warning - renamed to Router. * Update src/docs/development/ui/navigation/deep-linking.md Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * Apply suggestions from code review Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]> * remove redundant sentence * end sentence correctly * keep 80 column limit * 80 column limit * 80 column limit Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
This adds instructions for deep linking. There's a few remaining tasks:
Deployed: https://flutter-website-jr-stagi-988d4.firebaseapp.com/docs/development/ui/navigation/deep-linking