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
Updates add-flutter-screen.md to the latest embedding API. #3239
Updates add-flutter-screen.md to the latest embedding API. #3239
Conversation
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 minor tweaks, but LGTM otherwise.
Replace `YourFlutterActivityName` with the name of your Flutter `Activity`. | ||
The reference to `@style/LaunchTheme` can be replaced by any Android theme that | ||
you'd like to apply to your `FlutterActivity`. The choice of theme dictates the | ||
colors of system chrome and the background color of the `FlutterActivity` just |
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.
"colors of system chrome" feels like an odd phrase and I'm not sure I follow what you mean.
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 tried another approach. Let me know if it's still unclear.
.initialRoute("/my_route") | ||
.build(currentActivity) | ||
); | ||
} | ||
}); | ||
``` | ||
|
||
Replace `"mySpecialEntrypoint"` with the name of your desired Dart entrypoint, |
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.
Also, replace that ending comma with a period. ;)
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 line was deleted. Is there an issue with the new line that was added?
Replace `"/my_route"` with your desired initial route. | ||
|
||
The use of the `withNewEngine()` factory method configures a `FlutterActivity` | ||
that will internally create its own `FlutterEngine` instance. This comes 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.
Avoid "will". So "that will internally create" => "that internally creates"
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.
Done
`FlutterEngine` before arriving at your `FlutterActivity` and then you can use | ||
your pre-warmed `FlutterEngine` instead. | ||
|
||
To pre-warm a `FlutterEngine`, find a reasonable location in your app to | ||
instantiate and hold a `FlutterEngine`. For a simplistic app, storing a | ||
instantiate a `FlutterEngine`. For a simplistic app, pre-warming a |
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'm not seeing the whole file in this PR, but is it clear to the dev what constitutes a "simplistic" app? Also, would "simple" do instead? Trying to make it easier for the non-native English speaker.
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 tried a slightly different description. Let me know if it's still too ambiguous.
9ff7239
into
flutter:staging-add-to-app-do-not-delete-until-2020
`FlutterEngine` before arriving at your `FlutterActivity` and then you can use | ||
your pre-warmed `FlutterEngine` instead. | ||
|
||
To pre-warm a `FlutterEngine`, find a reasonable location in your app to | ||
instantiate and hold a `FlutterEngine`. For a simplistic app, storing a | ||
instantiate a `FlutterEngine`. For a small app or a prototype, pre-warming a |
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 makes it sound like it would be inappropriate to do so in a larger app where it might still be sensible to do if the flutter screen is the second screen and has a high probability of opening
} | ||
} | ||
}); | ||
``` |
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.
Reminder that we still need to create the kotlin version at some point
are beyond the scope of this guide. TODO(mattcarroll): link to | ||
resource. | ||
When re-using a cached engine across multiple `FlutterActivity` instances, you | ||
assume responsibility for showing the desired content for the given `Activity`. |
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.
Not sure I understood this section. Perhaps give an example of such a responsibility
When using the `withCachedEngine()` factory method, pass the same ID that you | ||
used when caching the desired `FlutterEngine`. | ||
|
||
Now, when you launch `FlutterActivity`, there is significantly less delay in |
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.
Not specifically for this line but we should also make sure to not make it sound like performance is the only reason. e.g. other reasons could be to run dart code or connect plugins early, or to keep the engine alive after the activity etc
Updates add-flutter-screen.md to the latest embedding API.