search

pushandplay / cordova-plugin-apprate

This plugin provide the "rate this app" functionality into your Cordova/Phonegap application
Apache License 2.0
300 stars 190 forks source link

fix customLocale initial value #293

Closed rei1011 closed 3 years ago

rei1011 commented 3 years ago

I think it would be better to modify the initial value of preferences.customLocale in AppRate.js to match the formatting with Index.d.ts.

At this time, setting customLocale in AppRate.setPreferences will likely result in customLocale = null.

@westonganger can you please review?

westonganger commented 3 years ago

customLocale should be null if its not explicitly set. Please clarify.

rei1011 commented 3 years ago

@westonganger Thank you for confirming this.

At the moment, the initial value of preferences.customLocale is not of type object, so when setPreferences(pref[key], prefObj[key]) is executed recursively on line 229, the conditional expression on line 227 is false so preferences.customLocale will not be set.

Please check it out.

westonganger commented 3 years ago

Please fix the code at line 229 then.

rei1011 commented 3 years ago

@westonganger Thank you for confirming this.

The conditional expression in setPreferences has been changed. Please check it out.

westonganger commented 3 years ago

Merged. Thanks for your contribution!

EinfachHans commented 3 years ago

I'm not sure if i understand that change. If the expression on line 227 is false, how can modifying line 229 help? Maybe i'm wrong, but with that change the customLocale object isn't updated recursively anymore, correct? So let's say i first call it with some values and then call it a second time it will completely override the values i have set in customLocale first? @westonganger @rei1011