-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Introduce an AndroidView widget and a RenderAndroidView render object. #19565
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
RenderAndroidView is responsible for sizing and displaying an embedded Android view. AndroidView is responsible for creating and disposing the Android view and is using RenderAndroidView to display it.
|
@HansMuller would love to get your feedback on this |
xster
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
|
|
||
| /// Embeds an Android view in the Widget hierarchy. | ||
| /// | ||
| /// Embedding Android views is an expensive operation and should be avoided when possible. |
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.
May need a bit more elaboration like what's the proposed alternative or why this exists (i.e. what are reasonable use cases)
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.
updated
| class AndroidView extends StatefulWidget { | ||
| /// Creates a widget that embeds an Android view. | ||
| /// | ||
| /// The `viewType` parameter must not be null. |
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.
assert 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.
done
| } | ||
|
|
||
| class _AndroidViewState extends State<AndroidView> { | ||
|
|
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.
uber nit: ha looks like google3 java style? I don't see whitespace at class start/end elsewhere in our codebase for consistency
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
| import 'object.dart'; | ||
|
|
||
|
|
||
| enum _PlatformViewState { |
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.
Please don't use SCREAMING_CAPS
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.
k
HansMuller
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.
| /// [RenderAndroidView] is responsible for sizing and displaying the Android view. | ||
| /// | ||
| /// See also: | ||
| /// * [AndroidView] which is a widget that is typically used to show an Android view. |
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 you can omit "typically"
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
|
|
||
| /// A render object for an Android view. | ||
| /// | ||
| /// [RenderAndroidView] is responsible for sizing and displaying the Android view. |
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 Android view => an Android View
Maybe View should be a link to https://developer.android.com/reference/android/view/View
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
| /// } | ||
| /// ``` | ||
| /// | ||
| /// The Android view lifetime is the same as the lifetime of the [State] object for this 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.
view => view's
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
| /// | ||
| /// The Android view lifetime is the same as the lifetime of the [State] object for this widget. | ||
| /// When the [State] is disposed the platform view (and auxiliary resources) are lazily | ||
| /// released (some resources are immediately released and some by platform garbage collector). |
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 would be a good place to provide an State disposal example or two. You could say it general terms, like: A stateful widget's state is disposed the the widget is removed from the tree or when it is moved within the tree. If the stateful widget has a key and it's only moved relative to its siblings, or it has a [GlobalKey] and it's moved within the tree, it will not be disposed.
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.
thanks, copy pasted 😄
|
|
||
| /// Embeds an Android view in the Widget hierarchy. | ||
| /// | ||
| /// Embedding Android views is an expensive operation and should be avoided when a Flutter |
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.
Should we explain that an embedded android view behaves just like a normal Flutter widget with respect to painting and input. And what about layout? I imagine that we need to explain that the parent has to constrain the size of its AndroidView.
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:
/// The embedded Android view is painted just like any other Flutter widget and transformations
/// apply to it as well.Anything else to add here?
I'll leave the comment about input to the PR where I add input support.
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.
Looks good
| /// A [PlatformViewFactory](/javadoc/io/flutter/plugin/platform/PlatformViewFactory.html) | ||
| /// for this type must have been registered. | ||
| /// | ||
| /// See also: [AndroidView] for an example of registering a platform view factory. |
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.
Are there any predefined view types?
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.
there aren't... we're going to add them in plugins.
| } | ||
|
|
||
| class _AndroidViewState extends State<AndroidView> { | ||
| int id; |
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.
_createNewAndroidView is private, maybe these state vars should be private as well.
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
|
|
||
| @override | ||
| void performResize() { | ||
| size = constraints.biggest; |
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.
What happens if the constraints aren't bounded?
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.
Red screen of death 😨
That's the same behavior we have for Texture.
Not sure what better to do here, I added a note about this to the class comments for the render object and the widget.
| await _viewController.setSize(size); | ||
| // We've resized the platform view to targetSize, but it is possible that | ||
| // while we were resizing the render object's size was changed again. | ||
| // In that case we will re-iterate to resize the platform view again. |
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 don't think re-iterate is a word, probably just say something like: In that case, set the view controller's size again.
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
| return; | ||
|
|
||
| context.addLayer(new TextureLayer( | ||
| rect: new Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height), |
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.
Same as offset & size
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 needs a dart linter :)
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
|
How should I use AndroidView to embed a WebView? |
|
AndroidView is not yet ready to be used. |
RenderAndroidView is responsible for sizing and displaying an embedded
Android view.
AndroidView is responsible for creating and disposing the Android view
and is using RenderAndroidView to display it.
#19030