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

Ability to explicitly pass timeZone to IntlProvider #702

Closed
brichardssa opened this issue Oct 23, 2016 · 17 comments
Closed

Ability to explicitly pass timeZone to IntlProvider #702

brichardssa opened this issue Oct 23, 2016 · 17 comments

Comments

@brichardssa
Copy link
Contributor

brichardssa commented Oct 23, 2016

I'd like to be able to set default formats for the entire application which are used by each of the different format functions. Currently, when passing defaultFormats to IntlProvider, these formats are only used by the formatMessage function.

For example, if I want all times displayed in a specific time zone (namely Africa/Johannesburg), it would be useful to pass the following to IntlProvider:

    return (
      <IntlProvider
        defaultFormats={{
          time: {
            timeZone: 'Africa/Johannesburg'
          }
        }}>
...
@ericf
Copy link
Collaborator

ericf commented Oct 25, 2016

@brichardssa is timeZone the main use case you have for this? Because I've been thinking for awhile that we should expose timeZone as a top-level prop to <IntlProvider>, similar to locale.

@brichardssa
Copy link
Contributor Author

For our particular use case it is. I still think that the ability to provide defaultFormats for dates and numbers would make this a lot more user friendly

@ericf
Copy link
Collaborator

ericf commented Oct 25, 2016

@brichardssa I'd like to understand what those concrete uses cases would be that aren't handled by the named formats features and/or component reuse. Timezone is the main one that stands out to me that's in the same realm as locale that <IntlProvider> should provide and date/time formatting should use as the default.

I would rather accept a PR that just added support for timeZone propagation first, before adding this generic feature.

@brichardssa
Copy link
Contributor Author

I'll do that. I just think that giving people the ability to specify default formats in the form of the Intl documentation provides a more direct way for users to control the behaviour of react-intl

@ericf
Copy link
Collaborator

ericf commented Oct 26, 2016

The formats and defaultFormats (fallback) props on <IntlProvider> require a named format configuration, e.g.:

<IntlProvider
  formats={{
    date: {
      foo: {
        timeZone: 'Africa/Johannesburg',
      },
    },
  }}
>

I see how your PR #707 supports formateDate/Time() and <FormattedDate/Time>, but it won't support <FormattedMessage> 😢. Is this what you were talking about with the consistency?

I think to make this work, we'd want to update the intl-messageformat package to support it taking in defaults, and this can be supplied {timeZone: valueFromIntlProviderProps}.

@juandopazo
Copy link
Contributor

I'm definitely ok with a timeZone top level attribute for IntlProvider. While the user might change their time zone in our use cases, we always refresh the page for it. Not sure it's reasonable to keep the TZ in a store.

Defaults sound like they might come back to bite you. It's kind of like using context, but without seeing the context.

@ericf
Copy link
Collaborator

ericf commented Oct 26, 2016

@juandopazo the implementation detail we need to figure out is how to propagate the timeZone value passed to IntlProvider down to Intl.DateTimeFormat and IntlMessageFormat. For IntlMessageFormat it can't involve the actual message string being any different; e.g. Today is {now, date}, and not Today is {now, date, userTZ}. This is why I was talking about passing defaults to IntlMessageFormat.

@brichardssa brichardssa changed the title Ability to pass defaultFormats to be used by all of the format functions Ability to explicitly pass timeZone to IntlProvider Oct 27, 2016
@brichardssa
Copy link
Contributor Author

Just updated the title of this issue to reflect what we are trying to achieve here. Will look into this as soon as I get some time

@juandopazo
Copy link
Contributor

juandopazo commented Oct 27, 2016

So... FYI, most browsers have trouble with arbitrary timezones: http://kangax.github.io/compat-table/esintl/#test-DateTimeFormat_accepts_IANA_timezone_names

Firefox bug if you want to lobby for them to fix it: https://bugzilla.mozilla.org/show_bug.cgi?id=1266290

@brichardssa
Copy link
Contributor Author

That's true. V8 logs a pretty straight forward error message when an unrecognised timezone is passed to it. Node < 6 give me this: "RangeError: Unsupported time zone specified Pacific/Samoa"

It would be interesting to see what the other browsers do.

@juandopazo
Copy link
Contributor

juandopazo commented Oct 28, 2016

Firefox and IE have the same issue. And the biggest problem is the polyfill does not support them either.

@PoojaSSW
Copy link

PoojaSSW commented Nov 8, 2016

Has the timeZone prop been added to the IntlProvider. I pulled latest and didnt see the changes?can we now add the prop to the IntlProvider directly?

@brichardssa
Copy link
Contributor Author

Unfortunately not yet. In order to achieve this, we need to request some changes in one of react-intl's dependencies.

MethodenMann pushed a commit to tocco/tocco-client that referenced this issue Dec 7, 2016
- because of a different timezone of travis, this tests will not match the same time.
   With this open issue the problem could be fixed without changing FormattedValue component
  formatjs/formatjs#702
MethodenMann pushed a commit to tocco/tocco-client that referenced this issue Dec 7, 2016
- because of a different timezone of travis, this tests will not match the same time.
   With this open issue the problem could be fixed without changing FormattedValue component
  formatjs/formatjs#702
@oscar-b
Copy link

oscar-b commented Jan 23, 2017

So... FYI, most browsers have trouble with arbitrary timezones: http://kangax.github.io/compat-table/esintl/#test-DateTimeFormat_accepts_IANA_timezone_names

@juandopazo What's the recommended way to handle this? Seems like Firefox supports this in a coming version (no idea how often they release nowadays) but IE11 and older Safari are still problematic. Right now IE11 outputs a long datetime string instead, when a timezone is specified. Is there a simple way to check if timezone is supported so it could fallback to browser tz, and couldn't that be added to react-intl?

@HDv2b
Copy link

HDv2b commented Aug 2, 2017

I found this polyfill works with ie11: date-time-format-timezone

@vijaysvs80
Copy link

@brichardssa @ericf Do we have a solution that decided ? Would be greatly appreciated if you could let me know workable solution ? We are facing issue on IE when timezone is passed.

@neutraali
Copy link

Any news on #893? Would solve a lot of arbitrary timezone-prop passes...

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