Skip to content
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

[platform_channels] adds Platform Image demo #475

Merged
merged 4 commits into from
Jun 19, 2020
Merged

[platform_channels] adds Platform Image demo #475

merged 4 commits into from
Jun 19, 2020

Conversation

AyushBherwani1998
Copy link
Member

Description

Demonstrates how to use BasicMessageChannel to send the message from the dart side and receive a reply from the platform. The demo also demonstrates how to receive
an image from the platform.

Screenshots

Copy link
Contributor

@RedBrogdon RedBrogdon left a comment

Choose a reason for hiding this comment

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

Looks like a great start. I left a few comments, but nothing big.

I do have an important question, though: What is the source of the image in this PR? We need to make sure we're completely covered on copyright. If you're not particularly wedded to this exact image, I can email you some options that I know we won't have problems with.

Comment on lines 47 to 48
// Registers a MessageHandler for BasicMessageChannel to receive a message from dart side and send
// the image data in reply.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

    // Registers a MessageHandler for BasicMessageChannel to receive
    // a message from Dart and send image data in reply.

platform_channels/lib/src/image_basic_message_channel.dart Outdated Show resolved Hide resolved
platform_channels/lib/src/platform_image_demo.dart Outdated Show resolved Hide resolved
platform_channels/lib/src/platform_image_demo.dart Outdated Show resolved Hide resolved
/// a platform image. The [BasicMessageChannel] is using [StandardMessageCodec]
/// since it supports [Uint8List] which can be easily used to get the image data
/// from platform and display an image using [Image.memory].
class PlatformImage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming this class to something slightly more descriptive ("PlatformImageFetcher" for example).

@AyushBherwani1998
Copy link
Member Author

I do have an important question, though: What is the source of the image in this PR?

The image is taken from AnimationSamples. Here

@RedBrogdon
Copy link
Contributor

I do have an important question, though: What is the source of the image in this PR?

The image is taken from AnimationSamples. Here

That'll work. 😄

Copy link
Contributor

@RedBrogdon RedBrogdon left a comment

Choose a reason for hiding this comment

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

LGTM. I left one small additional note about taking out an unnecessary widget. Happy to land this afterwards.

appBar: AppBar(
title: Text('Platform Image Demo'),
),
body: Builder(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Now that you're not using a snackbar, you don't need this builder any more.

@RedBrogdon RedBrogdon merged commit ecb5aab into flutter:master Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants