Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[Image_Picker] Fix example #1876

Merged
cyanglaz merged 7 commits into from
Jul 29, 2019
Merged

[Image_Picker] Fix example #1876

cyanglaz merged 7 commits into from
Jul 29, 2019

Conversation

@juliocbcotta
Copy link
Contributor

@juliocbcotta juliocbcotta commented Jul 19, 2019

Description

This PR updates the image picker plugin dependency for video_player and updates the video player logic to make it work (the current implementation fails to show the video).

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@juliocbcotta juliocbcotta requested a review from cyanglaz as a code owner July 19, 2019 03:06
@juliocbcotta juliocbcotta changed the title fix image picker plugin example [Image_Picker] Fix example Jul 19, 2019
cyanglaz
cyanglaz reviewed Jul 19, 2019
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Playing video seems failing on iOS with this patch. Could be a bug in the plugin itself too.

packages/image_picker/example/lib/main.dart Outdated

void _onVideoControllerUpdate() {
setState(() {});
// setState(() {});
Copy link
Contributor

@cyanglaz cyanglaz Jul 19, 2019

Choose a reason for hiding this comment

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

nits: if not necessary anymore, we can just remove the code instead of commenting out.

Copy link
Contributor Author

@juliocbcotta juliocbcotta Jul 20, 2019

Choose a reason for hiding this comment

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

done.

@cyanglaz cyanglaz self-assigned this Jul 19, 2019
@juliocbcotta
Copy link
Contributor Author

juliocbcotta commented Jul 20, 2019

Playing video seems failing on iOS with this patch. Could be a bug in the plugin itself too.

I don't have access to an iPhone, but I did test in the simulator. I was not able to see the video, but I could listen to it.
I ended doing some more cleaning in my last commit, if you could take a look.

@cyanglaz
Copy link
Contributor

cyanglaz commented Jul 23, 2019

It seems like the video is still not playing on iOS with this patch. I am adding this PR to backlog. Maybe you can create an issue and refer this PR to it. And we will prioritize the PR based on the thumb-ups of the issue?

@cyanglaz cyanglaz removed their assignment Jul 23, 2019
@cyanglaz
Copy link
Contributor

cyanglaz commented Jul 23, 2019

Actually I have found the issue. The video player dependency wasn't updated in the example app. I updated it in the same PR and going to merge it.
Thanks again for the contribution!

@cyanglaz cyanglaz added submit queue The Flutter team is in the process of landing this PR. and removed backlog labels Jul 23, 2019
@cyanglaz cyanglaz self-assigned this Jul 23, 2019
@juliocbcotta
Copy link
Contributor Author

juliocbcotta commented Jul 23, 2019

@cyanglaz It looks like there is some issue with cirrus. Could you help me with that?

@juliocbcotta
Copy link
Contributor Author

juliocbcotta commented Jul 29, 2019

@cyanglaz it is green.

@cyanglaz cyanglaz merged commit c92b040 into flutter:master Jul 29, 2019
mithun-mondal pushed a commit to bKash-developer/archived_plugins that referenced this pull request Aug 6, 2019
@juliocbcotta @mithun-mondal
[Image_Picker] Fix example (flutter#1876)
6f4f413
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@cyanglaz cyanglaz cyanglaz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cyanglaz cyanglaz

Labels

cla: yes submit queue The Flutter team is in the process of landing this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@juliocbcotta @cyanglaz @googlebot