-
Notifications
You must be signed in to change notification settings - Fork 441
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
Fix(MessagesList): Limit relative date up to a week. #11837
Conversation
@@ -561,6 +561,8 @@ export default { | |||
return t('spreed', 'Today') | |||
case 1: | |||
return t('spreed', 'Yesterday') | |||
case 7: | |||
return t('spreed', 'A week ago') | |||
default: | |||
return t('spreed', '{n} days ago', { n: diffDays }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return t('spreed', '{n} days ago', { n: diffDays }) | |
return n('spreed', .... { n: diffDays }) |
Needs to use the plural function
Don't know out of my head how it is in JS
But actually there should be a relative time function already which you can use directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something for it in moment lib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something for it in moment lib
Yeah, but it still needs some customization. We need a combination of date.fromNow()
and moment().calendar()
because the first shows the past days correctly ( except for yesterday and today, it shows in hours) and the second shows yesterday and tomorrow ( but along with hh:mm).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we simply use what the activity app does? https://github.com/nextcloud/activity/blob/master/src/components/ActivityGroup.vue#L32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's still not a default output from the lib. We are not avoiding the hurdle of custom anyhow.
We keep it as simple as it is now :) ? but it's better to use date.fromNow()
instead of t('spreed', '{n} days ago', { n: diffDays })
indeed
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
e6c93e3
to
30af508
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected, left a couple of comments
☑️ Resolves
🖌️ UI Checklist
🖼️ Screenshots / Screencasts
🏁 Checklist