Skip to content

lightbox: Use a friendlier format for the date, like "Yesterday at 4:47 PM" #45

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

Open
chrisbobbe opened this issue Mar 31, 2023 · 7 comments
Labels
a-i18n Translation, localization, internationalization a-lightbox The lightbox / image-viewer screen

Comments

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Mar 31, 2023

…or "A few seconds ago", etc.

Currently it shows something like "Mar 31, 2023 15:09:51".

Implementation (full solution)

It's possible that we'll want to merge a partial solution before implementing this, but anyway I wanted to write down some findings.

Dart intl.DateFormat doesn't support some desired features:

  1. Relative times, like "21 minutes ago"
  2. Combining a date and time into one string with a localized separator, such as " at " in English, " um " in German, or " " in Japanese.

We could work around, with our own code or maybe with a library in Dart (such as timeago or relative_time). But (1) is slightly tricky to implement, and requires giving more plural-syntax strings to our volunteer translators. For (2) we could just give the separator to our translators, but that feels unsatisfying because it shouldn't be necessary.

I propose using an ICU library via ffi, which we already plan to do for other localization features: #1165 (comment)

Otherwise, use DateFormat::createInstanceForSkeleton, passing an appropriate "skeleton" (see this helpful explainer):

  • If the timestamp was yesterday, say "Yesterday at {time}". For time, use skeleton jmm, for "12:15 PM".
  • If the timestamp was before yesterday and in the current year, omit the year; use skeleton MMMdjmm, for "Jan 2 at 5:00 PM"
  • If the timestamp was before the current year, include the year, using skeleton yMMMdjmm, for "Jan 2, 2024 at 5:00 PM".
  • If the timestamp is in the future (maybe the server's clock is wrong?), also use skeleton yMMMdjmm. A relative time like "in 21 minutes" would be awkward here.

The strings must be formatted with the same time zone and calendar (Gregorian, Chinese lunar, etc.) as we use to decide which day or year the timestamp is on. We should be able to choose an appropriate Calendar for the which-day/year calculation and pass that Calendar to DateFormat::createInstanceForSkeleton.

We should periodically force-rebuild the widget that shows the time, to avoid freezing with text like "1 minute ago".

We should offer a tooltip that shows the full date and time, with seconds, plus the timezone; skeleton yMMMdjmmsszzzz.

@gnprice gnprice added the m-beta label May 26, 2023
@gnprice gnprice added this to the Beta milestone May 27, 2023
@gnprice gnprice added a-lightbox The lightbox / image-viewer screen a-i18n Translation, localization, internationalization and removed m-beta labels May 27, 2023
@gnprice gnprice modified the milestones: Beta, Launch Sep 22, 2023
@gnprice gnprice modified the milestones: Launch, B2: Summer 2024 May 9, 2024
@mofirojean
Copy link

@gnprice can i work on this?

@gnprice
Copy link
Member

gnprice commented Oct 2, 2024

Sure. Do see our guide to picking an issue:
https://zulip.readthedocs.io/en/latest/contributing/contributing.html#picking-an-issue-to-work-on

so in particular before claiming an issue, you should have already gotten started working on it, finding the relevant piece of code and getting an idea of how you'll approach it.

Also note this issue is in the "Post-launch" milestone, not "Launch" or earlier. That means the core team will have limited time to spend helping with this issue for the next few months. But I think the issue is simple enough that that should be fine.

@mofirojean
Copy link

mofirojean commented Oct 5, 2024

@gnprice I had a shot at this issue and is the result

The device i am using is my Techno Spark 7 -
Android 11
Here is the result: the time stamps were changed to a more readable date format

Screenshots

Screenshot_20241005-040206
Screenshot_20241005-041103
Screenshot_20241005-031245
Screenshot_20241005-031431
Screenshot_20241005-031134

Before making a PR, I would like to verify if a change like this would require that tests be made or updated? since I just added a method to the widget to format the date.

In the course of solving this issue, i added localization to transform the time to the local of the user; it address #278 for this date.

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Dec 22, 2024
Changed the timestamp format from Mar 31, 2023 15:09:51 to a more
readable Mar 31, 2023 at 3:09 PM.
Added proper localization support to adapt timestamps to the user's
language and region preferences.
Optimized the timestamp logic to ensure accurate categorization into
Today, Yesterday, or specific dates.

Fixes: zulip#45
@lakshya1goel
Copy link
Contributor

Hello, I have worked on the issue, Please review the changes and let me know if anything else is required.
@chrisbobbe

@lakshya1goel
Copy link
Contributor

Hello, It would be great to receive some reviews on my PR. Looking forward for your response.
Thanks!

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Jan 14, 2025
Changed the timestamp format from Mar 31, 2023 15:09:51 to a more
readable Mar 31, 2023 at 3:09 PM.
Added proper localization support to adapt timestamps to the user's
language and region preferences.
Optimized the timestamp logic to ensure accurate categorization into
Today, Yesterday, or specific dates.

Fixes: zulip#45
@bebialen
Copy link

bebialen commented Apr 2, 2025

I noticed this issue is still open and labeled as "help wanted". Has the existing PR been abandoned, or can I contribute to improving the solution?

@gnprice
Copy link
Member

gnprice commented May 10, 2025

Thanks. In January we learned more about the options for how to implement this, which is reflected now in the issue description. Based on those updates, I think this no longer belongs as a "help wanted" issue

That's because this is going to require as a prerequisite pulling in an ICU implementation via dart:ffi, and it looks like the hoped-for such implementation isn't yet ready. For details on that implementation, see the upstream thread linked from #1165 (comment) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-i18n Translation, localization, internationalization a-lightbox The lightbox / image-viewer screen
Projects
Status: No status
5 participants