Skip to content

Commit 2a1141f

Browse files
authored
Stop repeating annotation date in tooltip when unambiguous (#6518)
* Stop repeating annotation date in tooltip when unambiguous * Add test that asserts the two ways that naive local time can be provided in note creation * Make annotations unaware of graph time label format (no T separator) * Fix missing unquote in controller test * Remove comment
1 parent b8484ea commit 2a1141f

10 files changed

Lines changed: 205 additions & 76 deletions

File tree

assets/js/dashboard/annotations/annotation-list-items.tsx

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,31 @@ const VerticalBar = () => (
2121
)
2222

2323
export const AnnotationItemRow = ({ children }: { children: ReactNode }) => (
24-
<div className="group flex flex-row gap-x-2">
24+
<div className="relative group flex flex-row gap-x-2">
2525
<VerticalBar />
2626
{children}
2727
</div>
2828
)
2929

3030
export const AnnotationAuthorshipLine = ({
31-
annotation
31+
annotation,
32+
showDateLabel
3233
}: {
3334
annotation: Annotation
35+
showDateLabel: boolean
3436
}) => (
35-
<div className="flex items-baseline text-xs text-gray-300 pr-8">
37+
<div
38+
data-testid="annotation-attribution"
39+
className="flex items-baseline text-xs text-gray-300 pr-8"
40+
>
3641
<span className="truncate min-w-0">
3742
{getAnnotationAuthorship(annotation)}
3843
</span>
39-
<span className="whitespace-nowrap shrink-0">
40-
{` • ${getAttributionDateLabel(annotation)}`}
41-
</span>
44+
{showDateLabel && (
45+
<span className="whitespace-nowrap shrink-0">
46+
&nbsp;{`• ${getAttributionDateLabel(annotation)}`}
47+
</span>
48+
)}
4249
</div>
4350
)
4451

assets/js/dashboard/annotations/annotations.test.ts

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import {
55
canEditAnnotation,
66
getAnnotationAuthorship,
77
getAnnotationGranularity,
8-
getAnnotationTimeLabel,
9-
groupAnnotationsByTimeLabel
8+
getAnnotationDatetimeGroup,
9+
groupAnnotationsByDatetime,
10+
allAnnotationsAreFromThisExactDatetime
1011
} from './annotations'
1112
import { Interval } from '../stats/graph/intervals'
1213
import { Role, UserContextValue } from '../user-context'
@@ -199,7 +200,7 @@ describe(`${getAnnotationGranularity.name}`, () => {
199200
})
200201
})
201202

