search

umbraco-community / umbraco-analytics

Google Analytics for the Umbraco CMS
41 stars 31 forks source link

Use LocalizationService instead of ui.Text #39

Open bjarnef opened 9 years ago

bjarnef commented 9 years ago

ui.Text is deprecated, so we should use LocalizationService.

https://twitter.com/Shazwazza/status/613986522631667712

warrenbuckley commented 9 years ago

Thanks for highlighting this @bjarnef looks like I need to get onto this at some point then. Will it be backwards compatible do we know?

bjarnef commented 9 years ago

@warrenbuckley I am not sure how it will be backwards compatible - if it doesn't handle that, I think we could check for the Umbraco version?

I guess Shannon knows :+1:

Shazwazza commented 9 years ago

You can still use ui.Text... it is backward compatible, just deprecated. It actually calls into LocalizationService.Text

... It would be impossible to make something new backward compatible btw unless i had a time machine :)

bjarnef commented 9 years ago

Okay, I was just wondering if LocalizationService.Text had fallback for earlier version of Umbraco 7, which just used ui.Text.

Shazwazza commented 9 years ago

No, if you use new APIs that don't exist in previous versions it would never work. We cannot create 'fallback' APIs for ones that don't exist already :P

bjarnef commented 9 years ago

So should we stick to ui.Text or check for the Umbraco version if switching to the new API?

Shazwazza commented 9 years ago

You can either keep using ui.Text for now for your packages, or you can do some compat checks in your packages or you can release different versions of your packages for different umbraco versions, or you can make your package dependent on certain versions of Umbraco (i.e. Articulate will make you use the latest Umb version normally... that is the way that I am supporting that project). In v8, ui.Text will not exist.

warrenbuckley commented 9 years ago

@bjarnef let me know on what approach you want to adopt.

As obviously in current version we do a lot of work of merging of keys into lang files on app startup, but should I do a version check if newer than 7.3, use the lovely auto lang key merge in 7.3 thats now available & continue to use ui.Text??

Or do we make latest version breaking changes & dependant on latest version of Umbraco?

bjarnef commented 9 years ago

@warrenbuckley I am thinking maybe do a check if newer than 7.3 .. and then maybe at the same time do a check if it should use the new LocalizationService.Text, if lower than 7.3 use ui.Text?

Then maybe later for a major version of Analytics only support Umbraco 7.3+ ... or what do you think?

warrenbuckley commented 9 years ago

Not sure to be honest & little busy at the moment to do anything about it. So if you have an idea & want to implement it in a PR then go for it.

But until I can find/make some time for this I won't get a chance to look at it currently, so feel free to decide.