Skip to content

Commit ec1a997

Browse files
TheodoreSpeaksTheodoreSpeaks
authored
fix(dashboards): keep DST fall-back range endpoints and resolve threshold axis from first series (#8541)
* fix(dashboards): keep DST fall-back range endpoints and resolve threshold axis from first series * fix(dashboards): reject invalid first-series axis refs and show exclusive end for sub-minute ranges * fix(dashboards): show inclusive range end in exact caption and cover id/x-axis threshold lookups * fix(charts): resolve threshold axis by index before id, matching ECharts
1 parent efca99c commit ec1a997

4 files changed

Lines changed: 163 additions & 6 deletions

File tree

‎apps/sim/lib/charts/annotations.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,98 @@ describe('chart annotations', () => {
116116
)
117117
).toThrow('Use highlights and thresholds instead of markArea or markLine on the series')
118118
})
119+
120+
it('measures thresholds on the axes the first series is plotted on', () => {
121+
const option = applyChartAnnotations(
122+
{
123+
xAxis: { type: 'time' },
124+
yAxis: [{ type: 'category' }, { id: 'latency', type: 'value' }],
125+
series: [{ type: 'line', yAxisIndex: 1 }],
126+
},
127+
{ thresholds: [{ value: 5 }] },
128+
palette
129+
)
130+
expect((option.series as Series[])[0].markLine).toMatchObject({ data: [{ yAxis: 5 }] })
131+
expect(() =>
132+
applyChartAnnotations(
133+
{
134+
xAxis: { type: 'time' },
135+
yAxis: [{ type: 'value' }, { id: 'stage', type: 'category' }],
136+
series: [{ type: 'line', yAxisId: 'stage' }],
137+
},
138+
{ thresholds: [{ value: 5 }] },
139+
palette
140+
)
141+
).toThrow('Thresholds require a value axis')
142+
})
143+
144+
it('resolves a value axis selected by id, including a value x-axis on horizontal bars', () => {
145+
const byId = applyChartAnnotations(
146+
{
147+
xAxis: { type: 'time' },
148+
yAxis: [{ type: 'category' }, { id: 'latency', type: 'value' }],
149+
series: [{ type: 'line', yAxisId: 'latency' }],
150+
},
151+
{ thresholds: [{ value: 5 }] },
152+
palette
153+
)
154+
expect((byId.series as Series[])[0].markLine).toMatchObject({ data: [{ yAxis: 5 }] })
155+
const horizontal = applyChartAnnotations(
156+
{
157+
xAxis: [{ type: 'category' }, { id: 'count', type: 'value' }],
158+
yAxis: [{ type: 'value' }, { id: 'stage', type: 'category', inverse: true }],
159+
series: [{ type: 'bar', xAxisId: 'count', yAxisIndex: 1 }],
160+
},
161+
{ thresholds: [{ value: 5, label: 'Limit' }] },
162+
palette
163+
)
164+
const [first, labels] = horizontal.series as Series[]
165+
expect(first.markLine).toMatchObject({ data: [{ xAxis: 5 }] })
166+
expect(labels.markLine).toMatchObject({ data: [{ xAxis: 5, label: { position: 'start' } }] })
167+
})
168+
169+
it('prefers the axis index over the axis id, as ECharts does', () => {
170+
const option = applyChartAnnotations(
171+
{
172+
xAxis: { type: 'time' },
173+
yAxis: [
174+
{ id: 'stage', type: 'category' },
175+
{ id: 7, type: 'value' },
176+
],
177+
series: [{ type: 'line', yAxisIndex: 1, yAxisId: 'stage' }],
178+
},
179+
{ thresholds: [{ value: 5 }] },
180+
palette
181+
)
182+
expect((option.series as Series[])[0].markLine).toMatchObject({ data: [{ yAxis: 5 }] })
183+
expect(() =>
184+
applyChartAnnotations(
185+
{ ...option, series: [{ type: 'line', yAxisId: '7' }] },
186+
{ thresholds: [{ value: 5 }] },
187+
palette
188+
)
189+
).not.toThrow()
190+
})
191+
192+
it('rejects a first series that references an axis the chart does not define', () => {
193+
for (const reference of [{ yAxisIndex: 1 }, { yAxisIndex: -1 }, { yAxisIndex: 0.5 }])
194+
expect(() =>
195+
applyChartAnnotations(
196+
{
197+
xAxis: { type: 'time' },
198+
yAxis: { type: 'value' },
199+
series: [{ type: 'line', ...reference }],
200+
},
201+
{ thresholds: [{ value: 5 }] },
202+
palette
203+
)
204+
).toThrow('The first series references a missing yAxis')
205+
expect(() =>
206+
applyChartAnnotations(
207+
{ xAxis: { type: 'time' }, yAxis: [], series: [{ type: 'line' }] },
208+
{ thresholds: [{ value: 5 }] },
209+
palette
210+
)
211+
).toThrow('The first series references a missing yAxis')
212+
})
119213
})

