-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[camera_web] Don't require enumerating cameras on web #6369
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
Conversation
At the time of this writing, a version of the example app where the enumeration of
can be tried out here. Note that, contrary to the normal example app, it doesn't ask for permission before you tap on a camera. |
45f54ec
to
42480cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please.
We're creating an application that uses the camera and we're baffled that we need to show a permission dialog to get the list of camera devices. We also noticed that devices with many cameras take a longer time to initialize and display a preview.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the changes in this file because the example had an existing issue, or is the web change breaking to clients?
@kristoffer-zliide This appears to be failing web tests in CI ( @ditman It looks like this is otherwise waiting for your review. |
From triage: @kristoffer-zliide Are you planning on fixing the failing tests in this PR? |
Marking as a draft pending resolution of the test failures. |
42480cf
to
0e9c0fc
Compare
6e06d43
to
6fbdca5
Compare
6fbdca5
to
aafe0f2
Compare
@stuartmorgan: The tests are passing now. |
Please note that the PR won't be able to land without these steps, which is why they are in the checklist, and not doing it before review will add another round to the review process since things like the CHANGELOG wording are part of the review. |
This will need to land until after the |
@ditman Has that transition happened? If not, is there a tracking issue we can link to? |
@stuartmorgan actually, yes, camera_web is in package:web since this PR: |
After merging
(@kristoffer-zliide I can move the changes to a new branch, if you'd prefer) |
@kristoffer-zliide Are you still planning on updating this PR based on the review feedback? |
I think I'm done here. Sorry if I wasted your time. Feel free to rebase/cherry-pick the four commits onto your own branches. |
Enumerating available cameras on web requires permission and activates hardware. This PR allows initializing the camera controller without having to enumerate available cameras first.
flutter/flutter#145541
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.md
to add a description of the change, [following repository CHANGELOG style].///
).If you need help, consider asking for advice on the #hackers-new channel on [Discord].