Skip to content
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

[ios][camera] Fix legacy camera getAvailablePictureSizes #27642

Merged
merged 2 commits into from Mar 13, 2024

Conversation

alanjhughes
Copy link
Collaborator

@alanjhughes alanjhughes commented Mar 13, 2024

Why

Fixes an issue reported by @kbrandwijk, where the pictureSize was not being set

How

The problem was getAvailablePictureSizes was returning pictureSizesDict.keys which does not return an array of String but an array of Key structs. This would not get correctly parsed so would result in an empty array being sent back to JS.

Test Plan

Bare-expo. The array is now correctly sent back and using the pictureSize prop works correctly.

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Mar 13, 2024
@expo-bot
Copy link
Collaborator

expo-bot commented Mar 13, 2024

The Pull Request introduced fingerprint changes against the base commit: 4844826

Fingerprint diff
[
  {
    "type": "dir",
    "filePath": "../../packages/expo-camera/ios",
    "reasons": [
      "expoAutolinkingIos"
    ],
    "hash": "9f3f091e46c30e63842b035fb47205315f903062"
  }
]

Generated by PR labeler 🤖

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Mar 13, 2024
@alanjhughes alanjhughes marked this pull request as ready for review March 13, 2024 21:45
@@ -201,7 +201,9 @@ public final class CameraViewModule: Module {

AsyncFunction("getAvailablePictureSizes") { (_: String?, _: Int) in
// Argument types must be compatible with Android which receives the ratio and view tag.
return pictureSizesDict.keys
return pictureSizesDict.map { k, _ in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why we need more tests. It seems pretty easy to detect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely. Not sure how that got through for so long

@alanjhughes alanjhughes merged commit cae81b6 into main Mar 13, 2024
17 of 18 checks passed
@alanjhughes alanjhughes deleted the @alanhughes/camera/fix-available-picture-sizes branch March 13, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants