-
Notifications
You must be signed in to change notification settings - Fork 182
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) Adds support for non-Gregorian calendars, with special work-arounds for Amharic #790
Conversation
…ounds for Amharic
cc: @amn-icap @gracepotma This is my proposal for a better way of achieving the goal of #786. |
Size Change: -2.09 MB (-41%) 🎉 Total Size: 2.99 MB
ℹ️ View Unchanged
|
Wow - bravo Ian for pushing this through, and so fast. CC @aman-icap |
// Custom formatting for English. Use hyphens instead of spaces. | ||
localeString = localeString.replace(/ /g, "-"); | ||
|
||
if (_locale.language === "am") { |
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.
@ibacher why do we need to keep the month names in English? Wouldn't it better to keep them in Amharic ? What do you think @aman-icap
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 did this because I was trying to keep this PR in line with the functionality in #786, which also uses English names for months in Amharic. I mentioned that in the PR description. My preference would be to not have that hack at all.
Any updates here? @ebambo |
In the absence of any updates, approvals, etc. I'm just going to merge this in as it should be harmless for existing use-cases. Future work needed here can be addressed as additional tickets. |
Update from Dagim: the ICAP-Eth team is testing the work done on this branch and recommended we disregard the other/original #786 PR for now. |
Requirements
For changes to apps
If applicable
Summary
Basically, this has the same intention as #786, but with a somewhat cleaner implementation, utilising the standard JS library. It also adds support for registering default custom calendars on a per-locale basis, which is currently only used to use the Ethiopian calendar when the locale is Amharic.
See the added tests for some of what this PR allows.
Screenshots
Related Issue
Other
There's a hack in here I don't love: if the locale is set to Amharic, we format the date in English, but using the Ethiopic calendar, but this was done to produce the same results as #786.
#786 also basically stops the display of time strings, but this is a configurable option for the call to
formatDate()
, and we should probably have a mechanism to handle the implementation-specific defaults rather than just relying on the workaround of having a separate formatter that doesn't display time in some circumstances.