202-
describe(`${getAnnotationTimeLabel.name}`, () => {
203+
describe(`${getAnnotationDatetimeGroup.name}`, () => {
203204
// 2025-02-26 is a Wednesday
204205
const dateAnnotation = {
205206
datetime: '2025-02-26',
@@ -215,7 +216,9 @@ describe(`${getAnnotationTimeLabel.name}`, () => {
215216
])(
216217
`date-granularity annotation on ${dateAnnotation.datetime} bucketed to %s yields %s`,
217218
(interval, expected) => {
218-
expect(getAnnotationTimeLabel(dateAnnotation, interval)).toBe(expected)
219+
expect(getAnnotationDatetimeGroup(dateAnnotation, interval)).toBe(
220+
expected
221+
)
219222
}
220223
)
221224

@@ -227,17 +230,19 @@ describe(`${getAnnotationTimeLabel.name}`, () => {
227230
[Interval.month, '2025-02-01'],
228231
[Interval.week, '2025-02-24'],
229232
[Interval.day, '2025-02-26'],
230-
[Interval.hour, '2025-02-26 10:00:00'],
231-
[Interval.minute, '2025-02-26 10:30:00']
233+
[Interval.hour, '2025-02-26T10:00:00'],
234+
[Interval.minute, '2025-02-26T10:30:00']
232235
])(
233236
`minute granularity annotation with datetime ${minuteAnnotation.datetime} bucketed to %s yields %s`,
234237
(interval, expected) => {
235-
expect(getAnnotationTimeLabel(minuteAnnotation, interval)).toBe(expected)
238+
expect(getAnnotationDatetimeGroup(minuteAnnotation, interval)).toBe(
239+
expected
240+
)
236241
}
237242
)
238243
})
239244

240-
describe(`${groupAnnotationsByTimeLabel.name}`, () => {
245+
describe(`${groupAnnotationsByDatetime.name}`, () => {
241246
const dateGranularity = AnnotationGranularity.date
242247
const annotations = [
243248
{ id: 1, datetime: '2025-02-24 00:00:00', granularity: dateGranularity }, // Mon
@@ -246,7 +251,7 @@ describe(`${groupAnnotationsByTimeLabel.name}`, () => {
246251
]
247252

248253
it('groups annotations by day when the interval is day', () => {
249-
const grouped = groupAnnotationsByTimeLabel(annotations, Interval.day)
254+
const grouped = groupAnnotationsByDatetime(annotations, Interval.day)
250255

251256
expect(Object.keys(grouped).sort()).toEqual([
252257
'2025-02-24',
@@ -259,15 +264,15 @@ describe(`${groupAnnotationsByTimeLabel.name}`, () => {
259264
})
260265

261266
it('collapses annotations from the same week into one bucket', () => {
262-
const grouped = groupAnnotationsByTimeLabel(annotations, Interval.week)
267+
const grouped = groupAnnotationsByDatetime(annotations, Interval.week)
263268

264269
expect(Object.keys(grouped).sort()).toEqual(['2025-02-24', '2025-03-03'])
265270
expect(grouped['2025-02-24']!.map((a) => a.id)).toEqual([1, 2])
266271
expect(grouped['2025-03-03']!.map((a) => a.id)).toEqual([3])
267272
})
268273

269274
it('collapses annotations from the same month into one bucket', () => {
270-
const grouped = groupAnnotationsByTimeLabel(annotations, Interval.month)
275+
const grouped = groupAnnotationsByDatetime(annotations, Interval.month)
271276

272277
expect(Object.keys(grouped).sort()).toEqual(['2025-02-01', '2025-03-01'])
273278
expect(grouped['2025-02-01']!.map((a) => a.id)).toEqual([1, 2])
@@ -281,17 +286,58 @@ describe(`${groupAnnotationsByTimeLabel.name}`, () => {
281286
{ id: 12, datetime: '2025-02-26 10:00:00', granularity: dateGranularity }
282287
]
283288

284-
const grouped = groupAnnotationsByTimeLabel(sameDay, Interval.day)
289+
const grouped = groupAnnotationsByDatetime(sameDay, Interval.day)
285290

286291
expect(grouped['2025-02-26']!.map((a) => a.id)).toEqual([10, 11, 12])
287292
})
288293

289294
it('returns an empty object when there are no annotations', () => {
290295
expect(
291-
groupAnnotationsByTimeLabel(
296+
groupAnnotationsByDatetime(
292297
[] as { datetime: string; granularity: AnnotationGranularity }[],
293298
Interval.day
294299
)
295300
).toEqual({})
296301
})
297302
})
303+
304+
describe(`${allAnnotationsAreFromThisExactDatetime.name}`, () => {
305+
it('returns true when every annotation shares the given datetime', () => {
306+
expect(
307+
allAnnotationsAreFromThisExactDatetime(
308+
[
309+
{ datetime: '2025-02-26T10:30:00' },
310+
{ datetime: '2025-02-26T10:30:00' }
311+
],
312+
'2025-02-26T10:30:00'
313+
)
314+
).toBe(true)
315+
})
316+
317+
it('returns true when every annotation shares the given date', () => {
318+
expect(
319+
allAnnotationsAreFromThisExactDatetime(
320+
[{ datetime: '2026-07-20' }, { datetime: '2026-07-20' }],
321+
'2026-07-20'
322+
)
323+
).toBe(true)
324+
})
325+
326+
it('returns false when some annotation has a different datetime', () => {
327+
expect(
328+
allAnnotationsAreFromThisExactDatetime(
329+
[
330+
{ datetime: '2025-02-26T10:30:00' },
331+
{ datetime: '2025-02-26T10:31:00' }
332+
],
333+
'2025-02-26T10:30:00'
334+
)
335+
).toBe(false)
336+
})
337+
338+
it('returns true when there are no annotations', () => {
339+
expect(
340+
allAnnotationsAreFromThisExactDatetime([], '2025-02-26T10:30:00')
341+
).toBe(true)
342+
})
343+
})

assets/js/dashboard/annotations/annotations.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ export type Annotation = {
3737
note: string
3838

3939
id: number
40-
/** datetime in site timezone, example 2025-02-26 10:00:00 */
40+
/** datetime in site timezone, example 2025-02-26T10:00:00 */
4141
inserted_at: string
42-
/** datetime in site timezone, example 2025-02-26 10:00:00 */
42+
/** datetime in site timezone, example 2025-02-26T10:00:00 */
4343
updated_at: string
4444
} & AnnotationOwnership
4545

@@ -139,7 +139,7 @@ export function canAddAnnotation({
139139
}
140140
}
141141

142-
export const getAnnotationTimeLabel = (
142+
export const getAnnotationDatetimeGroup = (
143143
annotation: Pick<Annotation, 'datetime' | 'granularity'>,
144144
interval: Interval
145145
): string => {
@@ -175,24 +175,24 @@ export const getAnnotationTimeLabel = (
175175
case Interval.hour: {
176176
const [dateYYYYMMDD, timeHHMMSS] = annotation.datetime.split('T')
177177
// floors time to hour
178-
return `${dateYYYYMMDD} ${timeHHMMSS.substring(0, 'HH'.length)}:00:00`
178+
return `${dateYYYYMMDD}T${timeHHMMSS.substring(0, 'HH'.length)}:00:00`
179179
}
180180
case Interval.minute:
181-
return annotation.datetime.split('T').join(' ')
181+
return annotation.datetime
182182
}
183183
}
184184
}
185185
}
186186

187-
export const groupAnnotationsByTimeLabel = <
187+
export const groupAnnotationsByDatetime = <
188188
T extends Pick<Annotation, 'datetime' | 'granularity'>
189189
>(
190190
annotations: T[],
191191
interval: Interval
192192
): Record<string, T[] | undefined> => {
193193
return annotations.reduce<Record<string, T[]>>((acc, annotation) => {
194-
const timeLabel = getAnnotationTimeLabel(annotation, interval)
195-
return { ...acc, [timeLabel]: [...(acc[timeLabel] ?? []), annotation] }
194+
const label = getAnnotationDatetimeGroup(annotation, interval)
195+
return { ...acc, [label]: [...(acc[label] ?? []), annotation] }
196196
}, {})
197197
}
198198

@@ -210,6 +210,11 @@ export const getAnnotationGranularity = (
210210
}
211211
}
212212

213+
export const allAnnotationsAreFromThisExactDatetime = (
214+
annotations: Pick<Annotation, 'datetime'>[],
215+
datetime: string
216+
): boolean => annotations.every((a) => a.datetime === datetime)
217+
213218
export const getApiFormattedPayload = ({
214219
granularity,
215220
datetime,

assets/js/dashboard/annotations/hover-annotations-list.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import React from 'react'
2-
import { Annotation } from './annotations'
2+
import {
3+
Annotation,
4+
allAnnotationsAreFromThisExactDatetime
5+
} from './annotations'
36
import {
47
AnnotationAuthorshipLine,
58
AnnotationItemRow,
@@ -10,20 +13,29 @@ import {
1013
const MAX_PREVIEW = 2
1114

1215
export const HoverAnnotationsList = ({
16+
annotationDatetime,
1317
annotations
1418
}: {
19+
annotationDatetime: string
1520
annotations: Annotation[]
1621
}) => {
1722
const preview = annotations.slice(0, MAX_PREVIEW)
1823
const extra = annotations.length - MAX_PREVIEW
24+
const showDateLabel = !allAnnotationsAreFromThisExactDatetime(
25+
preview,
26+
annotationDatetime
27+
)
1928

2029
return (
2130
<>
2231
<AnnotationsListContainer>
2332
{preview.map((annotation) => (
2433
<AnnotationItemRow key={annotation.id}>
25-
<div className="relative flex flex-col gap-y-px w-full max-w-64">
26-
<AnnotationAuthorshipLine annotation={annotation} />
34+
<div className="flex flex-col gap-y-px w-full max-w-64">
35+
<AnnotationAuthorshipLine
36+
annotation={annotation}
37+
showDateLabel={showDateLabel}
38+
/>
2739
<AnnotationNote note={annotation.note} clamp />
2840
</div>
2941
</AnnotationItemRow>

assets/js/dashboard/annotations/interactive-annotations-list.tsx

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import React, { ReactNode } from 'react'
2-
import { Annotation, canEditAnnotation } from './annotations'
2+
import {
3+
Annotation,
4+
allAnnotationsAreFromThisExactDatetime,
5+
canEditAnnotation
6+
} from './annotations'
37
import {
48
AnnotationAuthorshipLine,
59
AnnotationItemRow,
@@ -17,10 +21,12 @@ const ScrollableArea = (props: { children: ReactNode }) => (
1721
)
1822

1923
export const InteractiveAnnotationsList = ({
24+
annotationDatetime,
2025
annotations,
2126
isTouchDevice,
2227
closeTooltip
2328
}: {
29+
annotationDatetime: string
2430
annotations: Annotation[]
2531
isTouchDevice: boolean
2632
closeTooltip: () => void
@@ -31,6 +37,10 @@ export const InteractiveAnnotationsList = ({
3137
closeTooltip()
3238
setModal({ type: 'update-annotation', annotation })
3339
}
40+
const showDateLabel = !allAnnotationsAreFromThisExactDatetime(
41+
annotations,
42+
annotationDatetime
43+
)
3444

3545
return (
3646
<ScrollableArea>
@@ -39,7 +49,10 @@ export const InteractiveAnnotationsList = ({
3949
const editable = canEditAnnotation({ type: annotation.type, user })
4050
const content = (
4151
<>
42-
<AnnotationAuthorshipLine annotation={annotation} />
52+
<AnnotationAuthorshipLine
53+
annotation={annotation}
54+
showDateLabel={showDateLabel}
55+
/>
4356
<AnnotationNote note={annotation.note} />
4457
{editable && !isTouchDevice && (
4558
<button
@@ -56,13 +69,13 @@ export const InteractiveAnnotationsList = ({
5669
<AnnotationItemRow key={annotation.id}>
5770
{editable && isTouchDevice ? (
5871
<button
59-
className="relative flex flex-col gap-y-px w-full max-w-64 text-left focus:outline-none"
72+
className="flex flex-col gap-y-px w-full max-w-64 text-left focus:outline-none"
6073
onClick={() => openEdit(annotation)}
6174
>
6275
{content}
6376
</button>
6477
) : (
65-
<div className="relative flex flex-col gap-y-px w-full max-w-64">
78+
<div className="flex flex-col gap-y-px w-full max-w-64">
6679
{content}
6780
</div>
6881
)}

assets/js/dashboard/stats/graph/main-graph-data.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import {
22
getChangeInPercentagePoints,
33
getRelativeChange,
4-
getLineSegments
4+
getLineSegments,
5+
normalizeGraphTimeLabel
56
} from './main-graph-data'
67

78
describe(`${getChangeInPercentagePoints.name}`, () => {
@@ -109,3 +110,15 @@ describe(`${getLineSegments.name}`, () => {
109110
])
110111
})
111112
})
113+
114+
describe(`${normalizeGraphTimeLabel.name}`, () => {
115+
it('replaces the space separator with "T" for datetime labels', () => {
116+
expect(normalizeGraphTimeLabel('2026-10-10 10:00:15')).toBe(
117+
'2026-10-10T10:00:15'
118+
)
119+
})
120+
121+
it('leaves date-only labels unchanged', () => {
122+
expect(normalizeGraphTimeLabel('2026-10-10')).toBe('2026-10-10')
123+
})
124+
})

assets/js/dashboard/stats/graph/main-graph-data.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,3 +213,11 @@ type SeriesValue =
213213
isCurrent: boolean
214214
timeLabel: string
215215
}
216+
217+
/**
218+
* This function normalizes graph time labels in the format "2026-10-10 10:00:15"
219+
* to the standard format "2026-10-10T10:00:15". Passes YYYY-MM-DD format date strings
220+
* like "2026-01-01" through unchanged.
221+
*/
222+
export const normalizeGraphTimeLabel = (timeLabel: string) =>
223+
timeLabel.split(' ').join('T')

0 commit comments

Comments
 (0)