‎apps/sim/lib/charts/annotations.ts‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,43 @@ export interface ChartAnnotations {
2727

2828
const BAND_OPACITY = 0.08
2929

30-
function firstAxis(axis: unknown): Record<string, unknown> {
31-
return toRecord(Array.isArray(axis) ? axis[0] : axis)
30+
function firstSeries(option: Record<string, unknown>): Record<string, unknown> {
31+
return toRecord(Array.isArray(option.series) ? option.series[0] : option.series)
3232
}
3333

34-
/** The axis a threshold is measured on; ECharts defaults an unspecified yAxis to a value axis. */
34+
/**
35+
* The axis the first series is plotted on. Like ECharts, `*AxisIndex` wins over `*AxisId`, and ids
36+
* match across string and number.
37+
*/
38+
function seriesAxis(
39+
option: Record<string, unknown>,
40+
key: 'xAxis' | 'yAxis'
41+
): Record<string, unknown> {
42+
const axes = Array.isArray(option[key]) ? (option[key] as unknown[]) : [option[key]]
43+
const series = firstSeries(option)
44+
const id = series[`${key}Id`]
45+
if (series[`${key}Index`] === undefined && id !== undefined) {
46+
const axis = axes.find((candidate) => String(toRecord(candidate).id) === String(id))
47+
if (axis === undefined) throw new Error(`The first series references a missing ${key} "${id}"`)
48+
return toRecord(axis)
49+
}
50+
const index = series[`${key}Index`] ?? 0
51+
if (option[key] === undefined && index === 0) return {}
52+
if (!Number.isInteger(index) || axes[index as number] === undefined)
53+
throw new Error(`The first series references a missing ${key} at index ${String(index)}`)
54+
return toRecord(axes[index as number])
55+
}
56+
57+
/**
58+
* The axis a threshold is measured on, among the axes the first series is plotted on; ECharts
59+
* defaults an unspecified yAxis to a value axis.
60+
*/
3561
export function valueAxisKey(option: Record<string, unknown>): 'xAxis' | 'yAxis' {
3662
if (option.xAxis === undefined && option.yAxis === undefined)
3763
throw new Error('Thresholds require a value axis')
38-
const y = firstAxis(option.yAxis)
64+
const y = seriesAxis(option, 'yAxis')
3965
if (y.type === undefined || y.type === 'value' || y.type === 'log') return 'yAxis'
40-
const x = firstAxis(option.xAxis)
66+
const x = seriesAxis(option, 'xAxis')
4167
if (x.type === 'value' || x.type === 'log') return 'xAxis'
4268
throw new Error('Thresholds require a value axis')
4369
}
@@ -47,7 +73,7 @@ export function valueAxisKey(option: Record<string, unknown>): 'xAxis' | 'yAxis'
4773
* y axis is inverted (as horizontal bar charts usually are).
4874
*/
4975
function verticalTop(option: Record<string, unknown>): 'start' | 'end' {
50-
return firstAxis(option.yAxis).inverse === true ? 'start' : 'end'
76+
return seriesAxis(option, 'yAxis').inverse === true ? 'start' : 'end'
5177
}
5278

5379
/** A chart annotations can draw on: a first series with no hand-written marks to collide with. */

‎apps/sim/lib/dashboards/time.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
22
import {
33
dashboardAxisFormatter,
44
dashboardRangeFromCalendar,
5+
dashboardRangeText,
56
dashboardTimeLabel,
67
dashboardZoomRange,
78
parseDashboardCustomRange,
@@ -47,4 +48,21 @@ describe('dashboard time interactions', () => {
4748
expect(dashboardTimeLabel(stamp, 'America/Los_Angeles')).toContain('PDT')
4849
expect(dashboardTimeLabel('2026-12-20T02:30:00Z', 'America/Los_Angeles')).toContain('PST')
4950
})
51+
52+
it('keeps both ends of a range that repeats the same wall-clock minute across a DST fall-back', () => {
53+
const text = dashboardRangeText(
54+
{ from: '2026-11-01T05:30:00.000Z', to: '2026-11-01T06:31:00.000Z' },
55+
'America/New_York'
56+
)
57+
expect(text).toBe('Nov 1, 01:30:00 EDT – Nov 1, 01:30:59 EST')
58+
})
59+
60+
it('shows distinct endpoints for a sub-minute zoom range', () => {
61+
expect(
62+
dashboardRangeText(
63+
{ from: '2026-09-20T14:30:00.000Z', to: '2026-09-20T14:30:01.000Z' },
64+
'UTC'
65+
)
66+
).toBe('Sep 20, 14:30:00.000 UTC – Sep 20, 14:30:00.999 UTC')
67+
})
5068
})

‎apps/sim/lib/dashboards/time.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ export function dashboardRangeText(range: DashboardTimeRange, timeZone: string):
8989
const to = new Date(Date.parse(range.to) - 1)
9090
const fromLocal = zonedWallClock(from, timeZone)
9191
const toLocal = zonedWallClock(to, timeZone)
92+
if (fromLocal === toLocal) {
93+
// Same wall-clock minute (e.g. a DST fall-back repeat): formatRange would collapse both ends,
94+
// so show each inclusive end exactly, down to milliseconds if seconds still collide.
95+
const exact = (fractionalSecondDigits?: 3) =>
96+
new Intl.DateTimeFormat('en-US', {
97+
timeZone,
98+
month: 'short',
99+
day: 'numeric',
100+
hour: '2-digit',
101+
minute: '2-digit',
102+
second: '2-digit',
103+
fractionalSecondDigits,
104+
hourCycle: 'h23',
105+
timeZoneName: 'short',
106+
})
107+
const seconds = exact()
108+
const format = seconds.format(from) === seconds.format(to) ? exact(3) : seconds
109+
return `${format.format(from)} – ${format.format(to)}`
110+
}
92111
const sameDay = fromLocal.slice(0, 10) === toLocal.slice(0, 10)
93112
return new Intl.DateTimeFormat('en-US', {
94113
timeZone,

0 commit comments

Comments
 (0)