[DatePicker] Default to ISO-8601 DateTimeFormat & firstDayOfWeek

[mbrookes]

Closes #1839 #2980.

[oliviertassinari]

Yeah, let's kill this 😁.

[oliviertassinari]

I couldn't find a way to just output warnings without blocking the build.

[alitaheri]

I wanted to try to make it optional, but then I see it doesn't work well with docs either, so: npm start !== npm run lint. Let's kill it before it lays eggs 🙀

[mbrookes]

Yeah, let's kill this 😁.

Oops! Must have git commit -am after carefully adding the individual files.

If you;re serious about killing this though, I'll leave it. 👍

[alitaheri]

Count me in 👿 🔫 😎

[mbrookes]

I decided to put it back, and I'll kill it in a separate PR, as we reference it in contributing.md, so a separate PR will be cleaner as I can edit that at the same time. When I do, should I remove the whole preloaders section, or leave it empty for future reference?

[alitaheri]

I'd say remove it. like how we should remove unused vars. less clutter = less confusion 👍

[mbrookes]

Okay, moved the ISO date formatter to DateTime.format, which also solves the docs issue.

The linter is throwing a fit though...

[oliviertassinari]

@mbrookes That looks good so far 👍. Side question. What are we going to do with the documentation? Is this ok to have Monday as the default starting day?

[mbrookes]

Good point. Well, we no longer need to change the firstDayOfWeek for the fr example, but maybe we can have a US locale example that sets firstDayOfWeek to Sunday, and reinstates the wonky US date format ;-)

[mbrookes]

While toLocaleDateString() doesn't reliably convert the date to the user's locale across all browsers (some return US date format regardless), it should reliably show US date format for US users. Is that sufficient?

[oliviertassinari]

I don't get it, why can't we use the default formatDate?

[mbrookes]

Because the default is ISO 8601? That's kind of the point of this PR. 😉

[oliviertassinari]

Good point! So, what about

new DateTimeFormat('en-US', {
    day: 'numeric',
    month: 'numeric',
    year: 'numeric',
  }).format(date) // 2/25/2016
``

[mbrookes]

But I can use your DateTimeFormat example as a safer alternative to date.toLocaleDateString().

[mbrookes]

What do you think about the idea of modifying formatDate to pass the locale if present, as well as the date to format?

Alternatively / additionally, what if we internalised that, so that defaultDate uses the provided locale and DateTimeFormat if an alternative formatDate isn't provided?

[oliviertassinari]

What do you think about the idea of modifying formatDate to pass the locale if present, as well as the date to format?

I don't really mind. This has advantages and drawbacks.

[mbrookes]

This has advantages and drawbacks.

What drawbacks specifically?

Also:

Alternatively / additionally, what if we internalised that, so that defaultDate uses the provided locale and DateTimeFormat if an alternative formatDate isn't provided?

Any thoughts?

[oliviertassinari]

This new local argument would make the life easier for someone how want to implement the formatDate. However, that's adding a bit of complexity to the DatePicker component.
I feel like the advantage outperform the drawback 👍.

Any thoughts?

That sounds like an excellent idea. I wouldn't say it's a breaking changes, I see more this like an improvement. That would also make our i18n examples simpler 👍.

[mbrookes]

That would also make our i18n examples simpler.

Exactly! 👌 (I realised when I started to update the examples...)

[mbrookes]

I've added localised formatting.

I also coded up adding locale to the formatDate parameters (it really doesn't add any complexity internally to DatePicker), but then realised that if locale is being set programatically from state or store, then the user's formatDate function could take it from there, and it probably doesn't need it to be passed in as a param.

Does that make sense?

[oliviertassinari]

if (areIntlLocalesSupported(['fr', 'en-US'])) {

[mbrookes]

How did I miss that?! Although when I tested in Safari, which is the only mainstream desktop browser that doesn't support Intl, it worked fine (presumably since it's a binary condition).

Anyway, good catch (as always!). Updating now.

[oliviertassinari]

Well, that's not a big deal. I'm pretty sure that if fr is available, en-US is also available.

[oliviertassinari]

it probably doesn't need it to be passed in as a param

That's the complexity I was talking about 😁.

@mbrookes Could you have a look at my comment? Otherwise, that looks awesome 👍!

[mbrookes]

Updated. Thanks!

[oliviertassinari]

Could you squash down?

[mbrookes]

Done. 👍

[alitaheri]

This was a great work @mbrookes 👍 👍 Thanks a lot 😁