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

[av] fix record permission selector #28236

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Apr 16, 2024

Why

alternative solution for #28006 and hopefully to fix #26730

How

fix the permission requests like what we did in expo-location

Test Plan

NCL + recording to test the permission requester

Checklist


Copy link

linear bot commented Apr 16, 2024

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Apr 16, 2024
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Apr 16, 2024
@brentvatne brentvatne changed the title [av] obfuscate record permission selector [av] fix record permission selector Apr 16, 2024
@Kudo Kudo marked this pull request as ready for review April 17, 2024 05:15
@Kudo Kudo requested a review from lukmccall as a code owner April 17, 2024 05:15
@Kudo Kudo merged commit d51371f into main Apr 18, 2024
20 checks passed
@Kudo Kudo deleted the @kudo/eng-11965-optional-permissions-eg-for-expo-av branch April 18, 2024 19:50
Kudo added a commit that referenced this pull request Apr 18, 2024
alternative solution for #28006 and hopefully to fix #26730

fix the permission requests like what we did in expo-location

---------

Co-authored-by: Hein Rutjes <hrutjes@gmail.com>
(cherry picked from commit d51371f)
@Kudo Kudo added the published Changes from the PR have been published to npm label Apr 18, 2024
@hueter
Copy link
Contributor

hueter commented Apr 18, 2024

@Kudo - this fixed the issue, thanks! The app store just accepted my build that was previously rejected for expo-av (no microphone usage).

@Kudo
Copy link
Contributor Author

Kudo commented Apr 19, 2024

super excited to hear that the solution works. huge thanks to @hueter for confirming ❤️

@paulschreiber
Copy link
Contributor

What version is this fix included in? And why does the expo-av NPM page list 13.10.6 instead of 14.0.2?

@Kudo
Copy link
Contributor Author

Kudo commented Apr 24, 2024

@paulschreiber it's published as expo-av@13.10.6. you could use npx expo install --check to check the dependency versions.
14.0.2 (dist-tag next) is prepared for sdk 51.

@IjzerenHein
Copy link
Contributor

I'm curious to see whether these changes bypass the Apple detection mechanism. Has anyone tried to submit an app with this already?

@Kudo Sorry I didn't reply sooner, just bought a house, been busy :)

@Kudo
Copy link
Contributor Author

Kudo commented May 7, 2024

@IjzerenHein yes, thanks for @hueter to confirm that at #28236 (comment)

(sorry my turn for not response timely from sdk 51 release 😅)

@IjzerenHein
Copy link
Contributor

@IjzerenHein yes, thanks for @hueter to confirm that at #28236 (comment)

(sorry my turn for not response timely from sdk 51 release 😅)

Okay completely missed that 🫣

👍 👍 👍 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint compatible bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expo-av requires microphone permission even when not using microphone
8 participants