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.

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

Timestamps on created and modified at values #3532

Merged

Conversation

luka-nextcloud
Copy link
Contributor

@luka-nextcloud luka-nextcloud commented Jan 12, 2022

Signed-off-by: Luka Trovic luka@nextcloud.com

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@@ -25,6 +25,7 @@
:active="tabId"
:title="title"
:subtitle="subtitle"
:subtitleTooltip="subtitleTooltip"
Copy link
Member

Choose a reason for hiding this comment

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

Mind to fix the eslint warning?

src/components/card/CardSidebar.vue#L28
Attribute ':subtitleTooltip' must be hyphenated

@juliusknorr
Copy link
Member

Otherwise looks good, LGTM once linter is happy 👍

@luka-nextcloud luka-nextcloud force-pushed the feature/timestamps-on-created-and-modified-at-values branch 2 times, most recently from e119f75 to 25de24d Compare January 18, 2022 15:58
@luka-nextcloud luka-nextcloud assigned mejo- and unassigned mejo- Jan 18, 2022
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

This is not a big difference but I would prefer passing the timestamp directly to moment instead of calculating the milliseconds and building a Date object.

What do you think?

@@ -142,6 +146,9 @@ export default {
subtitle() {
return t('deck', 'Modified') + ': ' + this.relativeDate(this.currentCard.lastModified * 1000) + ' ' + t('deck', 'Created') + ': ' + this.relativeDate(this.currentCard.createdAt * 1000)
},
subtitleTooltip() {
return t('deck', 'Modified') + ': ' + this.formatDate(new Date(this.currentCard.lastModified * 1000)) + '\n' + t('deck', 'Created') + ': ' + this.formatDate(new Date(this.currentCard.createdAt * 1000))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return t('deck', 'Modified') + ': ' + this.formatDate(new Date(this.currentCard.lastModified * 1000)) + '\n' + t('deck', 'Created') + ': ' + this.formatDate(new Date(this.currentCard.createdAt * 1000))
return t('deck', 'Modified') + ': ' + this.formatDate(this.currentCard.lastModified) + '\n' + t('deck', 'Created') + ': ' + this.formatDate(this.currentCard.createdAt)

Comment on lines 199 to 200
formatDate(date) {
return moment(date).locale(this.locale).format('LLLL')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
formatDate(date) {
return moment(date).locale(this.locale).format('LLLL')
formatDate(timestamp) {
return moment.unix(timestamp).locale(this.locale).format('LLLL')

Signed-off-by: Luka Trovic <luka@nextcloud.com>
@luka-nextcloud luka-nextcloud force-pushed the feature/timestamps-on-created-and-modified-at-values branch from 25de24d to 054c5aa Compare January 20, 2022 08:23
@julien-nc julien-nc merged commit 51dffda into master Jan 20, 2022
@delete-merged-branch delete-merged-branch bot deleted the feature/timestamps-on-created-and-modified-at-values branch January 20, 2022 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timestamps on Created and Modified At Values
4 participants