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

traQ上のメッセージの投稿・編集日時に年月日を適宜省略しつつ付け足す #4338

Merged
merged 8 commits into from
Jul 22, 2024
Merged

traQ上のメッセージの投稿・編集日時に年月日を適宜省略しつつ付け足す #4338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c9ae32b
traQ上のメッセージの投稿・編集日時に年月日を適宜省略しつつ付け足す
kitsne241 Jul 1, 2024
8e949db
コードのミスを修正
kitsne241 Jul 1, 2024
6c40343
テストケースを新しい実装に対応させた
kitsne241 Jul 4, 2024
0fb8434
テストケースの日付設定のミスを修正
kitsne241 Jul 4, 2024
1704661
faketimersを用いてtestを書き直しました
kitsne241 Jul 4, 2024
a35b1b0
フォーマットしました
kitsne241 Jul 4, 2024
292f68a
常にupdatedAtを使用・yesterdayの再定義の解消
kitsne241 Jul 7, 2024
3480347
フォーマットの場合分けにgetFullDayStringを不使用
kitsne241 Jul 7, 2024
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
27 changes: 22 additions & 5 deletions src/lib/basic/date.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,29 @@ export const getDateRepresentationWithoutSameDate = (
}

export const getDisplayDate = (createdAt: string, updatedAt: string) => {
const createdDate = new Date(createdAt)
if (createdAt === updatedAt) {
return getTimeString(createdDate)
const displayDate = new Date(updatedAt)
const today = new Date()
const timeString = getTimeString(displayDate)
const yesterday = new Date(today.getTime() - 1000 * 60 * 60 * 24)

if (
displayDate.getFullYear() === today.getFullYear() &&
displayDate.getMonth() === today.getMonth() &&
displayDate.getDate() === today.getDate()
) {
return '今日' + ' ' + timeString
}
if (
displayDate.getFullYear() === yesterday.getFullYear() &&
displayDate.getMonth() === yesterday.getMonth() &&
displayDate.getDate() === yesterday.getDate()
) {
return '昨日' + ' ' + timeString
}
if (displayDate.getFullYear() === today.getFullYear()) {
Copy link
Member

Choose a reason for hiding this comment

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

ここから上は年が同じである条件はおなじなのでネストは増えるけど条件をまとめられそうです

Copy link
Contributor Author

Choose a reason for hiding this comment

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

文意をうまく汲み取れていないかもですが、こういうことですか…？

if (
  displayDate.getFullYear() === yesterday.getFullYear() &&
  displayDate.getMonth() === yesterday.getMonth() &&
  displayDate.getDate() === yesterday.getDate()
) {
  return '昨日' + ' ' + timeString
}
if (displayDate.getFullYear() === today.getFullYear()) {
  if (
    displayDate.getMonth() === today.getMonth() &&
    displayDate.getDate() === today.getDate()
  ) {
    return '今日' + ' ' + timeString
  }
  return getDayString(displayDate) + ' ' + timeString
}
return getFullDayString(displayDate) + ' ' + timeString

大晦日のメッセージを元日に閲覧する時には『20XX/12/31』ではなく『昨日』と表示されてほしいので、年が同じである条件をまとめるならばそれより先に『昨日』の判定を書くことになります。

しかし個人的には、メッセージの投稿日時が
『今日』かどうか →『昨日』かどうか →『今年』かどうか
と順に遡りつつ判定していく構造を保った方が整理された実装に見えるような気がします…。どうでしょうか？

Copy link
Member

Choose a reason for hiding this comment

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

↑のコードの想定でした
todayとyesterday混同しててもう少し単純なコードになると勘違いしてました🙏

それを踏まえても個人的にはtodayの比較とyesterdayの比較が交互に来るよりもyesterdayの比較→todayの比較の順に比較できる↑の実装が見やすいと感じますが、さっきの文字列で比較しないで欲しいという指摘ほど強い思いはないので任せます

Copy link
Member

Choose a reason for hiding this comment

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

や、共通化のことだけを考えてたんですが拡張性が低くなるのでやっぱり現在に近い順に記述するままでいいです🙇🙇🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

了解です。レビューありがとうございます！

return getDayString(displayDate) + ' ' + timeString
} else {
const updatedDate = new Date(updatedAt)
return getDateRepresentationWithoutSameDate(updatedDate, createdDate)
return getFullDayString(displayDate) + ' ' + timeString
}
}

Expand Down
35 changes: 25 additions & 10 deletions tests/unit/lib/basic/date.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,33 @@ describe('getDateRepresentationWithoutSameDate', () => {
})

describe('getDisplayDate', () => {
const dateISO = '2001-04-04T05:20:34'

it('should get time string when not modified', () => {
expect(getDisplayDate(dateISO, dateISO)).toBe('05:20')
beforeEach(() => {
vi.useFakeTimers()
})
it('should get time string when same date', () => {
const dateISO2 = '2001-04-04T08:25:34'
expect(getDisplayDate(dateISO, dateISO2)).toBe('08:25')
afterEach(() => {
vi.useRealTimers()
})
it('should get date string when not same date', () => {
const dateISO2 = '2001-06-04T08:25:34'
expect(getDisplayDate(dateISO, dateISO2)).toBe('06/04 08:25')

const createdDateISO = '2010-04-01T12:34:56'
const updatedDateISO = '2010-05-02T14:28:57'

it('should say 今日 when updated today', () => {
vi.setSystemTime('2010-05-02T15:00:00')
expect(getDisplayDate(createdDateISO, updatedDateISO)).toBe('今日 14:28')
})
it('should say 昨日 when updated yesterday', () => {
vi.setSystemTime('2010-05-03T15:00:00')
expect(getDisplayDate(createdDateISO, updatedDateISO)).toBe('昨日 14:28')
})
it('should get MM/DD when updated in the same year', () => {
vi.setSystemTime('2010-07-07T15:00:00')
expect(getDisplayDate(createdDateISO, updatedDateISO)).toBe('05/02 14:28')
})
it('should get YYYY/MM/DD when updated before last year', () => {
vi.setSystemTime('2015-10-10T15:00:00')
expect(getDisplayDate(createdDateISO, updatedDateISO)).toBe(
'2010/05/02 14:28'
)
})
})

Expand Down
Loading