Skip to content

Commit 6b4f7bd

Browse files
fix(dashboards): address review on chart annotations and embed refresh
1 parent b42e3f3 commit 6b4f7bd

9 files changed

Lines changed: 81 additions & 35 deletions

File tree

‎apps/sim/components/charts/echarts-view.tsx‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,14 @@ export function EChartsView({
5757
controllerRef.current = controller ?? null
5858
const next: { option: Record<string, unknown>; annotations?: ChartAnnotations } =
5959
JSON.parse(nextOption)
60-
const parsed = applyChartTooltipDefaults(
61-
next.annotations
62-
? applyChartAnnotations(
63-
next.option,
64-
next.annotations,
65-
readChartTonePalette(chart.getDom())
66-
)
67-
: next.option
68-
)
69-
chart.setOption(controller ? controller.prepareOption(parsed) : parsed, { notMerge: true })
60+
const parsed = applyChartTooltipDefaults(next.option)
61+
// Annotate after the bar helpers read the option: the label series would read as a non-bar chart.
62+
const rendered = next.annotations
63+
? applyChartAnnotations(parsed, next.annotations, readChartTonePalette(chart.getDom()))
64+
: parsed
65+
chart.setOption(controller ? controller.prepareOption(rendered) : rendered, {
66+
notMerge: true,
67+
})
7068
controller?.afterUpdate()
7169
rowHighlightRef.current?.()
7270
rowHighlightRef.current = installBarRowHighlight(chart, parsed)

‎apps/sim/components/dashboards/dashboard-embed.tsx‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { type ReactNode, useId, useMemo, useState } from 'react'
3+
import { type ReactNode, useEffect, useId, useMemo, useRef, useState } from 'react'
44
import { cn, Tooltip } from '@sim/emcn'
55
import { useParams } from 'next/navigation'
66
import { DashboardFeatureGate } from '@/components/dashboards/dashboard-feature-gate'
@@ -81,7 +81,9 @@ function LiveDashboardEmbed({ source, workspaceId }: LiveDashboardEmbedProps) {
8181
}
8282

8383
function EmbedView({ spec, workspaceId }: EmbedViewProps) {
84+
const rootRef = useRef<HTMLDivElement>(null)
8485
const embedId = useId()
86+
const [inView, setInView] = useState(false)
8587
const [state, setState] = useState<DashboardTimeState>({
8688
range: null,
8789
from: null,
@@ -94,16 +96,26 @@ function EmbedView({ spec, workspaceId }: EmbedViewProps) {
9496
time: spec.time,
9597
workspaceId,
9698
tableIds: dashboardTableIds(spec.blocks, spec.source),
97-
live: true,
99+
live: inView,
98100
})
101+
useEffect(() => {
102+
const root = rootRef.current
103+
if (!root) return
104+
const observer = new IntersectionObserver(([entry]) => setInView(entry.isIntersecting))
105+
observer.observe(root)
106+
return () => observer.disconnect()
107+
}, [])
99108
const zoomed = state.range !== null
100109
const { period } = time.controls
101110
const caption =
102111
period === 'custom'
103112
? dashboardRangeText(time.range, time.interactions.timeZone)
104113
: DASHBOARD_RANGE_LABELS[period]
105114
return (
106-
<div className='@container/dashboard relative flex flex-col gap-3 pt-8 font-season'>
115+
<div
116+
ref={rootRef}
117+
className='@container/dashboard relative flex flex-col gap-3 pt-8 font-season'
118+
>
107119
{spec.title && (
108120
<p className='pr-40 font-medium text-[var(--text-primary)] text-base'>{spec.title}</p>
109121
)}

‎apps/sim/components/dashboards/dashboard-panel.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { useDashboardInteractions } from '@/components/dashboards/dashboard-inte
1717
import type { QueryTableAnalyticsResponse } from '@/lib/api/contracts/table-analytics'
1818
import type { ChartAnnotations, ChartHighlight } from '@/lib/charts/annotations'
1919
import { buildChartRenderOption, horizontalBarChartHeight } from '@/lib/charts/option'
20-
import { isTimeSeriesOption } from '@/lib/charts/time-series'
20+
import { isTimeSeriesOption } from '@/lib/charts/spec'
2121
import {
2222
type DashboardDataBlock,
2323
type DashboardSource,

‎apps/sim/components/dashboards/use-dashboard-time.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useEffect, useRef, useState } from 'react'
3+
import { useEffect, useEffectEvent, useRef, useState } from 'react'
44
import { getErrorMessage } from '@sim/utils/errors'
55
import { toRecord } from '@sim/utils/object'
66
import { useIsFetching, useQueryClient } from '@tanstack/react-query'
@@ -32,7 +32,10 @@ interface UseDashboardTimeProps {
3232
time: DashboardTime | undefined
3333
workspaceId: string
3434
tableIds: ReadonlySet<string>
35-
/** Advances relative ranges to the present every minute, so the view keeps up with new rows. */
35+
/**
36+
* Refreshes every minute while true and the page is visible: relative ranges advance to the
37+
* present, fixed and zoomed ranges refetch in place.
38+
*/
3639
live?: boolean
3740
}
3841

@@ -79,19 +82,20 @@ export function useDashboardTime({
7982
}
8083
}
8184

82-
const relative = period !== 'custom'
85+
const tick = useEffectEvent(() => {
86+
if (document.visibilityState !== 'visible') return
87+
if (period === 'custom') void queryClient.invalidateQueries(queryFilter)
88+
else setNow(Date.now())
89+
})
8390
useEffect(() => {
84-
if (!live || !relative) return
85-
const tick = () => {
86-
if (document.visibilityState === 'visible') setNow(Date.now())
87-
}
91+
if (!live) return
8892
const interval = setInterval(tick, LIVE_TICK_MS)
8993
document.addEventListener('visibilitychange', tick)
9094
return () => {
9195
clearInterval(interval)
9296
document.removeEventListener('visibilitychange', tick)
9397
}
94-
}, [live, relative])
98+
}, [live])
9599

96100
const onZoom = (selected: DashboardTimeRange) => {
97101
setInputError(null)

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ function verticalTop(option: Record<string, unknown>): 'start' | 'end' {
4848
return firstAxis(option.yAxis).inverse === true ? 'start' : 'end'
4949
}
5050

51+
/** A chart annotations can draw on: a first series with no hand-written marks to collide with. */
52+
export function assertAnnotatable(option: Record<string, unknown>): void {
53+
const series = Array.isArray(option.series) ? option.series : [option.series]
54+
if (series[0] === undefined) throw new Error('Highlights and thresholds require a series')
55+
const first = toRecord(series[0])
56+
if (first.markArea !== undefined || first.markLine !== undefined)
57+
throw new Error('Use highlights and thresholds instead of markArea or markLine on the series')
58+
}
59+
5160
/**
5261
* Id of the empty series that carries annotation labels. ECharts draws a mark's label at the mark's
5362
* depth, so the marks sit under the data and their labels ride on this series above it.
@@ -77,11 +86,9 @@ export function applyChartAnnotations(
7786
const highlights = annotations.highlights ?? []
7887
const thresholds = annotations.thresholds ?? []
7988
if (highlights.length === 0 && thresholds.length === 0) return option
89+
assertAnnotatable(option)
8090
const series = Array.isArray(option.series) ? option.series : [option.series]
81-
if (series[0] === undefined) throw new Error('Highlights and thresholds require a series')
8291
const first = toRecord(series[0])
83-
if (first.markArea !== undefined || first.markLine !== undefined)
84-
throw new Error('Use highlights and thresholds instead of markArea or markLine on the series')
8592

8693
const bands: Array<AnnotationMark & { from: string; to: string }> = []
8794
const lines: Array<AnnotationMark & { coord: Record<string, string | number> }> = []

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import { getErrorMessage } from '@sim/utils/errors'
8+
import { toRecord } from '@sim/utils/object'
89
import { getColumnId } from '@/lib/table/column-keys'
910
import type { ColumnDefinition } from '@/lib/table/types'
1011

@@ -270,3 +271,9 @@ export function shapeTableRows(
270271
}
271272
return out
272273
}
274+
275+
/** Cartesian charts with one horizontal time axis share dashboard interactions. */
276+
export function isTimeSeriesOption(option: Record<string, unknown>): boolean {
277+
const axes = Array.isArray(option.xAxis) ? option.xAxis : [option.xAxis]
278+
return axes.length === 1 && toRecord(axes[0]).type === 'time'
279+
}

‎apps/sim/lib/charts/time-series.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,6 @@ export interface TimeSeriesInteractionOptions {
3030
onZoom?: (range: DashboardTimeRange) => void
3131
}
3232

33-
/** Cartesian charts with one horizontal time axis share dashboard interactions. */
34-
export function isTimeSeriesOption(option: Record<string, unknown>): boolean {
35-
const axes = Array.isArray(option.xAxis) ? option.xAxis : [option.xAxis]
36-
return axes.length === 1 && toRecord(axes[0]).type === 'time'
37-
}
38-
3933
/** Use ECharts' resolved encodings and colors, including transformed datasets. */
4034
export function readTimeSeriesTooltip(
4135
params: unknown,

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,21 @@ describe('highlights and thresholds', () => {
237237
).toBe('highlights.0.tone: Invalid option: expected one of "neutral"|"error"|"info"')
238238
})
239239

240+
it('rejects highlights or thresholds on a chart with hand-written marks', () => {
241+
const marked =
242+
'blocks:\n - chart: Weekly\n source: {groupBy: [createdAt], bucket: week, aggregate: {n: {op: count}}}\n option: {xAxis: {type: time}, yAxis: {type: value}, series: [{type: line, markLine: {data: []}}]}\n'
243+
const message = 'Use highlights and thresholds instead of markArea or markLine on the series'
244+
expect(
245+
parseDashboardEmbed(
246+
`source: {tableId: tbl_1}\nhighlights: [{at: 2026-09-10T00:00:00Z}]\n${marked}`
247+
).error
248+
).toBe(message)
249+
expect(
250+
parseDashboardEmbed(`source: {tableId: tbl_1}\n${marked} thresholds: [{value: 5}]\n`).error
251+
).toBe(message)
252+
expect(parseDashboardEmbed(`source: {tableId: tbl_1}\n${marked}`).error).toBeUndefined()
253+
})
254+
240255
it('accepts thresholds on a chart with a value axis and rejects them without one', () => {
241256
expect(
242257
parseDashboardEmbed(

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ import { omit } from '@sim/utils/object'
33
import { JSON_SCHEMA, load } from 'js-yaml'
44
import { z } from 'zod'
55
import {
6+
assertAnnotatable,
67
CHART_TONES,
78
type ChartHighlight,
89
type ChartThreshold,
910
valueAxisKey,
1011
} from '@/lib/charts/annotations'
11-
import { parseChartSpec } from '@/lib/charts/spec'
12+
import { isTimeSeriesOption, parseChartSpec } from '@/lib/charts/spec'
1213
import {
1314
type DashboardTimeRange,
1415
parseDashboardCustomRange,
@@ -307,7 +308,8 @@ function loadBoundedYaml(content: string, maxBytes: number, tooLarge: string): u
307308
function validateDashboardBlocks(
308309
blocks: DashboardBlock[],
309310
defaults: DashboardSource | undefined,
310-
maxBlocks: number
311+
maxBlocks: number,
312+
highlights: ChartHighlight[] | undefined
311313
): void {
312314
let count = 0
313315
const visit = (children: DashboardBlock[], depth: number): void => {
@@ -339,6 +341,8 @@ function validateDashboardBlocks(
339341
if (!chart.spec) throw new Error(chart.error)
340342
block.option = chart.spec.option
341343
if (block.thresholds) valueAxisKey(block.option)
344+
if (block.thresholds || (highlights && isTimeSeriesOption(block.option)))
345+
assertAnnotatable(block.option)
342346
}
343347
}
344348
}
@@ -353,7 +357,7 @@ export function parseDashboardSpec(content: string): ParseResult<DashboardSpec>
353357
const parsed = dashboardSchema.safeParse(raw)
354358
if (!parsed.success)
355359
throw new Error(describeSchemaIssues(parsed.error.issues, BLOCK_KINDS).join('\n'))
356-
validateDashboardBlocks(parsed.data.blocks, parsed.data.source, 48)
360+
validateDashboardBlocks(parsed.data.blocks, parsed.data.source, 48, parsed.data.highlights)
357361
return { spec: parsed.data }
358362
} catch (error) {
359363
return { error: getErrorMessage(error, 'Invalid dashboard') }
@@ -371,7 +375,12 @@ export function parseDashboardEmbed(content: string): ParseResult<DashboardEmbed
371375
const parsed = dashboardEmbedSchema.safeParse(raw)
372376
if (!parsed.success)
373377
throw new Error(describeSchemaIssues(parsed.error.issues, EMBED_BLOCK_KINDS).join('\n'))
374-
validateDashboardBlocks(parsed.data.blocks, parsed.data.source, MAX_DASHBOARD_EMBED_BLOCKS)
378+
validateDashboardBlocks(
379+
parsed.data.blocks,
380+
parsed.data.source,
381+
MAX_DASHBOARD_EMBED_BLOCKS,
382+
parsed.data.highlights
383+
)
375384
return { spec: parsed.data }
376385
} catch (error) {
377386
return { error: getErrorMessage(error, 'Invalid dashboard embed') }

0 commit comments

Comments
 (0)