-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
Feat/Android locale support #233
Feat/Android locale support #233
Conversation
…. Adding support for passing positive and negative dialog buttons labels (good for passing localized labels).
…cker into feat/Android_locale_support
…nd/datetimepicker into feat/Android_locale_support # Conflicts: # android/src/main/java/com/reactcommunity/rndatetimepicker/RNConstants.java # android/src/main/java/com/reactcommunity/rndatetimepicker/RNDatePickerDialogFragment.java # android/src/main/java/com/reactcommunity/rndatetimepicker/RNDatePickerDialogModule.java # package-lock.json # src/datetimepicker.android.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello and thanks for working on this. I have some questions I'd like to discuss to see how to best proceed about this. Thank you!
@@ -0,0 +1,6 @@ | |||
#Wed Jul 22 14:26:15 IDT 2020 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you belive these gradle files should be included in the repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are probably auto-generated when running the example project. Do you think they are not necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they are not necessary
@@ -86,6 +87,10 @@ DatePickerDialog getDialog( | |||
display | |||
); | |||
default: | |||
if (args != null && args.containsKey(RNConstants.ARG_LOCALE)) { | |||
// for header (NOTE: will change the app locale) | |||
Locale.setDefault(getLocale(args)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite possible there is no other way to localize the pickers, but this seems like an action that can have far-reaching consequences.
on ios the prop just "overrides the system default with a specific locale." - that is a "safe way" of specifying locale.
Here, the call would have side effects beyond the datepicker, and I'm not sure we want to take that path. Seem to me that code like this should probably be called from userland upon app start. What do you think?
(also applies to activityContext.getResources().getConfiguration().setLocale(getLocale(args))
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know and I do understand your point. I was looking for a way to localize it without localizing the entire app but couldn't find another way. I was also thinking that passing the 'locale' prop to localize the picker is made for an already localized app when the user what to have the picker in the app's locale, not the device's. Thus no real harm is done when we change the locale this way. We can add a note to these side-effects when using this feature. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to not do it this way. I'm not even sure setting locale is a job for this module, maybe it should be done in the scope of some different package..
edit: since this seems like an often-requested feature, I'm in support of supporting locale change.
I would prefer for this locale-changing logic to be exposed via a @ReactMethod
that users can call. That will express the side effect more clearly. Such way is more clear for the consumers of the library
@Inbal-Tish would you please address the review? I'd like to merge the PR once the comments are addressed. Thank you! |
removing auto-generated android files
Removing auto-generated Android file gradle-wrapper
Removing auto-generated Android files gradlew
@vonovak Hi. Sorry I didn't have the time to get to it again. Hopefully soon... |
Any progress with this? |
Any progress on this now in 2021? :( |
Any update on this?? |
hello to everyone commenting; as I explained in #313, we're looking for backers that would support the development of this module and due to reasons explained in the post, I no longer actively develop this module (and seems like other maintainers don't have much time either). In order to move this forward, I invite you to do any of the following:
Thank you :) |
closed by #540 (v5 release) |
Should re-open this as the v5 release (removing locale prop) was reverted in v5.1.0. |
Summary
Currently, DateTimePickers on Android use system locale and not the app one, so a user with English system language and Spanish app language would see a picker in English instead of Spanish. See issue #82
I added support for passing 'locale' prop, the same way as in iOS, and get the picker in that locale (date picker will localize the dialog header and calendar's month and days of the week views).
As part of this effort, I added the 'positiveButtonLabel' and 'negativeButtonLabel' props to help pass localized strings to the buttons labels.
Uncomment App.js lines #214-216 to pass the appropriate props to see the example.
Compatibility
Checklist
README.md
CHANGELOG.md
example/App.js
)