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

(feat): Support GDPR Compliance #8

Merged
merged 1 commit into from
Jan 18, 2021
Merged

(feat): Support GDPR Compliance #8

merged 1 commit into from
Jan 18, 2021

Conversation

EinfachHans
Copy link

fixes #7

  • Add FACEBOOK_AUTO_LOG_EVENTS Variable
  • Add setAutoLogAppEventsEnabled Method
  • Add explanation in readme

@noahcooper noahcooper changed the base branch from master to develop January 7, 2021 16:08
@noahcooper
Copy link

Thanks for the PR @EinfachHans! Is there a reason to require the developer to update Info.plist themselves, rather than doing this in the plugin?

@EinfachHans
Copy link
Author

I was unsure about how to do that based on the variable as we have to set it as an boolean? 🤔

@EinfachHans
Copy link
Author

@noahcooper other Plugins mostly have scripts for that i think, as <bool>$SOME_VARIABLE</bool> doesn't work? 🤔 It's <false /> or <true /> directly...

@noahcooper
Copy link

Yeah, I think the simplest solution will be to use a hook to do this, since I don't think there's any way to use a variable as a Boolean in plugin.xml in this way. I need to think about that a little more.

In the meantime, would you mind rebasing with the develop branch to address the merge conflict? And could I bother you to clean up the unnecessary whitespace changes?

@EinfachHans
Copy link
Author

@noahcooper yes hook should work here - For the moment i think the user set this by his own is acceptable. 🤔Merge conflict should be resolved, but i have no idea what is causing these white spaces changes... i rebased and explicit checked in my idea that they are not there and now they are here again? Is that a huge problem?

@EinfachHans
Copy link
Author

@noahcooper okay finally managed my idea to remove the whitespaces

@noahcooper
Copy link

This looks good. I'll go ahead and merge this as-is, and if I can't get a hook working to update the plist before cutting the next release, that's OK.

Thanks for the PR!

@noahcooper noahcooper merged commit dbdca06 into cordova-plugin-facebook-connect:develop Jan 18, 2021
@noahcooper
Copy link

@EinfachHans I've updated the iOS after_prepare hook to handle this automatically. (If a developer does happen to update Info.plist in their app's config file, that will take precedence over the plugin.)

Thanks again for the contribution!

@EinfachHans
Copy link
Author

awesome!! 😊

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.

FR: Support GDPR Compliance
2 participants