diff --git a/packages/app/src/HDXMultiSeriesTableChart.tsx b/packages/app/src/HDXMultiSeriesTableChart.tsx index 78641fe7e9..604e0c3140 100644 --- a/packages/app/src/HDXMultiSeriesTableChart.tsx +++ b/packages/app/src/HDXMultiSeriesTableChart.tsx @@ -307,6 +307,33 @@ export const Table = ({ ); const [wrapLinesEnabled, setWrapLinesEnabled] = useState(false); + // Store the virtual index of the hovered row (not its description string) + // so the label re-derives on every render. If the virtualiser replaces the + // row at that index (scroll re-virtualisation, auto-refetch) the label + // reflects the new row immediately rather than showing stale text. Storing + // the index also makes the safety-net `onMouseLeave` on correct: + // it sets null rather than the prior row's description. See HDX-4405. + const [hoveredVirtualIndex, setHoveredVirtualIndex] = useState( + null, + ); + + // Derive the label from whichever row currently occupies hoveredVirtualIndex. + // Returns null when no row is hovered, the index is out of range, or the + // row's action has no URL (error-toast rows show no hint). + const hoveredRowDescription = useMemo(() => { + if (hoveredVirtualIndex == null) return null; + const virtualRow = items.find(v => v.index === hoveredVirtualIndex); + if (!virtualRow) return null; + const row = rows[virtualRow.index] as TableRow | undefined; + if (!row) return null; + const rowAction = getRowAction ? getRowAction(row.original) : null; + return rowAction?.url != null && rowAction.description + ? rowAction.description + : null; + }, [hoveredVirtualIndex, items, rows, getRowAction]); + + const clearHovered = useCallback(() => setHoveredVirtualIndex(null), []); + const { csvData } = useCsvExport( truncatedData, columns.map(col => ({ @@ -371,65 +398,62 @@ export const Table = ({ ))} - - {paddingTop > 0 && ( - - - - )} - {items.map(virtualRow => { - const row = rows[virtualRow.index] as TableRow; - // Compute the action once per row so the row-level HoverCard - // sees the same description and per-cell renders share the - // memoized result from useOnClickLinkBuilder. - const rowAction = getRowAction ? getRowAction(row.original) : null; - const tr = ( - - {row.getVisibleCells().map(cell => { - return ( - - {flexRender( - cell.column.columnDef.cell, - cell.getContext(), - )} - - ); - })} + {/* Single Tooltip.Floating wrapping the whole so the hint + follows the cursor without being tied to the lifecycle of any + individual virtual row. Per-row Tooltip.Floating instances get + stranded in the Portal when a row unmounts before onMouseLeave + fires (rapid mouse movement in a virtualised list). With this + approach the tooltip state lives on , which never unmounts, + and the label is re-derived from hoveredVirtualIndex each render so + scroll re-virtualisation never shows stale text. See HDX-4405. */} + {hoveredRowDescription} + } + withinPortal + disabled={!hoveredRowDescription} + > + {/* onMouseLeave on is a safety net: if a virtual row + unmounts before its own onMouseLeave fires (rapid cursor + movement or re-virtualisation), leaving the table body still + clears the hovered index. */} + + {paddingTop > 0 && ( + + - ); - // Row-level Tooltip.Floating so the hint follows the cursor - // and anchors near the cell the user is over, not at the row's - // center-top. Tooltip.Floating tracks the cursor via floating-ui - // and stays within the row's bounding box; one tooltip per row - // means no flicker as the cursor moves between cells. - // - // The hint is suppressed when rowAction.url === null because - // the click only fires an error toast on those rows, so showing - // "Open in search" would mislead the user. - if (rowAction && rowAction.url) { + )} + {items.map(virtualRow => { + const row = rows[virtualRow.index] as TableRow; return ( - setHoveredVirtualIndex(virtualRow.index)} + onMouseLeave={clearHovered} > - {tr} - + {row.getVisibleCells().map(cell => { + return ( + + {flexRender( + cell.column.columnDef.cell, + cell.getContext(), + )} + + ); + })} + ); - } - return tr; - })} - {paddingBottom > 0 && ( - - - - )} - + })} + {paddingBottom > 0 && ( + + + + )} + + {isTruncated && (
diff --git a/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx index c63e72941b..8947a731dc 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx +++ b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx @@ -102,6 +102,61 @@ describe('HDXMultiSeriesTableChart ', () => { { timeout: 1000 }, ); }); + + it('hides the hint when the hovered virtual index maps to a no-URL row (HDX-4405)', async () => { + // Regression for the virtualiser race: the hovered can unmount + // before its onMouseLeave fires (rapid movement / data refresh). The + // old per-row Tooltip.Floating was stranded in the Portal; the fix + // stores a virtual index and re-derives the label via useMemo each + // render. When the row at that index no longer has a URL, the tooltip + // hides without a leave event from the (now-gone) element. + // + // We verify the key invariant structurally: hovering index 0 (URL row) + // shows the hint; hovering index 1 (no-URL row) hides it — no + // mouseLeave fires between the two enterevents, simulating the + // cursor jumping over the table faster than leave events dispatch. + const multiRowData = [ + { ServiceName: 'web', Count: 10 }, + { ServiceName: 'api', Count: 5 }, + ]; + const getRowAction = jest.fn((row: { ServiceName: string }) => + row.ServiceName === 'web' + ? { url: '/search?source=src_1&where=', description: 'Search Logs' } + : { url: null, description: '', onClickError: jest.fn() }, + ); + + renderWithMantine( +
{}} + />, + ); + + const webRow = screen.getByText('web').closest('tr')!; + const apiRow = screen.getByText('api').closest('tr')!; + + // Hover the URL row — tooltip must appear + fireEvent.mouseEnter(webRow); + await waitFor(() => { + const hint = screen.getByTestId('row-action-hint'); + const tooltipBox = hint.closest('[style*="display"]'); + expect(tooltipBox?.style.display).toBe('block'); + }); + + // Hover the no-URL row WITHOUT firing mouseLeave on the first row. + // This simulates the cursor jumping faster than leave events dispatch. + fireEvent.mouseEnter(apiRow); + + // The label must derive to null (apiRow has url:null) so tooltip hides. + await waitFor(() => { + const hint = screen.getByTestId('row-action-hint'); + const tooltipBox = hint.closest('[style*="display"]'); + expect(tooltipBox?.style.display).toBe('none'); + }); + }); }); describe('getRowAction failure path', () => { diff --git a/packages/app/src/__tests__/utils.test.ts b/packages/app/src/__tests__/utils.test.ts index bedea9c851..fc986f06e1 100644 --- a/packages/app/src/__tests__/utils.test.ts +++ b/packages/app/src/__tests__/utils.test.ts @@ -1,4 +1,8 @@ -import { NumericUnit, TSource } from '@hyperdx/common-utils/dist/types'; +import { + ColorCondition, + NumericUnit, + TSource, +} from '@hyperdx/common-utils/dist/types'; import { SortingState } from '@tanstack/react-table'; import { act, renderHook } from '@testing-library/react'; @@ -6,6 +10,7 @@ import { MetricsDataType, NumberFormat } from '../types'; import * as utils from '../utils'; import { COLORS, + evaluateColorCondition, formatAttributeClause, formatDurationMs, formatDurationMsCompact, @@ -16,6 +21,7 @@ import { mapKeyBy, orderByStringToSortingState, parseTimestampToMs, + resolveConditionalColor, sortingStateToOrderByString, stripTrailingSlash, useQueryHistory, @@ -1265,3 +1271,298 @@ describe('getColorFromCSSToken', () => { } }); }); + +// ─── evaluateColorCondition ─────────────────────────────────────────────────── + +describe('evaluateColorCondition', () => { + describe('numeric ordered operators', () => { + it('gt: returns true when value > rule.value', () => { + const rule: ColorCondition = { + operator: 'gt', + value: 10, + color: 'chart-1', + }; + expect(evaluateColorCondition(11, rule)).toBe(true); + expect(evaluateColorCondition(10, rule)).toBe(false); + expect(evaluateColorCondition(9, rule)).toBe(false); + }); + + it('gte: returns true when value >= rule.value', () => { + const rule: ColorCondition = { + operator: 'gte', + value: 10, + color: 'chart-1', + }; + expect(evaluateColorCondition(10, rule)).toBe(true); + expect(evaluateColorCondition(11, rule)).toBe(true); + expect(evaluateColorCondition(9, rule)).toBe(false); + }); + + it('lt: returns true when value < rule.value', () => { + const rule: ColorCondition = { + operator: 'lt', + value: 10, + color: 'chart-1', + }; + expect(evaluateColorCondition(9, rule)).toBe(true); + expect(evaluateColorCondition(10, rule)).toBe(false); + }); + + it('lte: returns true when value <= rule.value', () => { + const rule: ColorCondition = { + operator: 'lte', + value: 10, + color: 'chart-1', + }; + expect(evaluateColorCondition(10, rule)).toBe(true); + expect(evaluateColorCondition(9, rule)).toBe(true); + expect(evaluateColorCondition(11, rule)).toBe(false); + }); + + it('numeric operators return false for string values', () => { + const rule: ColorCondition = { + operator: 'gt', + value: 10, + color: 'chart-1', + }; + expect(evaluateColorCondition('15', rule)).toBe(false); + }); + }); + + describe('between operator', () => { + it('returns true when value is within [lo, hi]', () => { + const rule: ColorCondition = { + operator: 'between', + value: [10, 100], + color: 'chart-1', + }; + expect(evaluateColorCondition(50, rule)).toBe(true); + expect(evaluateColorCondition(10, rule)).toBe(true); + expect(evaluateColorCondition(100, rule)).toBe(true); + expect(evaluateColorCondition(9, rule)).toBe(false); + expect(evaluateColorCondition(101, rule)).toBe(false); + }); + + it('handles inverted range (first > second) by normalising to [lo, hi]', () => { + const rule: ColorCondition = { + operator: 'between', + value: [100, 10], + color: 'chart-1', + }; + expect(evaluateColorCondition(50, rule)).toBe(true); + expect(evaluateColorCondition(5, rule)).toBe(false); + }); + + it('returns false for string values', () => { + const rule: ColorCondition = { + operator: 'between', + value: [10, 100], + color: 'chart-1', + }; + expect(evaluateColorCondition('50', rule)).toBe(false); + }); + }); + + describe('eq / neq operators', () => { + it('eq: returns true on strict equality (number)', () => { + const rule: ColorCondition = { + operator: 'eq', + value: 5, + color: 'chart-1', + }; + expect(evaluateColorCondition(5, rule)).toBe(true); + expect(evaluateColorCondition(6, rule)).toBe(false); + }); + + it('eq: returns true on strict equality (string)', () => { + const rule: ColorCondition = { + operator: 'eq', + value: 'CRIT', + color: 'chart-1', + }; + expect(evaluateColorCondition('CRIT', rule)).toBe(true); + expect(evaluateColorCondition('crit', rule)).toBe(false); + }); + + it('eq: cross-type mismatch returns false ("5" vs 5)', () => { + const rule: ColorCondition = { + operator: 'eq', + value: '5', + color: 'chart-1', + }; + expect(evaluateColorCondition(5, rule)).toBe(false); + }); + + it('neq: returns true when value differs', () => { + const rule: ColorCondition = { + operator: 'neq', + value: 0, + color: 'chart-1', + }; + expect(evaluateColorCondition(1, rule)).toBe(true); + expect(evaluateColorCondition(0, rule)).toBe(false); + }); + }); + + describe('string operators', () => { + it('contains: returns true when string includes value', () => { + const rule: ColorCondition = { + operator: 'contains', + value: 'error', + color: 'chart-error', + }; + expect(evaluateColorCondition('fatal error occurred', rule)).toBe(true); + expect(evaluateColorCondition('warning', rule)).toBe(false); + }); + + it('contains: returns false for number values', () => { + const rule: ColorCondition = { + operator: 'contains', + value: 'error', + color: 'chart-error', + }; + expect(evaluateColorCondition(42, rule)).toBe(false); + }); + + it('startsWith: matches prefix', () => { + const rule: ColorCondition = { + operator: 'startsWith', + value: 'ERR', + color: 'chart-error', + }; + expect(evaluateColorCondition('ERR_500', rule)).toBe(true); + expect(evaluateColorCondition('WARN_ERR', rule)).toBe(false); + }); + + it('endsWith: matches suffix', () => { + const rule: ColorCondition = { + operator: 'endsWith', + value: 'CRIT', + color: 'chart-error', + }; + expect(evaluateColorCondition('ALERT_CRIT', rule)).toBe(true); + expect(evaluateColorCondition('CRIT_OK', rule)).toBe(false); + }); + + it('regex: matches valid pattern', () => { + const rule: ColorCondition = { + operator: 'regex', + value: '^err.*', + color: 'chart-error', + }; + expect(evaluateColorCondition('error123', rule)).toBe(true); + expect(evaluateColorCondition('warning', rule)).toBe(false); + }); + + it('regex: bad pattern returns false without throwing', () => { + const rule = { + operator: 'regex' as const, + value: '[invalid', + color: 'chart-error' as const, + }; + expect(() => evaluateColorCondition('test', rule)).not.toThrow(); + expect(evaluateColorCondition('test', rule)).toBe(false); + }); + }); +}); + +// ─── resolveConditionalColor ────────────────────────────────────────────────── + +describe('resolveConditionalColor', () => { + it('returns fallback when rules is undefined', () => { + expect(resolveConditionalColor(50, undefined, 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('returns fallback when rules is empty', () => { + expect(resolveConditionalColor(50, [], 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('returns fallback when value is null', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 0, color: 'chart-warning' }, + ]; + expect(resolveConditionalColor(null, rules, 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('returns fallback when value is undefined', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 0, color: 'chart-warning' }, + ]; + expect(resolveConditionalColor(undefined, rules, 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('returns the matching rule color when one rule matches', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 100, color: 'chart-warning' }, + ]; + expect(resolveConditionalColor(200, rules, 'chart-success')).toBe( + 'chart-warning', + ); + }); + + it('returns the LAST matching rule color (last-match-wins)', () => { + // value 1000: both rules match; last (chart-error) wins + const rules: ColorCondition[] = [ + { operator: 'gte', value: 100, color: 'chart-warning' }, + { operator: 'gte', value: 500, color: 'chart-error' }, + ]; + expect(resolveConditionalColor(1000, rules, 'chart-success')).toBe( + 'chart-error', + ); + }); + + it('returns fallback when no rule matches', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 100, color: 'chart-warning' }, + { operator: 'gte', value: 500, color: 'chart-error' }, + ]; + // value 50: no rule matches, return fallback + expect(resolveConditionalColor(50, rules, 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('covers the DBNumberChart success/warning/error scenario', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 100, color: 'chart-warning' }, + { operator: 'gte', value: 500, color: 'chart-error' }, + ]; + // 50 → no match → static color + expect(resolveConditionalColor(50, rules, 'chart-success')).toBe( + 'chart-success', + ); + // 200 → rule 1 matches, rule 2 doesn't → chart-warning + expect(resolveConditionalColor(200, rules, 'chart-success')).toBe( + 'chart-warning', + ); + // 1000 → both match → last match = chart-error + expect(resolveConditionalColor(1000, rules, 'chart-success')).toBe( + 'chart-error', + ); + }); + + it('string rules do not match numeric values', () => { + const rules: ColorCondition[] = [ + { operator: 'contains', value: 'err', color: 'chart-error' }, + ]; + // numeric value, string rule: no match + expect(resolveConditionalColor(42, rules, 'chart-success')).toBe( + 'chart-success', + ); + }); + + it('returns undefined fallback when fallback is undefined and no rule matches', () => { + const rules: ColorCondition[] = [ + { operator: 'gte', value: 100, color: 'chart-warning' }, + ]; + expect(resolveConditionalColor(50, rules, undefined)).toBeUndefined(); + }); +}); diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 5f5f8005bc..12cb3eba8f 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -20,6 +20,12 @@ import { import { shouldFillNullsWithZero } from '@/ChartUtils'; import { FormatTime } from '@/useFormatTime'; +import { + attachLocalIds, + ColorRulesEditor, + ColorRuleWithId, + stripLocalIds, +} from './ColorRulesEditor'; import { ColorSwatchInput } from './ColorSwatchInput'; import { CheckBoxControlled } from './InputControlled'; import { DEFAULT_NUMBER_FORMAT, NumberFormatForm } from './NumberFormat'; @@ -31,10 +37,19 @@ export type ChartConfigDisplaySettings = Pick< | 'fillNulls' | 'compareToPreviousPeriod' | 'color' + | 'colorRules' > & { groupByColumnsOnLeft?: boolean; }; +/** + * Internal form shape: `colorRules` is stored with `localId`s for dnd-kit + * stability; they are stripped before the settings are passed to `onChange`. + */ +type DrawerFormValues = Omit & { + colorRules?: ColorRuleWithId[]; +}; + interface ChartDisplaySettingsDrawerProps { opened: boolean; settings: ChartConfigDisplaySettings; @@ -53,7 +68,7 @@ interface ChartDisplaySettingsDrawerProps { function applyDefaultSettings( settings: ChartConfigDisplaySettings, fallbackNumberFormat?: NumberFormat, -): ChartConfigDisplaySettings { +): DrawerFormValues { return { numberFormat: settings.numberFormat ?? fallbackNumberFormat ?? DEFAULT_NUMBER_FORMAT, @@ -65,6 +80,9 @@ function applyDefaultSettings( compareToPreviousPeriod: settings.compareToPreviousPeriod ?? false, groupByColumnsOnLeft: settings.groupByColumnsOnLeft ?? false, color: settings.color, + colorRules: settings.colorRules + ? attachLocalIds(settings.colorRules) + : undefined, }; } @@ -84,10 +102,9 @@ export default function ChartDisplaySettingsDrawer({ [settings, defaultNumberFormat], ); - const { control, handleSubmit, reset, setValue } = - useForm({ - defaultValues: appliedDefaults, - }); + const { control, handleSubmit, reset, setValue } = useForm({ + defaultValues: appliedDefaults, + }); useEffect(() => { reset(appliedDefaults); @@ -102,12 +119,24 @@ export default function ChartDisplaySettingsDrawer({ }, [onClose, reset, appliedDefaults]); const applyChanges = useCallback(() => { - handleSubmit(onChange)(); + handleSubmit(formValues => { + // Strip client-side localIds before passing rules to the config. + const { colorRules, ...rest } = formValues; + onChange({ + ...rest, + colorRules: colorRules ? stripLocalIds(colorRules) : undefined, + }); + })(); onClose(); }, [onChange, handleSubmit, onClose]); const resetToDefaults = useCallback(() => { - reset(applyDefaultSettings({}, defaultNumberFormat)); + reset( + applyDefaultSettings( + {} as ChartConfigDisplaySettings, + defaultNumberFormat, + ), + ); }, [reset, defaultNumberFormat]); const isTimeChart = @@ -199,6 +228,15 @@ export default function ChartDisplaySettingsDrawer({ )} /> + + ( + + )} + /> + )} diff --git a/packages/app/src/components/ColorRulesEditor.tsx b/packages/app/src/components/ColorRulesEditor.tsx new file mode 100644 index 0000000000..af0dc83b12 --- /dev/null +++ b/packages/app/src/components/ColorRulesEditor.tsx @@ -0,0 +1,387 @@ +import React, { useCallback } from 'react'; +import { + DndContext, + DragEndEvent, + MouseSensor, + TouchSensor, + useSensor, + useSensors, +} from '@dnd-kit/core'; +import { + arrayMove, + SortableContext, + useSortable, + verticalListSortingStrategy, +} from '@dnd-kit/sortable'; +import { CSS } from '@dnd-kit/utilities'; +import type { + ChartPaletteToken, + ColorCondition, +} from '@hyperdx/common-utils/dist/types'; +import { + ActionIcon, + Box, + Button, + Group, + NumberInput, + Select, + Stack, + Text, + TextInput, +} from '@mantine/core'; +import { IconGripVertical, IconTrash } from '@tabler/icons-react'; + +import { ColorSwatchInput } from './ColorSwatchInput'; + +// ─── Types ─────────────────────────────────────────────────────────────────── + +/** A ColorCondition with a client-side `localId` used as a stable dnd-kit key. */ +export type ColorRuleWithId = ColorCondition & { localId: string }; + +type ColorRulesEditorProps = { + value: ColorRuleWithId[]; + onChange: (rules: ColorRuleWithId[]) => void; +}; + +// ─── Operator options (number-tile subset) ──────────────────────────────────── + +const OPERATOR_OPTIONS = [ + { value: 'gt', label: '>' }, + { value: 'gte', label: '>=' }, + { value: 'lt', label: '<' }, + { value: 'lte', label: '<=' }, + { value: 'between', label: 'between' }, + { value: 'eq', label: '=' }, + { value: 'neq', label: '≠' }, +] as const; + +type NumericTileOperator = (typeof OPERATOR_OPTIONS)[number]['value']; + +/** Default rule added when the user clicks "Add rule". */ +function makeDefaultRule(): ColorRuleWithId { + return { + localId: crypto.randomUUID(), + operator: 'gt', + value: 0, + color: 'chart-1', + }; +} + +// ─── Single sortable rule row ───────────────────────────────────────────────── + +function SortableRuleRow({ + rule, + index, + onUpdate, + onDelete, +}: { + rule: ColorRuleWithId; + index: number; + onUpdate: (index: number, next: ColorRuleWithId) => void; + onDelete: (index: number) => void; +}) { + const { + attributes, + listeners, + setNodeRef, + transform, + transition, + isDragging, + } = useSortable({ id: rule.localId }); + + const style: React.CSSProperties = { + transform: CSS.Transform.toString(transform), + transition, + opacity: isDragging ? 0.5 : 1, + }; + + const handleOperatorChange = useCallback( + (op: string | null) => { + if (!op) return; + const operator = op as NumericTileOperator; + // When switching to/from 'between', reset the value to a valid shape. + if (operator === 'between') { + onUpdate(index, { + ...rule, + operator: 'between', + value: [0, 100], + } as ColorRuleWithId); + } else if (operator === 'eq' || operator === 'neq') { + // eq/neq accept number or string; default to number 0 + const currentVal = + rule.operator !== 'between' && typeof rule.value === 'number' + ? rule.value + : 0; + onUpdate(index, { + ...rule, + operator, + value: currentVal, + } as ColorRuleWithId); + } else { + // Numeric ordered: gt/gte/lt/lte need a number + const currentVal = + rule.operator !== 'between' && typeof rule.value === 'number' + ? rule.value + : 0; + onUpdate(index, { + ...rule, + operator, + value: currentVal, + } as ColorRuleWithId); + } + }, + [index, onUpdate, rule], + ); + + const handleColorChange = useCallback( + (color?: ChartPaletteToken) => { + onUpdate(index, { ...rule, color: color ?? 'chart-1' }); + }, + [index, onUpdate, rule], + ); + + const handleDelete = useCallback(() => onDelete(index), [index, onDelete]); + + // Value inputs differ by operator + let valueInputs: React.ReactNode; + if (rule.operator === 'between') { + const [lo, hi] = rule.value; + valueInputs = ( + + + onUpdate(index, { + ...rule, + operator: 'between', + value: [typeof v === 'number' ? v : 0, hi], + } as ColorRuleWithId) + } + aria-label={`Rule ${index + 1} lower bound`} + w={72} + /> + + to + + + onUpdate(index, { + ...rule, + operator: 'between', + value: [lo, typeof v === 'number' ? v : 0], + } as ColorRuleWithId) + } + aria-label={`Rule ${index + 1} upper bound`} + w={72} + /> + + ); + } else if (rule.operator === 'eq' || rule.operator === 'neq') { + // Accept text for eq/neq; if parseable as number convert it, else keep string + const displayVal = + typeof rule.value === 'number' + ? String(rule.value) + : (rule.value as string); + valueInputs = ( + { + const raw = e.currentTarget.value; + const num = Number(raw); + const coerced = + raw !== '' && !Number.isNaN(num) && Number.isFinite(num) + ? num + : raw; + onUpdate(index, { + ...rule, + operator: rule.operator, + value: coerced, + } as ColorRuleWithId); + }} + aria-label={`Rule ${index + 1} value`} + w={120} + /> + ); + } else { + // Numeric ordered: gt/gte/lt/lte + valueInputs = ( + + onUpdate(index, { + ...rule, + value: typeof v === 'number' ? v : 0, + } as ColorRuleWithId) + } + aria-label={`Rule ${index + 1} value`} + w={120} + /> + ); + } + + return ( + + + {/* Drag handle */} + + + + + {/* Operator */} + so its state is + // never tied to a virtual row's lifecycle. + const ts = Date.now(); + + await test.step('Create a table tile with a Search row-click action', async () => { + await addTableTile(`E2E Tooltip ${ts}`); + await dashboardPage.chartEditor.openRowClickDrawer(); + await dashboardPage.chartEditor.setRowClickMode('Search'); + await dashboardPage.chartEditor.fillRowClickTemplate( + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.chartEditor.applyRowClickDrawer(); + await dashboardPage.saveTile(); + }); + + await test.step('Set time range to Last 6 hours so rows render', async () => { + await dashboardPage.timePicker.selectRelativeTime('Last 6 hours'); + }); + + await dashboardPage.waitForTableTileRows(0); + + await test.step('Hover over first row — tooltip hint must appear', async () => { + await dashboardPage.hoverFirstTableRowAndGetTooltip(0); + // hoverFirstTableRowAndGetTooltip already asserts visibility before + // returning; reaching here means the tooltip appeared successfully. + }); + + await test.step('Move mouse away from the table — tooltip must disappear', async () => { + // Move to a neutral area well outside the table. The + // onMouseLeave safety net clears hoveredVirtualIndex, which makes + // hoveredRowDescription derive to null, disabling the shared + // Tooltip.Floating (display:none in the Portal). + await page.mouse.move(10, 10); + await expect(page.getByTestId('row-action-hint')).toBeHidden({ + timeout: 3000, + }); + }); + }); }, ); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 93c14f1e6a..4f5d2778c4 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -970,6 +970,45 @@ export class DashboardPage { .click(); } + /** + * Return the first data row () of the table in the + * given tile. Used for hover-based interactions (e.g. tooltip tests). + */ + getFirstTableRow(tileIndex = 0): Locator { + return this.getTile(tileIndex) + .locator('table tbody tr[data-index]') + .first(); + } + + /** + * Hover over the first data row of a table tile and wait for the + * floating tooltip to appear. Returns the tooltip locator so callers + * can make further assertions. + * + * Tooltip.Floating renders its content inside a Portal at the document + * body level. Mantine uses CSS modules (hashed classes) so we locate the + * floating popup by text content that matches the known hint patterns used + * by describeOnClick (e.g. "Open in search", "Search ", + * "Open dashboard"). The tooltip is shown via `display: block` on the + * Portal div, so Playwright's `toBeVisible` checks that correctly. + */ + async hoverFirstTableRowAndGetTooltip(tileIndex = 0): Promise { + const row = this.getFirstTableRow(tileIndex); + await row.hover(); + // Trigger a mousemove inside the row so Tooltip.Floating's internal + // mousemove handler has a coordinate to position against. + const box = await row.boundingBox(); + if (box) { + await this.page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); + } + // The Tooltip.Floating label is a . + // The portal div uses display:none when disabled, so Playwright's + // toBeVisible() correctly reflects the open/closed state. + const tooltip = this.page.getByTestId('row-action-hint'); + await tooltip.waitFor({ state: 'visible', timeout: 5000 }); + return tooltip; + } + /** * Locator for the Mantine toast raised by useOnClickLinkBuilder when the * configured onClick action fails (unknown source, missing row column, etc). diff --git a/packages/common-utils/src/__tests__/types.test.ts b/packages/common-utils/src/__tests__/types.test.ts new file mode 100644 index 0000000000..ba16b02195 --- /dev/null +++ b/packages/common-utils/src/__tests__/types.test.ts @@ -0,0 +1,284 @@ +import { z } from 'zod'; + +import { ColorConditionSchema } from '../types'; + +describe('ColorConditionSchema', () => { + // ─── Positive cases ───────────────────────────────────────────────────────── + + describe('numeric ordered operators', () => { + it.each(['gt', 'gte', 'lt', 'lte'] as const)( + 'parses operator %s with a valid numeric value', + operator => { + const result = ColorConditionSchema.safeParse({ + operator, + value: 42, + color: 'chart-success', + }); + expect(result.success).toBe(true); + }, + ); + + it('parses with an optional label', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'gte', + value: 100, + color: 'chart-warning', + label: 'High', + }); + expect(result.success).toBe(true); + }); + }); + + describe('between operator', () => { + it('parses a valid between rule', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'between', + value: [10, 100], + color: 'chart-1', + }); + expect(result.success).toBe(true); + }); + + it('allows inverted between (first > second)', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'between', + value: [100, 10], + color: 'chart-1', + }); + expect(result.success).toBe(true); + }); + }); + + describe('eq / neq operators', () => { + it('parses eq with a number value', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'eq', + value: 5, + color: 'chart-error', + }); + expect(result.success).toBe(true); + }); + + it('parses eq with a string value', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'eq', + value: 'CRIT', + color: 'chart-error', + }); + expect(result.success).toBe(true); + }); + + it('parses neq with a number value', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'neq', + value: 0, + color: 'chart-2', + }); + expect(result.success).toBe(true); + }); + }); + + describe('string operators', () => { + it.each(['contains', 'startsWith', 'endsWith'] as const)( + 'parses operator %s with a non-empty string value', + operator => { + const result = ColorConditionSchema.safeParse({ + operator, + value: 'error', + color: 'chart-error', + }); + expect(result.success).toBe(true); + }, + ); + + it('parses regex with a valid pattern', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'regex', + value: '^error.*', + color: 'chart-error', + }); + expect(result.success).toBe(true); + }); + }); + + it('parses with all palette tokens', () => { + const tokens = [ + 'chart-1', + 'chart-2', + 'chart-3', + 'chart-4', + 'chart-5', + 'chart-6', + 'chart-7', + 'chart-8', + 'chart-9', + 'chart-10', + 'chart-success', + 'chart-warning', + 'chart-error', + ] as const; + for (const token of tokens) { + const result = ColorConditionSchema.safeParse({ + operator: 'gt', + value: 0, + color: token, + }); + expect(result.success).toBe(true); + } + }); + + // ─── Negative cases ────────────────────────────────────────────────────────── + + it('rejects an unknown operator', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'notAnOp', + value: 1, + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects NaN on numeric operators', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'gt', + value: Number.NaN, + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects Infinity on numeric operators', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'lt', + value: Infinity, + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects a string value on a numeric operator (gt)', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'gt', + value: 'oops', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects a number value on a string operator (contains)', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'contains', + value: 42, + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects an invalid palette token', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'gt', + value: 1, + color: 'not-a-token', + }); + expect(result.success).toBe(false); + }); + + it('rejects an empty string on contains', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'contains', + value: '', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects an empty string on startsWith', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'startsWith', + value: '', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects an empty string on endsWith', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'endsWith', + value: '', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects an empty string on regex', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'regex', + value: '', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects an unparseable regex pattern', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'regex', + value: '[invalid', + color: 'chart-1', + }); + expect(result.success).toBe(false); + }); + + it('rejects a label longer than 40 characters', () => { + const result = ColorConditionSchema.safeParse({ + operator: 'gt', + value: 1, + color: 'chart-1', + label: 'a'.repeat(41), + }); + expect(result.success).toBe(false); + }); +}); + +describe('colorRules array in SharedChartSettingsSchema', () => { + // Test array constraints directly with a z.array(ColorConditionSchema).max(10) schema, + // mirroring how SharedChartSettingsSchema declares colorRules. + const rulesSchema = z.array(ColorConditionSchema).max(10).optional(); + + it('accepts 0 rules', () => { + expect(rulesSchema.safeParse([]).success).toBe(true); + }); + + it('accepts 1 rule', () => { + expect( + rulesSchema.safeParse([{ operator: 'gt', value: 0, color: 'chart-1' }]) + .success, + ).toBe(true); + }); + + it('accepts 5 rules', () => { + const rules = Array.from({ length: 5 }, (_, i) => ({ + operator: 'gt' as const, + value: i * 10, + color: 'chart-1' as const, + })); + expect(rulesSchema.safeParse(rules).success).toBe(true); + }); + + it('accepts 10 rules', () => { + const rules = Array.from({ length: 10 }, (_, i) => ({ + operator: 'gte' as const, + value: i * 10, + color: 'chart-1' as const, + })); + expect(rulesSchema.safeParse(rules).success).toBe(true); + }); + + it('rejects 11 rules', () => { + const rules = Array.from({ length: 11 }, (_, i) => ({ + operator: 'gte' as const, + value: i * 10, + color: 'chart-1' as const, + })); + expect(rulesSchema.safeParse(rules).success).toBe(false); + }); +}); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 4f7eab990b..cba3959f77 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -848,6 +848,73 @@ export function isChartPaletteToken( /** Zod schema that accepts only the curated palette tokens above. */ export const ChartPaletteTokenSchema = z.enum(CHART_PALETTE_TOKENS); +/** + * A single conditional color rule. Rules are evaluated in order against + * the tile's displayed value; the LAST matching rule's color wins + * (last-match-wins: higher-priority rules go last). If no rule matches, + * the tile's static `color` applies; if that is unset, the default text + * color applies. + * + * String operators (`contains`, `startsWith`, `endsWith`, `regex`) are + * included at the schema level so a future table-tile slice can reuse + * the same type without a schema change. The number-tile UI only exposes + * numeric / equality operators. + * + * Lives in common-utils so both the app and a future external-API parity + * PR can import it. + */ +export const ColorConditionSchema = z.discriminatedUnion('operator', [ + // Numeric ordered operators + z.object({ + operator: z.enum(['gt', 'gte', 'lt', 'lte']), + value: z.number().finite(), + color: ChartPaletteTokenSchema, + label: z.string().max(40).optional(), + }), + z.object({ + operator: z.literal('between'), + value: z.tuple([z.number().finite(), z.number().finite()]), + color: ChartPaletteTokenSchema, + label: z.string().max(40).optional(), + }), + // Equality (number OR string) + z.object({ + operator: z.enum(['eq', 'neq']), + value: z.union([z.number().finite(), z.string().max(200)]), + color: ChartPaletteTokenSchema, + label: z.string().max(40).optional(), + }), + // String operators (allowed at schema level for future table-tile reuse) + z.object({ + operator: z.enum(['contains', 'startsWith', 'endsWith']), + value: z.string().min(1).max(200), + color: ChartPaletteTokenSchema, + label: z.string().max(40).optional(), + }), + z.object({ + operator: z.literal('regex'), + value: z + .string() + .min(1) + .max(500) + .refine( + v => { + try { + new RegExp(v); + return true; + } catch { + return false; + } + }, + { message: 'Invalid regex pattern' }, + ), + color: ChartPaletteTokenSchema, + label: z.string().max(40).optional(), + }), +]); + +export type ColorCondition = z.infer; + // When making changes here, consider if they need to be made to the external API // schema as well (packages/api/src/utils/zod.ts). /** @@ -870,6 +937,11 @@ const SharedChartSettingsSchema = z.object({ // also a Number-tile-only field stored at shared level and gated in // the UI. color: ChartPaletteTokenSchema.optional(), + // Ordered conditional color rules for number tiles. Last matching rule + // wins (higher-priority rules go last). Kept at shared level so a future + // table-tile slice can attach per-column rules without a schema change. + // The UI gates the section on `displayType === DisplayType.Number`. + colorRules: z.array(ColorConditionSchema).max(10).optional(), }); export const _ChartConfigSchema = SharedChartSettingsSchema.extend({