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

More customization to autoloading #4

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

nobre84
Copy link

@nobre84 nobre84 commented Mar 11, 2014

Hi! Very interesting project you published, thanks very much!
I made some changes that were required for my use case, please check out wether any of it is of interest to the library:

  • I made a public -(void)updateLocalization; in NSObject that can be called manually (we change the language dynamically)
  • Added a OHAutoNIBi18n_AUTOLOAD macro that completely disables awakeFromNib swizzling in case one wants to trigger the localizations manually only.
  • Added a OHAutoNIBi18n_OBSERVE_LOCALE macro that enables the observation of NSCurrentLocaleDidChangeNotification , which can be posted with a userInfo dictionary containing a custom NSBundle in which the translations are obtained instead of the mainBundle.

Unfortunately the observer feature introduces a dependency on a platform 6.0 library, LRNotificationObserver which disposes of the observers in the right way easily. I don't know too much about CocoaPods, I think it can be made optional somehow as a sub-pod. It was important to me, I don't know if it is to you

@nobre84 nobre84 closed this Jul 9, 2014
… dictionary prior to applying localization
@AliSoftware
Copy link
Owner

Hi @nobre84

I was not very reactive on OHAutoNIBi18n lately, sorry about that.

Why did you close this PR? Seems interesting to me, maybe I can reopen it if it can still be merged (even if I will surely refactor it to fix your LRNotificationObserver dependency issue)?

Thanks!

@nobre84
Copy link
Author

nobre84 commented Aug 11, 2014

Hi! I had this PR come from master, and I made more severe change in another aspect of the library not related to the PR's title.
The lib is originally meant to do a one-way translation, and this is the recommended approach to 99% of apps (i.e take a label's text, use it as the key in the localizable table, and put it back in the label's text). In a specific app scenario, I need dynamic language changes, and the Localizable Keys were lost in the first translation, so I now added an associated dictionary to each object, holding the original keys so one could change the localization several times, looking up different bundles each time.

I also use Cocoapods extensively, and the Macros defined in the podspec's pch isn't very convenient (each pod install will overwrite any changes), so I took it off and change it in a Podfile's post_install hook.

Let me know which of my changes are wanted to improve the library, and I can create a clean branch so you could review / merge in another PR.

@AliSoftware
Copy link
Owner

The work you did to support multiple translations seems interesting.
A PR about this could be useful to others, especially since you seem not to be the only one needing this feature.

Anyway, while using associatedObjects is a good idea to keep the original translation key, I would only use it when we are in the dynamic-language mode. In other words, in the 99% case scenario, I would not add the associatedObjects (that would be useless), and only use your mechanism when the user opt-in for this feature.

I'm interesting in your feedback about this, especially provided that OHAutoNIBi18n loads itself automagically using +load, how would you suggest for the developer to configure which mode he wants to use? Maybe a subspec in the podspec to choose one mode or another (avoiding each pod install to override things)?

PS: Can you be more specific about the Macros defined in the podspec's pch? Didn't get the issue you got with that.

@nobre84
Copy link
Author

nobre84 commented Aug 24, 2014

Hi! I agree with you, dynamic language support and the overhead from associatedObjects to support it should be an opt-in feature. My implementation on this still requires some refactoring, it was made on a hurry to release. Do you recall a more efficient way of storing the original keys or running the localization code in spite of associatedObjects and NSNotificationCenter?
About Macros, I wasn't able to define the macros from an app's project. It will always pick up the definitions from the Pods project which are cleared on each pod install, thus prone to error specially on a multi-developer project.
A subspec sounds like a good idea to enable this without having to mess with post install hooks which are very cumbersome. This would mean one subspec for each optional behavior such as debug mode, non-autoload mode, is that right ?

@AliSoftware
Copy link
Owner

I think using associatedObjects to keep the original keys is a good idea. Actually I can't seem to think about any other idea that would work for any UIView (except keeping a huge NSDictionary of UIView* pointers as keys and original key as value, but that would complexify the solution and the dictionary won't be emptied when views get deallocated, so that's not a good solution)

And yes we would need one subspec for each optional behavior.

Another solution (maybe better?) would be to allow the configuration of OHAutoNIBi18n using class methods. Like adding the ability to call [OHAutoNIBi18n setDebugMode:YES], [OHAutoNIBi18n setAutoLoadLocalizations:NO], or [OHAutoNIBi18n setLocale:…] etc in application:didFinishLaunchingWithOptions: or anywhere else. Those methods would then either set a property on an OHAutoNIBi18n singleton instance dedicated to keep track of those properties, or store those values in NSUserDefaults.
This would then mean that the code of OHAutoNIBi18n itself would not rely on macros and #ifdef anymore, but would perform actions depending on these properties tested using some if condition at runtime.

@nobre84
Copy link
Author

nobre84 commented Sep 11, 2014

I like the idea of class methods to configure these aspects of the library. However, I think the subspec approach would stil be required for autoloading, as swizzing can only be done safely in +(void)load . Then, the main podspec would be autoload ON, with an optional subspec to turn it OFF. And the regular features such as debugMode, locale, etc configured at runtime.

@AliSoftware
Copy link
Owner

Yep, fine by me!

Let me know if you need some help with the implied refactoring. Unfortunately I won't have much time lately (especially with all the news around iOS8/iPhone6 ^^) to spend on OHAutoNIBi18n otherwise I would have laid out the global new architecture with subspecs and class methods for configuration (and let you implement the manual loading with associatedObjects).

@AliSoftware AliSoftware mentioned this pull request Nov 12, 2014
@AliSoftware
Copy link
Owner

I'm reopening this PR to avoid forgetting about implementing such a behavior some time (unfortunately not anytime soon due to lot of work)

@AliSoftware AliSoftware reopened this Nov 12, 2014
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.

2 participants