Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented May 13, 2025

* needed for nextcloud/server#29807
* replaces nextcloud-libraries/nextcloud-vue#6543

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux requested review from ShGKme and artonge May 13, 2025 11:08
@susnux susnux added type: enhancement 🚀 New feature or request 3. to review 3️⃣ Waiting for reviews labels May 13, 2025
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Should we also expose the formatDateTime function here?

const options: Required<FormatDateOptions> = {
ignoreSeconds: false,
language: getLanguage(),
relativeTime: 'long' as const,
Copy link
Contributor

Choose a reason for hiding this comment

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

What as const does here? Isn't string always a const as it is immutable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type is 'long' rather than string

Copy link
Contributor

Choose a reason for hiding this comment

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

But you have explicit : Required<FormatDateOptions> type, which defines relativeTime as 'long' | 'short' | 'narrow' already

@ShGKme
Copy link
Contributor

ShGKme commented May 13, 2025

In talk-desktop the part that defines the corresponding unit is moved to an individual function, allowing to reuse it in other cases like formatDuration

https://github.com/nextcloud/talk-desktop/blob/main/src/shared/datetime.utils.ts#L72

@ShGKme ShGKme requested a review from Antreesy May 13, 2025 16:08
@ShGKme
Copy link
Contributor

ShGKme commented May 13, 2025

Adding @Antreesy, who's worting on moment replacing functions in Talk

@Antreesy
Copy link

Antreesy commented May 13, 2025

Talk implementation: nextcloud/spreed@d094cf5
Omitting the same year: nextcloud/spreed@5c99b88

@susnux
Copy link
Contributor Author

susnux commented May 14, 2025

to show 'yesterday', 'today', and 'tomorrow'

Its already like this :)

This format was approved by the designers to be used for all our relative dates.

@Antreesy
Copy link

to be used for all our relative dates

Maybe that was the point =)

Discussed in person, Files and Talk have different usecases:

  • this PR provides a wide spectre of the time passed since the timestamp (e.g. minutes, days ago, or some date)
  • Talk needs:
    1. Dates in past (chat separators) - <today | yesterday | 3 days ago | ''>, <day month [year]>
    2. Dates in future (meeting start) - <today | tomorrow | Friday | day month [year]''>, <[hour, minutes]>

Not sure this logic would need to end up in shared library, if no other app wouldn't be using it. cc @ShGKme to proceed with own implementation in Talk

Comment on lines +36 to +39
${'2025-03-01T10:00:00Z'} | ${'long'} | ${'March 1'}
${'2025-03-14T10:00:00Z'} | ${'long'} | ${'March 14'}
${'2025-03-14T10:00:00Z'} | ${'short'} | ${'Mar 14'}
${'2025-03-14T10:00:00Z'} | ${'narrow'} | ${'M 14'}

Choose a reason for hiding this comment

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

Not sure that would fit all cases and expected formats (e.g. March 14 and 14^th March), but still better to have in locale language

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have an example?

Copy link

@Antreesy Antreesy May 14, 2025

Choose a reason for hiding this comment

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

en_US and en_GB maybe? I didn't test, just popped out in my mind

https://meta.m.wikimedia.org/wiki/Date_formats_in_various_languages

I guess English would be the only overlap

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean for different formats.

Copy link
Contributor

@ShGKme ShGKme May 14, 2025

Choose a reason for hiding this comment

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

Or do you mean that we need more then a language here, not a new format?

Copy link
Contributor

@ShGKme ShGKme May 28, 2025

Choose a reason for hiding this comment

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

I am not sure what this is about? It uses the format that is defined by the locale.

Not locale, but language. Works fine for English with British English, as there is British English as a language in Nextcloud).

So the question from Maksim is, are there other such cases, where there is a different locale, but there is no different language (in Nextcloud).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still do not get the problem here? Relative time is formatted in language not locale as the format is mostly returning a "human readable" string instead of a date format (which would be the case for plain date format like "May 14 2025" vs "14 May 2025")

Choose a reason for hiding this comment

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

Yeah, it doesn't sound like a big deal, as long as month is not written as a number

Copy link
Contributor

Choose a reason for hiding this comment

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

It uses the format that is defined by the locale

Relative time is formatted in language not locale

Somewhere there is a typo 👀

I still do not get the problem here? Relative time is formatted in language not locale as the format is mostly returning a "human readable" string instead of a date format (which would be the case for plain date format like "May 14 2025" vs "14 May 2025")

Depends on language and locale difinition.

In Intl.RelativeTimeFormat it is locale, not a 2-digit language code.
In the context of Nextcloud — we suppose to use language, as it is not about regional format.

However, as Maksim mentioned above, there are languages with different formats, for example, English with March 14 and 14^th March in US and British. It is still March, but the format is different.

This case works correctly, as we have en and en-gb as languages in Nextcloud. Users don't need to specify a different locale here.

Not sure that would fit all cases

So the question was, are there other cases where in a single language different formats can be expected here, and not covered in Nextcloud as different languages (like en and en-gb do).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, as Maksim mentioned above, there are languages with different formats, for example, English with March 14 and 14^th March in US and British. It is still March, but the format is different.

That is true but there is no way for us to fix this, because often people use e.g. English language but other locale to get metric units, decimal comma instead of dot or different date format.
But still I for example want the text to be "March 14" rather than having mixed English / German text like "File created vorgestern" or "File created 13. März".

So for relative time its more language than locale.

And for the special case of US vs GB English I think it is already fixed as Nextcloud will report en for US and en-GB for British English which also is the valid BCP47 code expected by the Intl.RelativeTimeFormat as locale.

@susnux susnux merged commit 5efc8cb into main May 28, 2025
16 checks passed
@susnux susnux deleted the feat/format-relative-time branch May 28, 2025 12:24
@susnux susnux mentioned this pull request May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review 3️⃣ Waiting for reviews type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants