-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[image_picker] Clarify maxDuration parameter behavior #10473
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
base: main
Are you sure you want to change the base?
[image_picker] Clarify maxDuration parameter behavior #10473
Conversation
Updates documentation to explicitly state that the maxDuration parameter only applies when recording videos from the camera (ImageSource.camera), and is ignored when selecting videos from the gallery (ImageSource.gallery). This addresses confusion reported in issue #83630 where developers expected maxDuration to filter gallery video selections, but it only controls camera recording duration. Changes: - Updated README.md with detailed explanation and code examples - Enhanced dartdoc comments for pickVideo() method - Added important notes about gallery selection behavior Fixes flutter/flutter#83630
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.
Code Review
This pull request provides a valuable clarification to the documentation for the maxDuration parameter in pickVideo. By explicitly stating that it only applies to camera recordings and is ignored for gallery selections, it addresses a point of common developer confusion. The changes in both the README.md and the Dartdoc comments are clear and well-written. My review includes suggestions to extend this clarification to the pickMultiVideo method as well, to ensure consistency across the package's API, as it exhibits the same behavior.
Extends the documentation clarification to pickMultiVideo() method to maintain consistency across the API. This method only selects from the gallery, so maxDuration is always ignored. Addresses feedback from code review.
Addresses remaining code review feedback by adding pickMultiVideo() to the Video Duration Limitations section in README. The maxDuration parameter is ignored for pickMultiVideo() as well, since it only selects from the gallery. Also added example code showing pickMultiVideo() usage with maxDuration.
stuartmorgan-g
left a comment
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.
Thanks for the contribution!
You appear to have deleted our PR template and replaced it with a version of the flutter/flutter template; please don't do that. We have different templates in the different repositories for a reason.
I've restored this repository's template; please mark the PR as ready for review again once the checklist is completed.
| /// **Important:** The [maxDuration] parameter is ignored when selecting videos from the gallery | ||
| /// ([ImageSource.gallery]). Users can select videos of any duration from the gallery, regardless | ||
| /// of the [maxDuration] value. If you need to enforce duration limits on gallery-selected videos, | ||
| /// you must validate the video duration programmatically after selection. |
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.
This doesn't need to be an whole extra paragraph, it should just be noted in the paragraph that documents the parameter. E.g., "The [maxDuration] argument specifies the maximum duration of the captured video when recording from the camera ([ImageSource.camera]), and is ignored for [ImageSource.gallery]."
| final List<XFile> medias = await picker.pickMultipleMedia(); | ||
| ``` | ||
|
|
||
| ### Video Duration Limitations |
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.
We generally don't attempt to cover every possible use case of an API in the README; I don't think this section adds any substantive information beyond what the updated doc comments says, so doesn't justify the additional README length.
Description
Updates documentation to explicitly state that the
maxDurationparameter only applies when recording videos from the camera (ImageSource.camera), and is ignored when selecting videos from the gallery (ImageSource.gallery).This addresses confusion reported in issue #83630 where developers expected
maxDurationto filter gallery video selections, but it only controls camera recording duration.Changes
README.mdwith detailed explanation and code examples showing the difference between camera recording and gallery selectionpickVideo()method with important notes about gallery behaviormaxDurationis ignored for gallery selectionsRelated Issues
Fixes flutter/flutter#83630
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.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.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