Skip to content

Recipient header date format #429

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

Merged
merged 1 commit into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,14 @@
"@errorMarkAsReadFailedTitle": {
"description": "Error title when mark as read action failed."
},
"today": "Today",
"@today": {
"description": "Term to use to reference the current day."
},
"yesterday": "Yesterday",
"@yesterday": {
"description": "Term to use to reference the previous day."
},
"userRoleOwner": "Owner",
"@userRoleOwner": {
"description": "Label for UserRole.owner"
Expand Down
43 changes: 40 additions & 3 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ class RecipientHeaderDate extends StatelessWidget {

@override
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
return Padding(
padding: const EdgeInsets.fromLTRB(10, 0, 16, 0),
child: Text(
Expand All @@ -725,12 +726,48 @@ class RecipientHeaderDate extends StatelessWidget {
// https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-caps#all-small-caps
fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')],
).merge(weightVariableTextStyle(context)),
_kRecipientHeaderDateFormat.format(
DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000))));
formatHeaderDate(
zulipLocalizations,
DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000),
now: DateTime.now())));
}
}

final _kRecipientHeaderDateFormat = DateFormat('y-MM-dd', 'en_US'); // TODO(#278)
@visibleForTesting
String formatHeaderDate(
ZulipLocalizations zulipLocalizations,
DateTime dateTime, {
required DateTime now,
}) {
assert(!dateTime.isUtc && !now.isUtc,
'`dateTime` and `now` need to be in local time.');

if (dateTime.year == now.year &&
dateTime.month == now.month &&
dateTime.day == now.day) {
return zulipLocalizations.today;
}

final yesterday = now
.copyWith(hour: 12, minute: 0, second: 0, millisecond: 0, microsecond: 0)
.add(const Duration(days: -1));
Comment on lines +751 to +753
Copy link
Member

Choose a reason for hiding this comment

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

I believe I've heard of a locale where this wouldn't work — IIRC in parts of Ethiopia, the days run from midnight to midnight but the hours are counted from noon to noon.

But I'm not certain how to write correct logic for that: partly because I'm not sure I remember the details right, partly because I'm not sure how other software handles it, including DateTime.copyWith, and partly because it's just complicated to think about. So it wouldn't make sense to try to handle that case without writing tests for it. Since we don't have a reasonable way to control the timezone (as commented on that existing test below), I think it's best to just ignore that case at present.

if (dateTime.year == yesterday.year &&
dateTime.month == yesterday.month &&
dateTime.day == yesterday.day) {
return zulipLocalizations.yesterday;
}

// If it is Dec 1 and you see a label that says `Dec 2`
// it could be misinterpreted as Dec 2 of the previous
// year. For times in the future, those still on the
// current day will show as today (handled above) and
// any dates beyond that show up with the year.
if (dateTime.year == now.year && dateTime.isBefore(now)) {
return DateFormat.MMMd().format(dateTime);
} else {
return DateFormat.yMMMd().format(dateTime);
}
}

/// A Zulip message, showing the sender's name and avatar if specified.
class MessageWithPossibleSender extends StatelessWidget {
Expand Down
29 changes: 26 additions & 3 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ void main() {
testWidgets('show dates', (tester) async {
await setupMessageListPage(tester, messages: [
eg.streamMessage(timestamp: 1671409088),
eg.dmMessage(timestamp: 1692755322, from: eg.selfUser, to: []),
eg.dmMessage(timestamp: 1661219322, from: eg.selfUser, to: []),
]);
// We show the dates in the user's timezone. Dart's standard library
// doesn't give us a way to control which timezone is used — only to
Expand All @@ -377,11 +377,34 @@ void main() {
// https://github.com/dart-lang/sdk/issues/28985 (about DateTime.now, not timezone)
// https://github.com/dart-lang/sdk/issues/44928 (about the Dart implementation's own internal tests)
// For this test, just accept outputs corresponding to any possible timezone.
tester.widget(find.textContaining(RegExp("2022-12-1[89]")));
tester.widget(find.textContaining(RegExp("2023-08-2[23]")));
tester.widget(find.textContaining(RegExp("Dec 1[89], 2022")));
tester.widget(find.textContaining(RegExp("Aug 2[23], 2022")));
});
});

group('formatHeaderDate', () {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
final now = DateTime.parse("2023-01-10 12:00");
final testCases = [
("2023-01-10 12:00", zulipLocalizations.today),
("2023-01-10 00:00", zulipLocalizations.today),
("2023-01-10 23:59", zulipLocalizations.today),
("2023-01-09 23:59", zulipLocalizations.yesterday),
("2023-01-09 00:00", zulipLocalizations.yesterday),
("2023-01-08 00:00", "Jan 8"),
("2022-12-31 00:00", "Dec 31, 2022"),
// Future times
("2023-01-10 19:00", zulipLocalizations.today),
("2023-01-11 00:00", "Jan 11, 2023"),
];
for (final (dateTime, expected) in testCases) {
test('$dateTime returns $expected', () {
check(formatHeaderDate(zulipLocalizations, DateTime.parse(dateTime), now: now))
.equals(expected);
});
}
});

group('MessageWithPossibleSender', () {
testWidgets('Updates avatar on RealmUserUpdateEvent', (tester) async {
addTearDown(testBinding.reset);
Expand Down