Skip to content

Conversation

TheWirv
Copy link

@TheWirv TheWirv commented Oct 14, 2021

Summary

Added workaround for App Tracking Transperency on iOS 15.0 to the documentation.

Motivation: ... as discussed in this comment

Test Plan

No testing needed, only docs have changed.

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

@zoontek
Copy link
Owner

zoontek commented Oct 14, 2021

You have been to fast! Check my comment: #648 (comment)

I suggest avoiding the wording "workaround" here since it's the correct behavior: Your app must be active to request ATT. A workaround is more of a temporary solution until the issue is properly fixed.

@mikehardy
Copy link

:-) too fast is a good problem to have, but kudos for taking my not-that-gentle nudge for a PR and doing it :-) 🍾

@TheWirv
Copy link
Author

TheWirv commented Oct 14, 2021

@mikehardy Sure, I mean this is how Open Source works, and I can't stand people who are just complaining, without the thought of actually helping out ever occuring to them.

@zoontek Do you think it would make sense to say something in the vein of, "ATT permissions should be requested when the App becomes active because requesting it on mount can lead to problems," or do you think the explanation is not necessary?

@zoontek
Copy link
Owner

zoontek commented Oct 14, 2021

@TheWirv Instead of "on mount can lead to problems", you can say that "if the app is not active, the request prompt will not appear". Problems is a bit too vague

@zoontek
Copy link
Owner

zoontek commented Oct 16, 2021

I merge it and will add a few changes since I want it to be in the README for the next release (npm one is not synced with GitHub README). Thanks a lot 🙂

@zoontek zoontek merged commit b1e6cd9 into zoontek:master Oct 16, 2021
@TheWirv
Copy link
Author

TheWirv commented Oct 20, 2021

Hey @zoontek, sorry, I spaced out there for a moment. I 100% agree with your points, but thanks for merging anyway. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants