-
Notifications
You must be signed in to change notification settings - Fork 3.4k
review and update the android plugin migration doc #3388
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
Conversation
| deprecated, but we encourage you to migrate to the new APIs. | ||
| Over time, plugins using the old APIs might produce strange | ||
| behaviors when embedding Flutter into an Android app. | ||
| The new APIs has the advantage of providing a cleaner set of accessors for |
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.
"The new API has 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.
ok
| Over time, plugins using the old APIs might produce strange | ||
| behaviors when embedding Flutter into an Android app. | ||
| The new APIs has the advantage of providing a cleaner set of accessors for | ||
| lifecycle dependent components compared to the old APIs. For instance |
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.
s/APIs/API
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
| 6. Create an `EmbeddingV1Activity.java` file that uses the v1 | ||
| embedding in the same folder as `MainActivity`. For example: | ||
| 1. (Optional) Create an `EmbeddingV1Activity.java` file that uses the v1 | ||
| embedding in the same folder as `MainActivity` in the example project to keep |
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.
Awkward. "Create an EmbeddingV1Activity.java file that uses the v1 embedding for the example project in the same folder as MainActivity to keep..."
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
| ``` | ||
|
|
||
| 7. Add the `EmbeddingV1Activity` to the | ||
| 7. (Optional) Add the `EmbeddingV1Activity` to 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.
For these optional steps should we mention when they would want to be done?
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.
added a few more words
e775b6a to
a75c327
Compare
|
@xster, it looks like you forked this when the build was broken. Would you mind grabbing the latest, merging, and submitting again? The build is fixed now. Thz |
a75c327 to
83cc141
Compare
|
CI passes now |
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 for me
|
Waiting for @mklim, @gaaclarke, and/or @matthew-carroll to review. |
83cc141 to
ded9813
Compare
mklim
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.
Sorry for the slow review pass, I saw a couple things here that I think we may want to update again.
| ```yaml | ||
| environment: | ||
| sdk: ">=2.0.0-dev.28.0 <3.0.0" | ||
| flutter: ">=1.9.1+hotfix.4 <2.0.0" |
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 think this should be the new stable now, 1.12. 1.9 is missing some fixes that will matter in a few edge cases so I think it's better to just recommend stable here.
| ``` | ||
|
|
||
| 4. Manually register the E2E plugin in `MainActivity.java` | ||
| 1. Manually register the E2E plugin in `MainActivity.java` |
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.
Is this still needed in v1.12?
Fixes flutter/flutter#44224