Skip to content

Conversation

@amirh
Copy link
Contributor

@amirh amirh commented Jul 19, 2018

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

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.
@amirh
Copy link
Contributor Author

amirh commented Jul 19, 2018

@HansMuller would love to get your feedback on this

Copy link
Member

@xster xster left a 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.
Copy link
Member

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)

Copy link
Contributor Author

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.
Copy link
Member

Choose a reason for hiding this comment

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

assert this?

Copy link
Contributor Author

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> {

Copy link
Member

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

Copy link
Contributor Author

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 {
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k

Copy link
Contributor

@HansMuller HansMuller left a 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.
Copy link
Contributor

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"

Copy link
Contributor Author

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

view => view's

Copy link
Contributor Author

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).
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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;
Copy link
Contributor

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.

Copy link
Contributor Author

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;
Copy link
Contributor

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?

Copy link
Contributor Author

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.
Copy link
Contributor

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.

Copy link
Contributor Author

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),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as offset & size

Copy link
Member

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@amirh amirh merged commit debd501 into flutter:master Jul 20, 2018
@amirh amirh deleted the render_platform_view branch July 20, 2018 22:59
@lcodecorex
Copy link

How should I use AndroidView to embed a WebView?

@amirh
Copy link
Contributor Author

amirh commented Aug 6, 2018

AndroidView is not yet ready to be used.
You can follow the referenced issue for progress.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants