cordova-plugin-camera
cordova-plugin-camera copied to clipboard
Localizing hardcoded messages
Platforms affected
iOS
What does this PR do?
Localizes messages hardcoded
What testing has been done on this change?
Works on Cordova based application
Checklist
- [ ] Reported an issue in the JIRA database
- [ ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [ ] Added automated test coverage as appropriate for this change.
Please merge this
We use https://github.com/fairmanager-cordova/plugin-localization-strings for this. It applies pretty much universally to all native localizable strings. I like a centralized solution for this problem. Might be worth a look.
We use https://github.com/fairmanager-cordova/plugin-localization-strings for this. It applies pretty much universally to all native localizable strings. I like a centralized solution for this problem. Might be worth a look.
I gather the values in InfoPlist.strings cannot be accessed in code, correct me if I am wrong
@kohlia I'm not sure I understand the question correctly. The plugin will create localization projects from the JSON configuration. The remaining process is pretty much transparent.
If you need a localized string from ObjC, this is how to do it: https://github.com/fairmanager-cordova/plugin-barcodescanner/commit/73d0f497b242627b33dcd6fbfb32d0d14501b100
@oliversalzburg makes sense so now I need to update the code to fetch from InfoPlist.strings but this will add dependency on this https://github.com/fairmanager-cordova/plugin-localization-strings plugin
@kohlia I'm sorry, I didn't mean to suggest to depend on that plugin. I wanted to merely point out an existing solution in this area. This patch looks like it tries to achieve similar things and this approach might even conflict with existing solutions.
Ideally, all solutions play nice together. Sadly, I can't fully evaluate the situation at the moment.
So what now?
Now you can decide if this has any impact on your PR and change something or not. And then some maintainer will maybe review your PR and decide if it can and should be merged or not.
Now you can decide if this has any impact on your PR and change something or not. And then some maintainer will maybe review your PR and decide if it can and should be merged or not.
This still does not impact the PR as dependency on another plugin should not be created. I feel this solution works fine
Nobody told you to include the other plugin, @oliversalzburg just mentioned it so we all can consider if these changes might make this plugin incompatible to that other plugin that solves a similar problem for existing users.
Its up to the maintainers now.
Please merge the change
Hey, I just fixed the problem that caused Android tests to fail in master
. Could you rebase this PR please? This should get rid of the Android failures and possibly fix all test failures for this PR.
Closing and re-opening to trigger a new CI/test run with new PR merge.
Strings are already localizable as they are NSLocalizedString
, I don't see any improvement on changing them to NSLocalizedStringFromTable
That is done to specify language file, so as to keep if separate from overlapping any application language files
I'd prefer if the literals were prefixed (as they already are in this patch) and just be left in the main table. Creating multiple tables might be the cleaner approach, but it's also a new solution in the entire environment and might require more thought in general.
Hello there :wave: Any chance to see the conflict resolved? We need localization for this message but it doesn't seem possible today.