Skip to content

Fix iOS takeSnapshot not working#83

Merged
solid-software merged 4 commits into
solid-software:masterfrom
Antonimo:patch-1
Aug 15, 2020
Merged

Fix iOS takeSnapshot not working#83
solid-software merged 4 commits into
solid-software:masterfrom
Antonimo:patch-1

Conversation

@Antonimo

Copy link
Copy Markdown
Contributor

Fix iOS takeSnapshot not working

@solid-software solid-software left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @Antonimo, thanks for your PR!

I've added couple suggestions - could you please take a look and let us know what you think?

Comment on lines +150 to +151
} else {
print("no arguments")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We usually try to avoid print statements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug Log statement may be better

Comment thread lib/flutter_vlc_player.dart Outdated

Future<Uint8List> takeSnapshot() async {
var result = await _methodChannel.invokeMethod("getSnapshot");
var result = await _methodChannel.invokeMethod("getSnapshot", {'test': ''});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need to pass this 'test' data always or it can be removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was a quick fix, if no arguments then the invoke is ignored

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just change 'test' to 'getSnapShot' then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Antonimo can you make the change?

@Antonimo

Antonimo commented Aug 9, 2020

Copy link
Copy Markdown
Contributor Author

@solid-software @mitchross updated

@mitchross

mitchross commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

@Antonimo did you push up your changes? I dont see it.

NVM see it now. All good. @solid-software LGTM!

@solid-software solid-software merged commit 639c222 into solid-software:master Aug 15, 2020
@solid-software

Copy link
Copy Markdown
Collaborator

Thanks @Antonimo @mitchross!

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.

4 participants