-
Notifications
You must be signed in to change notification settings - Fork 63
refactor: move to NcDateTime for formatted dates
#2398
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
Conversation
This also removes the dependency on the server provided `.live-relative-timestamp` handling. Signed-off-by: Ferdinand Thiessen <[email protected]>
Antreesy
left a comment
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.
Tested, thanks for taking care!
Signed-off-by: Ferdinand Thiessen <[email protected]>
|
/compile |
Signed-off-by: nextcloud-command <[email protected]>
| <div class="notification-heading"> | ||
| <span class="hidden-visually">{{ absoluteDate }}</span> | ||
| <span | ||
| <NcDateTime |
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.
Similar to https://github.com/nextcloud/announcementcenter/pull/945/files#r2158705117 I'd also here use a bit longer format on the date for human-readability
@Antreesy did you notice the change and think the new format is good/better? Or would you be okay reverting to the previous longer format?
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.
I personally like short version, if it's about 20. Dezember 2020 against 20.12.2020,
But we can pass :format="{ timeStyle: 'short', dateStyle: 'long' }" as in another app, yes
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.
@nickvergessen in that case I would say designers should be asked and we change the default on @nextcloud/vue as this is also the same on files app, logreader etc.
Meaning we should have a consistent design on that.
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.
Not fully sure we will find "one that fits all" that's why there is a full/long/short for the relative thing as well, but yeah I'll try to bring it up with designers.
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.
@nickvergessen but we all use it for the same purpose: alternative hover title when using relative time.
So as we have all the same use case we should also all use the same format for consistency.
I agree your proposal looks better!
This also removes the dependency on the server provided
.live-relative-timestamphandling.