Skip to content

Multilingual version #144

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

malyfko
Copy link

@malyfko malyfko commented Jun 8, 2017

Based on another PR (#129), which was merged to version 2.3.0 but was removed in version 2.4.0, support for locale language was added. Also comparing to previous PR, this one translates not only names for days of week, but also months and time units. All user has to do is to pass 'locale' parameter when initializing MaterialDateTimePicker. Also unlike previous PR, this one uses the same version of moment.js

@joews
Copy link
Member

joews commented Jun 12, 2017

Thanks for the contribution! I hope to take a look later this week.

@@ -67,20 +72,23 @@ class DateTimePicker extends Events {
// style options
initializeRome(container, validator) {
const onData = this.onChangeDate.bind(this);

const weekDays = moment.localeData(this.options.locale)._weekdaysMin;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there!

In JS there is a convention that indicates variables starting with underscore are private (Futher reading here or here) They should not be read or written directly. It's just a convention but that's why I decided to update moment to 2.13.0.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Luisetelo this solution was used due to the version of moment.js this project uses.

@lucasbento
Copy link

This looks good, any ideia when we can get it merged in, @joews?

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.

4 participants