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

Added support for locale on DatePicker dialog component #680

Merged
merged 2 commits into from Aug 7, 2016

Conversation

aabilio
Copy link
Contributor

@aabilio aabilio commented Aug 1, 2016

Updated components, docs and spec.

@aabilio
Copy link
Contributor Author

aabilio commented Aug 2, 2016

This is not a PR to address #623, just a simple way to provide strings to DatePicker to show months and days in other languages (with some predefined languages loaded).

@javivelasco javivelasco merged commit 157c29d into react-toolbox:dev Aug 7, 2016
@javivelasco
Copy link
Member

Beautiful work! I think this is very useful and does not create any breaking changes so happy to merge! Thanks 💃 !

@sanfilippopablo
Copy link
Contributor

sanfilippopablo commented Aug 8, 2016

This is great. When is this going to be on NPM?
Edit: Nevermind. I see it's on the latest release (1.1.1).

@aabilio
Copy link
Contributor Author

aabilio commented Aug 30, 2016

I just realize that action buttons ('Cancel' and 'OK') need to be translated too. I was thinking on inserting it in locale prop too (pro: easy api for Date Picker component) but I think that inserting new properties like okButtonLabel, cancelButtonLabel, dismissButtonLabel... could make sense too (pro: standarization of action buttons). What do you think @javivelasco?

@javivelasco
Copy link
Member

I think they should come from props as you mentioned. In fact, all internationalization in a library such as react-toolbox should be based on that. Anyway there are not a lot of components with text so it should be easy.

@sanfilippopablo
Copy link
Contributor

sanfilippopablo commented Aug 30, 2016

While the props option sounds more technically correct, I think that the labels being attributes on the locale object is more useful, because the default locale with the string locale prop is simple option that covers 95% of the use cases. Besides, it would still allow customization through the object locale prop.

@javivelasco
Copy link
Member

javivelasco commented Aug 30, 2016

Yeah maybe. It also would be a cleaner api. More opinions on this would be great

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

3 participants