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.

Sign up for GitHub

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

Jump to bottom

NcDatetime: New component for displaying timestamps as time relative from now #4219

Merged
merged 1 commit into from Jun 22, 2023
Merged

NcDatetime: New component for displaying timestamps as time relative from now #4219

merged 1 commit into from Jun 22, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jun 15, 2023

☑️ Resolves

This implements a component showing for displaying timestamps like x seconds ago without the need of huge libraries like moment.js. It is based on the Intl browser API, which is supported by all of our browser targets.

Tests included.

🖼️ Screenshots

image

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@susnux susnux added enhancement New feature or request 3. to review Waiting for reviews labels Jun 15, 2023
@susnux susnux requested review from raimund-schluessler and a team June 15, 2023 13:52
@susnux susnux added this to the 8.0.0 milestone Jun 15, 2023
@susnux
Copy link
Contributor Author

susnux commented Jun 15, 2023

ℹ️ Fixed the tests by enforcing UTC as the timezone, sadly this needs to be done by setting the environment on the node script as setting

process.env.TZ = 'UTC'

does not to work

marcoambrosini
marcoambrosini approved these changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Very nice addition. Tested in the docs and it works nicely

Would it be possible to have a prop that allows to show just "a few seconds ago" until the first minute instead of each and every second?

src/components/NcDatetime/NcDatetime.vue
*/
format: {
type: Object,
default: () => ({ timeStyle: 'medium', dateStyle: 'short' }),
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make sense to separate these into two separate props?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which two properties? title format and main format?
Or do you mean timeStyle and dateStyle?

If the latter one, then I do not think so. As there are more options possible I just allowed every Intl.DateTimeFormatOptions option (which allow finer control over the style by setting the format of weekday, hour, month... directly).

@susnux
Copy link
Contributor Author

susnux commented Jun 16, 2023

Would it be possible to have a prop that allows to show just "a few seconds ago" until the first minute instead of each and every second?

Sure, something like :ignore-seconds="true"?

@susnux
Copy link
Contributor Author

susnux commented Jun 22, 2023

Sure, something like :ignore-seconds="true"?

@marcoambrosini This is now implemented :)

@susnux
feat(NcDatetime): Add new component for displaying a timestamp as tim…
9bacc98 
…e from now

This implements a component showing for displaying timestamps like *x seconds ago*
without the need of huge libraries like moment.js

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux requested a review from a team June 22, 2023 18:04
@skjnldsv skjnldsv merged commit e756489 into master Jun 22, 2023
17 checks passed
@skjnldsv skjnldsv deleted the feat/nc-datetime branch June 22, 2023 18:27
@ChristophWurst ChristophWurst mentioned this pull request Jun 23, 2023
Open
1 task
@Chartman123
Copy link
Contributor

Will this have any effect on nextcloud-libraries/nextcloud-moment#407?

@skjnldsv skjnldsv mentioned this pull request Jun 28, 2023
Merged
@joshtrichards joshtrichards mentioned this pull request Jul 11, 2023
Open
@susnux
Copy link
Contributor Author

susnux commented Sep 11, 2023

Will this have any effect on nextcloud-libraries/nextcloud-moment#407?

@Chartman123 Not really, @nextcloud/moment is still supported, this only adds a new component which solves the most common problem when displaying time(stamps). It is not based on moment because it is not required (would introduce much larger dependency and Moment.js is not developed anymore so for new projects it is not recommended).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

Improve Moment.vue, e.g. to get short timestamps Moment
4 participants
@susnux @Chartman123 @skjnldsv @marcoambrosini