packages
packages copied to clipboard
[video_player] #60048 ios picture in picture
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue.
This feature is only available on iOS because android has a different implementation. simple_pip_mode can be used for Android
Fixes https://github.com/flutter/flutter/issues/60048
Migration from flutter/plugin to flutter/packages is done with this pr
old pr: https://github.com/flutter/plugins/pull/6284
No migration needed this is an addon
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
- [X] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [X] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [X] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [X] I signed the CLA.
- [X] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [X] I listed at least one issue that this PR fixes in the description above.
- [X] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [X] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style. - [X] I updated/added relevant documentation (doc comments with
///). - [X] I added new tests to check the change I am making, or this PR is test-exempt.
- [X] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
@hellohuanlin & @stuartmorgan I did the migration from https://github.com/flutter/plugins/pull/6284 to here
@tarrinneal can I assist you with the review?
@tarrinneal I updated my pull request fully retested it to make sure it does what I expect.
@tarrinneal I updated my pull request fully retested it to make sure it does what I expect.
Seems like you still have a few broken tests. I should be able to get this re-reviewed this week, if you can git them sorted out.
@tarrinneal the tests are failing on camera because of the dependency overrides. How should I handle this? Because the dependency override script doesn't update it in the camera package.
I will fix the merge conflicts today
@tarrinneal the tests are failing on camera because of the dependency overrides. How should I handle this? Because the dependency override script doesn't update it in the camera package.
I will fix the merge conflicts today
That's right. Sorry, I got this pr mixed up with another one. I'll take a look!
Thanks for your extended review! I will go through it this weekend! And process all your feedback
@stuartmorgan can you give me another review/feedback
Android changes look good to me!
Any other issues I should fix? Not sure with android changes you are referring to.
Android changes look good to me!
Any other issues I should fix? Not sure with android changes you are referring to.
Not on my part. Just the changes to the example look fine
Hey @vanlooverenkoen are you still working on this pr? I think it's about ready to be merged.
Yes I am, i will be back from holiday tomorrow, i will do a final cleanup
@tarrinneal fixed the pr
I think this is ready to be broken up into the federated pr's. Here's a link on how to do that. https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins
@stuartmorgan can you do another review?
@vanlooverenkoen Hi, are you still planning on updating this?
Hi, yes of course. I changed jobs and forgot to add my GitHub email to my phone and PC. Give me some time so I can fix this, but yes I will still fix all the comments and discussions
Status check from triage: It looks like there were some commits, but a lot of CI is still failing. Is this intended to be reviewed again now, or still waiting for more fixes?
I thought I had fixed the latest comments, but again there are merge conflicts. Quite hard to keep track of what still should be fixed.
I thought I had fixed the latest comments, but again there are merge conflicts. Quite hard to keep track of what still should be fixed.
Merge conflicts aren't an issue during review; I was referring to the many CI failures in the last run. A few of those may have been unrelated flake, but most in my spot-checks were actual failures in the PR, which need to be resolved.
I will double check when all ci/checks are passed/failed again. I indeed saw some linux, windows & macos ci checks fail. But because I did not touch any of these integrations I thought it was something that is broken on main.
I indeed saw some linux, windows & macos ci checks fail.
The first word in the name of a CI check is the OS of the host machine running the tests; every check will be one of those three.
But because I did not touch any of these integrations I thought it was something that is broken on main.
Breakage on main is relatively rare; unless you've checked the hash you've merged to and it was red in our postsubmit, it's best to assume it's any failures are caused by the PR unless you've looked at the details and found that they can't be related.
Some ci's fail because it seems that the error are related to camera which depends on video_player but video_player are currently stil using overrides? How do I fix that?
For this combo PR you can just add dependency overrides to the camera example so that it uses the platform interface from path. (It won't be a problem in the final landed PR because by then the platform interface will be published.)
@vanlooverenkoen Are you still planning on updating this PR?
I missed your comment. I will check this PR again this weekend.
From triage: @vanlooverenkoen Are you still planning on completing this PR?
@stuartmorgan yes working on it right now
I have taken the week of from work to work on side projects, And this PR is one of them :)
@stuartmorgan any idea why this is happening?
test/test_api.g.dart:368:20: Error: Method not found: 'wrapResponse'.
return wrapResponse(error: e);