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(datepicker): use the angular locale API #2066

Closed
wants to merge 1 commit into from

Conversation

jnizet
Copy link
Member

@jnizet jnizet commented Dec 29, 2017

the datepicker now honors the Angular LOCALE_ID and uses the registered
locale data to internationalize the months and days names. Unless you
really want custom names, providing a custom NgbDatepickerI18n service
isn't necessary anymore to internationalize the datepicker.

fix #2065

BREAKING CHANGE: if your application provides a LOCALE_ID other than
the default en-US, registers the locale data for this locale, and
doesn't use a custom NgbDatepickerI18n, then the days and months
of the datepicker won't be displayed in English anymore, but in the
langguage of the provided locale.

@pkozlowski-opensource
Copy link
Member

@maxokorokov I would love to land this one, but I would live to have your opinion first. Could you PTAL?

Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

Thanks for the update, LGTM generally speaking.

One comment: we should use FormStyle.Standalone instead of FormStyle.Format, as the former is in Nominative and latter is Genitive. Checked for Russian and current solution returns incorrect form.

And a minor concern: these APIs are still marked as experimental, but I guess it shouldn't be a problem

@@ -34,9 +30,23 @@ export abstract class NgbDatepickerI18n {

@Injectable()
export class NgbDatepickerI18nDefault extends NgbDatepickerI18n {
getWeekdayShortName(weekday: number): string { return WEEKDAYS_SHORT[weekday - 1]; }
private weekdaysShort: Array<string>;
Copy link
Member

Choose a reason for hiding this comment

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

If you'll be fixing FormStyle.Format, could you also prefix these with _ as we usually do with private members

the datepicker now honors the Angular LOCALE_ID and uses the registered
locale data to internationalize the months and days names. Unless you
really want custom names, providing a custom NgbDatepickerI18n service
isn't necessary anymore to internationalize the datepicker.

fix ng-bootstrap#2065

BREAKING CHANGE: if your application provides a LOCALE_ID other than
the default en-US, registers the locale data for this locale, and
doesn't use a custom NgbDatepickerI18n, then the days and months
of the datepicker won't be displayed in English anymore, but in the
langguage of the provided locale.
@jnizet
Copy link
Member Author

jnizet commented Feb 18, 2018

@maxokorokov sorry for the late update. I somehow missed your review, then was in holiday. I applied your suggested fixes and rebased on master.

@nicoabie
Copy link
Contributor

nicoabie commented Feb 21, 2018

I would be nice to add a default implementation of NgbDateAdapter<Date> as well, it is very common case too, right? @pkozlowski-opensource

@maxokorokov
Copy link
Member

@nicoabie #1977

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datepicker i18n could now be automatic
4 participants