[image_picker] Remove MethodCall from most Android code #3438
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stuartmorgan-g
requested review from
camsim99,
gmackall and
tarrinneal
as code owners
stuartmorgan-g
force-pushed
the
image-picker-android-pigeon-prep
branch
from
02706ab to
889e0bd
Compare
stuartmorgan-g
commented
Mar 10, 2023
Collaborator
Author
There was a problem hiding this comment.
The previous code treated quality at a nullable value until the very bottom layer where it was used, at which point null and 100 were actually handled the same, so I moved that logic into this layer to simplify things.
| @Test | ||
| public void ImageCache_ShouldBeAbleToSetAndGetQuality() { | ||
| when(mockMethodCall.argument(MAP_KEY_IMAGE_QUALITY)).thenReturn(IMAGE_QUALITY); | ||
| public void imageCache_shouldBeAbleToSetAndGetQuality() { |
Collaborator
Author
There was a problem hiding this comment.
Since I was touching most of the tests anyway, I fixed their names to follow Google Java style.
tarrinneal
approved these changes
Mar 10, 2023
Contributor
tarrinneal
left a comment
tarrinneal
left a comment
There was a problem hiding this comment.
This is going to be a solid improvement in design for sure!
tarrinneal
added
the
autosubmit
jackyshan
added a commit
to jackyshan/packages
that referenced
this pull request
Mar 11, 2023
* origin/main: [image_picker] Remove MethodCall from most Android code (flutter#3438) add missing_enum_constant_in_switch (flutter#3435) [go_router_builder] Generate the enum map for enum used in iterable (flutter#3415) [pigeon] finish kotlin and swift safe cast removal (flutter#3360) [tool] Include examples when pathifying deps (flutter#3393) url change for platform interface (flutter#3323) # Conflicts: # packages/pigeon/CHANGELOG.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Mar 13, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Mar 13, 2023
10 tasks
nploi
pushed a commit
to nploi/packages
that referenced
this pull request
Jul 16, 2023
[image_picker] Remove MethodCall from most Android code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current code passes
MethodCallobjects across multiple layers of code, and retains them to store state during the intent process, which makes adopting Pigeon very difficult. This refactors the code to extract all relevantMethodCallinformation at the initial call site, and passes/stores specific data objects instead. This improves type safety at other layers, and lays the foundation for a later PR that switches to Pigeon.The new data classes added here will likely be replaced by Pigeon-generated classes during that conversion, but doing this refactoring as a separate step should make reviewing each part much easier, so adding a couple of trivial temporary classes seemed like a reasonable tradeoff.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).