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

on_date translation is problematic #24074

Closed
silverwind opened this issue Apr 12, 2023 · 16 comments · Fixed by #24106
Closed

on_date translation is problematic #24074

silverwind opened this issue Apr 12, 2023 · 16 comments · Fixed by #24106

Comments

@silverwind
Copy link
Member

silverwind commented Apr 12, 2023

Description

As seen on Crowdin, translators have a hard time translating the on alone in on <date> phrases because some languages don't have a concept of such a word. We should evaluate if there are alternatives and whether we can somehow translate the whole phrase.

@yardenshoham
Copy link
Member

Blocked by #23863?

@lunny
Copy link
Member

lunny commented Apr 13, 2023

Blocked by #23863?

I don't think so.

@silverwind
Copy link
Member Author

silverwind commented Apr 13, 2023

I think the solution will be to never have the time element emit this "on", and instead render the element into the translation string via a token. I also checked the JS Intl API, it seems it also can not emit a "on", it can emit a "at" but that does not help us.

@yardenshoham
Copy link
Member

One way to avoid on is to use tense=past whenever possible. Ref #24106

@wxiaoguang
Copy link
Contributor

I would suggest to remove the on prefix from TimeSince, if it is not necessary.

Is there a strong reason to keep it?

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2023

In some contexts the on makes sense to form sentences:

image

I agree on needs to be removed from TimeSince (and in turn <relative-time>), and these sentences should be formed based on a translation string on %s. where %s than renders the TimeSince.

@yardenshoham
Copy link
Member

about the last sentence. Not TimeSince, an absolute datetime is more like template "shared/datetime/short"

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2023

Though, the challenge ist that <relative-time> may or may not output the on <date> format, depending on how far back the date is. So I guess the backend must know as well when the on appears to emit the correct translation to avoid on 5 days ago.

@yardenshoham
Copy link
Member

Threshold is configurable

@wxiaoguang
Copy link
Contributor

Quote my comment from #24106

In history , IIRC the TimeSince always used relative date, then <relative-time> changed the behavior.

Now, we just make <relative-time> always use tense=past, then it behaves like the old TimeSince , then everything should be fine, and just like before.

Then we do not need the on either.

Correct me if I am wrong.

@yardenshoham
Copy link
Member

If you use template "shared/datetime/short" it will just output the day, no on

@wxiaoguang
Copy link
Contributor

New code:

<td>{{template "shared/datetime/short" (dict "Datetime" .CreatedUnix.FormatLong "Fallback" .CreatedUnix.FormatShort)}}</td>

Old code:

<td><span class="notice-created-time" data-tooltip-content="{{.CreatedUnix.AsTime}}"><time data-format="short-date" datetime="{{.CreatedUnix.FormatLong}}">{{.CreatedUnix.FormatShort}}</time></span></td>

There is no on in old code either. Since this problem couldn't be resolved easily, I would suggest to use the old code's behavior.

@yardenshoham
Copy link
Member

I'm fine with tense=past by default, just have to be careful with future dates. @silverwind the problem with this is it will show 4 years ago instead of on April 14, 2019 which you prefer

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2023

Threshold is configurable

Then I think it's clear what needs to be done: Make backend aware of the threshold, and make it output a on %s translation string before the threshold and %s when not.

have to be careful with future dates

Future dates should be treated the same as past dates in regards to the threshold.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 14, 2023

IMO, there are different cases:

  1. Only relative time
  2. An absolute time without on: a table cell, without context text
  3. In a sentence with on

So, only the caller knows whether there should be an on word. As a framework function, TimeSince has better to keep simple.

@yardenshoham
Copy link
Member

The first should use TimeSince (relative-time with format="relative"). The second and third should be inserted into a phrase and use a function (we currently have template "shared/datetime/short" which is relative-time with format="datetime")

@yardenshoham yardenshoham added this to the 1.20.0 milestone Apr 14, 2023
silverwind pushed a commit that referenced this issue Apr 15, 2023
- Follows #23988 
- Fixes: #24074 by removing this key

GitHub's `relative-time` elements allow us to force their rendering to
`auto`, `past`, or `future` tense. We will never show an absolute date
`on ...` in `TimeSince`

## Before

![image](https://user-images.githubusercontent.com/20454870/231735872-048c7bf3-6aa1-4113-929d-75a985c9922c.png)

## After

![image](https://user-images.githubusercontent.com/20454870/231736116-6ad47b63-77f4-4d3f-82a2-ee9a46ba2bd1.png)

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants