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

Formatting of relative and absolute dates using Intl.* APIs #14173

Open
wants to merge 61 commits into
base: development
Choose a base branch
from

Conversation

niik
Copy link
Member

@niik niik commented Mar 15, 2022

Closes #13943

Description

This adds region-aware formatting for relative and absolute dates using the Intl namespace. This builds on #14149, #14123 and app.getLocaleCountryCode

Note that this is not about localizing strings, the interface will remain US-english, it will however apply the correct (hopefully) regional settings for formatting numbers. Most notably right now this means that users like myself who have their OS configured to display 24-hour format will now.

Note that this is feature-flagged and currently only enabled for dev.

cc @tsvetilian-ty as author of #13965

Screenshots

image

For reference these are my regional settings on Windows and macOS (they resolve to en-SE on both platforms)

image

image

Release notes

Notes:

Comment on lines 26 to 28
transformIgnorePatterns: ['node_modules/(?!(@github))'],
transformIgnorePatterns: [
'node_modules/(?!(@github|bcp-47|is-decimal|is-alphabetical|is-alphanumerical))',
],
Copy link
Member

Choose a reason for hiding this comment

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

Everything is terrible

:its-true-all-of-it:

@niik niik requested a review from sergiou87 March 15, 2022 12:41
@niik
Copy link
Member Author

niik commented Mar 15, 2022

@sergiou87 Merge conflict, need a little ✅ again please

Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

As discussed in Slack, it's not picking the 24h format for me, but seems to be an issue with Intl rather than with your implementation:

image

image

app/src/lib/formatting-locale.ts Outdated Show resolved Hide resolved
niik and others added 3 commits March 16, 2022 12:49
Co-authored-by: Sergio Padrino <sergio.padrino@gmail.com>
en-US-POSIX gives us something very close to the standard behavior of number.toString() et al, i.e. an invariant culture
@niik niik changed the title Region-aware formatting based on user locale Formatting of relative and absolute dates using Intl.* APIs Mar 18, 2022
@niik
Copy link
Member Author

niik commented Mar 18, 2022

Alright, after significant investigation here we've determined that we're not quite ready to jump on the regional-aware train just yet. There are some notable en-* locales that isn't defined in the Unicode Common Locale Data Repository such as en-ES, en-NO, en-FR, etc.

We have ideas for how to address this though and in the meantime we'll be using the en-US-POSIX locale by default. The POSIX variant of en-US is essentially US-english but without number grouping (i.e it's essentially .toString()).

Given the recent discoveries I'm moving the feature flag back to dev and intend to tackle the locale resolution in a separate PR.

@sergiou87 Another look when you've got time please?

@niik niik requested a review from sergiou87 March 18, 2022 10:50
sergiou87
sergiou87 previously approved these changes Mar 18, 2022
Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

Looks and works good! :shipit:

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.

Option to display time/date with 24h clock
3 participants