[camera] Remove deprecated feature from Windows example #10380
Conversation
The Windows camera example was showing usage of `maxVideoDuration`, but the parameter is deprecated and ignored, so shouldn't be part of the example.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
| return cameras.map((String cameraName) { | ||
| return CameraDescription( | ||
| // This type is only nullable due to Pigeon limitations, see | ||
| // https://github.com/flutter/flutter/issues/97848. The native code |
There was a problem hiding this comment.
This was fixed a while ago, I just missed updating this when I updated the Pigeon file.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the usage of the deprecated maxVideoDuration parameter from the Windows camera example. The changes include removing the timed recording logic, the associated state variable _recordingTimed, and the corresponding UI elements. This simplifies the example code significantly. Additionally, a minor cleanup in camera_windows.dart removes unnecessary null-safety checks for camera names, aligning with updated Pigeon-generated code. The changes are correct and improve the example's clarity.
| } | ||
|
|
||
| Future<void> _toggleRecord() async { | ||
| if (_initialized && _cameraId > 0) { |
There was a problem hiding this comment.
The check _cameraId > 0 seems incorrect. Camera IDs are non-negative integers, so camera ID 0 is valid. This check prevents video recording on the first available camera. It should probably be _cameraId >= 0.
| if (_initialized && _cameraId > 0) { | |
| if (_initialized && _cameraId >= 0) { |
1 participant
The Windows camera example was showing usage of
maxVideoDuration, but the parameter is deprecated and ignored, so shouldn't be part of the example.Also makes a minor improvement to type safety of some code interacting with Pigeon, since the Pigeon definition had already been changed to non-nullable.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3