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

Create <Time/> component #5481

Closed
bmuenzenmeyer opened this issue Jul 8, 2023 · 6 comments · Fixed by #5497
Closed

Create <Time/> component #5481

bmuenzenmeyer opened this issue Jul 8, 2023 · 6 comments · Fixed by #5497
Labels

Comments

@bmuenzenmeyer
Copy link
Collaborator

bmuenzenmeyer commented Jul 8, 2023

Create a generic Time cimponent that displays date info in the curent locale.

The component should have two props: date, which should be a string or Date object and the second one. corresponding to the shape of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat#options

The component should return a time element, with the DateTime formatted on the currently applied locale (so the component must use useLocale hook).

See the repeated usage of a workaround from #5479. The new component would replace these.

<time dateTime={post.date}>
  {new Date(post.date).toLocaleString('en-GB', {
    timeZone: 'UTC',
    month: 'short',
    day: '2-digit',
  })}
</time>
@AugustinMauroy
Copy link
Member

What do you thinks about using formattedDate ?

@ovflowd
Copy link
Member

ovflowd commented Jul 9, 2023

What do you thinks about using formattedDate ?

Could you elaborate? formattedDate was a file on util that we removed because we got rid of strftime dependency, as it is not really needed. This issue simple entails about making a simple component.

@AugustinMauroy
Copy link
Member

We don’t actually need a time commponent because react-intl furnish one

in formattedDate we just need to intput date and nothing else because there are an intlProvider

@ovflowd
Copy link
Member

ovflowd commented Jul 9, 2023

We don’t actually need a time commponent because react-intl furnish one

The implementation we're working on does not replace react-intl's FormattedDate/FormattedTime Components.

While we could use <FormattedDate> to achieve a similar outcome as toLocaleString, it's important to note that react-intl utilizes DateTimeFormat underneath.

However, what you mentioned isn't entirely accurate because we're not duplicating react-intl's functionality. Our intention is to create a reusable component that returns a time element.

If you refer to the documentation of react-intl (https://formatjs.io/docs/react-intl/components/#formatteddate), you'll find the following statement:

This component uses the formatDate and Intl.DateTimeFormat APIs and has props that correspond to the DateTimeFormatOptions specified above.

This aligns perfectly with our approach. While you could argue for using <FormattedDate>, both options achieve the same effect.

@AugustinMauroy
Copy link
Member

A good point for react-intl is that we don't have to maintain it. It's an api of the package

@ovflowd
Copy link
Member

ovflowd commented Jul 10, 2023

A good point for react-intl is that we don't have to maintain it. It's an api of the package

We neither need to maintain toLocaleString, @AugustinMauroy, it's part of EcmaScript specification. That's the point you're not getting.

Using toLocaleString and FormattedDate have the same options, same result, same outcome. Yet the only benefit of FormattedDate over toLocaleString is that we don't need to pass the locale code and call useLocale. (So it's a convenience)

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