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

Feat/Android locale support #233

Conversation

Inbal-Tish
Copy link

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.

Screen Shot 2020-07-27 at 17 00 30

Compatibility

OS Implemented
Android

Checklist

  • [✅ ] I have tested this on a device and a simulator
  • [✅ ] I added the documentation in README.md
  • [✅ ] I mentioned this change in CHANGELOG.md
  • [✅ ] I updated the typed files (TS and Flow)
  • [ ✅] I added a sample use of the API in the example project (example/App.js)

…. Adding support for passing positive and negative dialog buttons labels (good for passing localized labels).
…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
@vonovak vonovak self-assigned this Jul 27, 2020
Copy link
Member

@vonovak vonovak left a 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
Copy link
Member

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?

Copy link
Author

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?

Copy link
Member

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));
Copy link
Member

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))).

Copy link
Author

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?

Copy link
Member

@vonovak vonovak Aug 10, 2020

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

@vonovak
Copy link
Member

vonovak commented Aug 14, 2020

@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
@Inbal-Tish
Copy link
Author

@vonovak Hi. Sorry I didn't have the time to get to it again. Hopefully soon...

@marvin-42
Copy link

Any progress with this?

@pauloaapereira
Copy link

Any progress on this now in 2021? :(
Thanks

@nartenergy
Copy link

Any update on this??

@vonovak
Copy link
Member

vonovak commented Apr 22, 2021

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:

  • take this PR, address comments from code review and open another PR that me or other maintainers will merge
  • support the module via one of the offered sponsorship options and once we collect enough balance, I would take a look at it.
  • (shamlesss plug) you can hire me. I'm sorry if this sounds like I'm trying to sell anything; in fact, I have plenty of work to do elsewhere, I am just listing the options :)

Thank you :)

@vonovak vonovak mentioned this pull request Dec 11, 2021
5 tasks
@vonovak
Copy link
Member

vonovak commented Dec 11, 2021

closed by #540 (v5 release)

@vonovak vonovak closed this Dec 11, 2021
@KMathisGit
Copy link

Should re-open this as the v5 release (removing locale prop) was reverted in v5.1.0.

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.

None yet

8 participants