[MOD-2203] Facebook: Module should inject tiapp.xml requirements for user
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Medium |
Status | Open |
Resolution | Unresolved |
Affected Version/s | Release 5.2.0 |
Fix Version/s | n/a |
Components | |
Labels | n/a |
Reporter | Fokke Zandbergen |
Assignee | Unknown |
Created | 2016-03-02T10:17:19.000+0000 |
Updated | 2018-03-06T18:52:39.000+0000 |
Description
To use the Facebook module the user has to add a whole set of properties to
tiapp.xml
:
http://docs.appcelerator.com/platform/latest/#!/api/Modules.Facebook
However, except for maybe those that depend on the linked Facebook app (ID, name) all of this should be injected by the module itself using the timodule.xml
.
{quote} Right now, the timodule.xml is only used in Android, and it supports all of the tags listed in the Android section above. Any custom metadata your Android module defines in it's timodule.xml
So for Android we could still do this right?
Yes, but there are no android-specific manifest settings required by ti.facebook as far as I know.
We do at http://docs.appcelerator.com/platform/latest/#!/api/Modules.Facebook :
But those are user-specific values that we cannot pre-fill. :-) Tell me that exactly can be added and I'll add it. Simple as that.
They are not user-specific. The only user-specific thing is the value of
@string/facebook_app_id
which you set viastrings.xml
. See the linked doc.PR: https://github.com/appcelerator-modules/ti.facebook/pull/44 We need to ensure the values get merged with the values set in the
tiapp.xml
correctly.Well, there's one way to find out ;)
As stated on Flowdock, the
timodule.xml
andtiapp.xml
are not merged and nor will they ever be. Only in the Android builds will thetimodule.xml
's<application><android><manifest>
be scrubbed and merged with the finalAndroidManifest.xml
. I'm not sold that setting the@string/facebook_app_id
viastrings.xml
is a good idea. First it's Android only. Second we should have all settings in one place, namely thetiapp.xml
. Since this is a module, it must not modify the Titanium app. The module's documentation should be clear as to what settings the user must add to thetiapp.xml
.[~cbarber] we actually do the same with modules like [ti.map](https://github.com/appcelerator-modules/ti.map/blob/master/android/timodule.xml) and it's much more user friendly (and less likely to give errors) then letting the user add those. I agree it shouldn't use string though. Just like the linked ti.map it should refer to a tiapp property.
Plus.. iOS modules should be able to do the same: TIMOB-20542
Removing from review, since it's currently not technically possible to inject it properly.