-
-
Notifications
You must be signed in to change notification settings - Fork 841
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
Support ATT framework for iOS 14 #501
Conversation
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 looks really cool - fantastic @sohail-dragon
@zoontek I have no idea how I would handle release on this one since it will require Xcode 12 which is a not even in stable release yet, but at beta5 it should be soon (Xcode 11 relesaed after beta6 if I recall correctly). Does that make it a breaking change? I don't think so but I don't have the history / context from the Xcode 10 to Xcode 11 transition. I know the general expectation in other modules is "you build with Xcode current" (to say "When Xcode issues an update, you update, and you build with it, old versions are not supported") and there was not a semver major for it. |
@sohail-dragon Thanks! This looks like solid work. Just one though: I know the official API name is What about naming it @mikehardy I don't think it's really a problem since this new permission handler doesn't have to be linked. |
Just a thought - if you diverge from the upstream names, you are creating a new "mental translation layer" that everyone has to remember as they go from native to javascript and back. I'm a fan of just passing the names straight through - no opinions on naming required and no extra mental labor. But this is definitely your show :-), that's just my opinion |
@mikehardy Yeah, you are right. I will try it as soon as possible and ship it in |
Hello, thank you for the PR. |
Yeah - this is definitely open-source-land so putting pressure on anyone is invalid, but this issue (the new ATT framework) is a big deal in one of the main repos I work in (react-native-firebase), so I'm also excited about it 😄 Worst case - everyone that works in react-native projects should be familiar with https://github.com/ds300/patch-package And if someone posted the patch here (you do have to zip it as github won't take .txt extensions) that would help everyone test it out to prove it's working (and use it, prior to release) |
@mikehardy I will release it this weekend. Did you tried it to see if everything run correctly ? |
I have not yet I am sorry @zoontek - I haven't heard any reports (success or failure, zero reports) on react-native-firebase either though it's cross-linked twice
...I was hoping someone would try it, if there was a patch-package patch available (:pray: someone on this thread ?) that would probably overcome the testing barrier Unrelated: I'm going to write a |
I set a reminder for tomorrow. I will try to find 10 minutes to test it. It'a a bit difficult lately, I have a ton of work 😅 |
Yeah, I don't have Xcode12 or iOS14 personally, on any of machines, so I'm a non-starter at the moment. I still have your new boot-splash queued for testing actually and have no excuse there except no time either. c'est la vie :-) |
This is for iOS. Thanks @sohail-dragon , the |
Summary
With iOS 14 launching soon, it'll be useful to support the AppTrackingTransparency framework to display the prompt for the user to allow the app to track them (and make the IDFA available).
FIxes #500
Test Plan
What's required for testing (prerequisites)?
Compatibility
Checklist
README.md
CHANGELOG.md
example/App.js
)Note: Since the framework requires Xcode 12 beta 5 and iOS 14, we should wait for iOS 14 to launch otherwise react-native run-ios will fail for users unless they use xcode 12 to build. Alternatively, this could be a beta feature.