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.

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

Table: Highlight row on shared crosshair #78392

Merged
merged 21 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ Experimental features might be changed or removed without prior notice.
| `flameGraphItemCollapsing` | Allow collapsing of flame graph items |
| `logRowsPopoverMenu` | Enable filtering menu displayed when text of a log line is selected |
| `pluginsSkipHostEnvVars` | Disables passing host environment variable to plugin processes |
| `tableSharedCrosshair` | Enables shared crosshair in table panel |

## Development feature toggles

Expand Down
1 change: 1 addition & 0 deletions packages/grafana-data/src/types/featureToggles.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,5 @@ export interface FeatureToggles {
alertingSimplifiedRouting?: boolean;
logRowsPopoverMenu?: boolean;
pluginsSkipHostEnvVars?: boolean;
tableSharedCrosshair?: boolean;
}
60 changes: 60 additions & 0 deletions packages/grafana-ui/src/components/Table/Table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -657,4 +657,64 @@ describe('Table', () => {
expect(subTable.style.height).toBe('108px');
});
});

describe('when data hover events are emitted', () => {
it('should highlight the row', () => {
const frame = toDataFrame({
fields: [
{ name: 'time', type: FieldType.time, values: [10000, 20000, 30000, 40000] },
{ name: 'val', type: FieldType.number, values: [1, 2, 3, 4] },
],
});

getTestContext({ data: frame, rowHighlightIndex: 0 });

let rows = within(getTable()).getAllByRole('row');

// if not set, returns an empty string
expect(rows[1].style.backgroundColor).toBeTruthy();

expect(rows[2].style.backgroundColor).toBeFalsy();
expect(rows[3].style.backgroundColor).toBeFalsy();
expect(rows[4].style.backgroundColor).toBeFalsy();
});
it('should highlight the first row that matches the time value', () => {
const frame = toDataFrame({
fields: [
{ name: 'time', type: FieldType.time, values: [10000, 20000, 30000, 40000, 10000, 10000] },
{ name: 'val', type: FieldType.number, values: [1, 2, 3, 4, 5, 6] },
],
});

getTestContext({ data: frame, rowHighlightIndex: 0 });

let rows = within(getTable()).getAllByRole('row');

// if not set, returns an empty string
expect(rows[1].style.backgroundColor).toBeTruthy();

expect(rows[2].style.backgroundColor).toBeFalsy();
expect(rows[3].style.backgroundColor).toBeFalsy();
expect(rows[4].style.backgroundColor).toBeFalsy();
expect(rows[5].style.backgroundColor).toBeFalsy();
expect(rows[6].style.backgroundColor).toBeFalsy();
});
it('should not highlight the row if wrong index', () => {
const frame = toDataFrame({
fields: [
{ name: 'time', type: FieldType.time, values: [10000, 20000, 30000, 40000] },
{ name: 'val', type: FieldType.number, values: [1, 2, 3, 4] },
],
});

getTestContext({ data: frame, rowHighlightIndex: 6 });

let rows = within(getTable()).getAllByRole('row');

expect(rows[2].style.backgroundColor).toBeFalsy();
expect(rows[2].style.backgroundColor).toBeFalsy();
expect(rows[3].style.backgroundColor).toBeFalsy();
expect(rows[4].style.backgroundColor).toBeFalsy();
});
});
});
31 changes: 28 additions & 3 deletions packages/grafana-ui/src/components/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export const Table = memo((props: Props) => {
enablePagination,
cellHeight = TableCellHeight.Sm,
timeRange,
onRowHover,
onRowLeave,
rowHighlightIndex,
} = props;

const listRef = useRef<VariableSizeList>(null);
Expand Down Expand Up @@ -228,6 +231,15 @@ export const Table = memo((props: Props) => {
setPageSize(pageSize);
}, [pageSize, setPageSize]);

let scrollTop = undefined;
if (rowHighlightIndex !== undefined) {
const firstMatchedRowIndex = rows.findIndex((row) => row.index === rowHighlightIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

I know findIndex returns the first instance of the match, but for the sake of curiosity, would there ever be more then 1 unique time value in a table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There could be, but I don't think there are many use-cases for such a scenario. I've also thought about it and decided it's better to return just one match.


if (firstMatchedRowIndex !== -1) {
scrollTop = headerHeight + (firstMatchedRowIndex - 1) * tableStyles.rowHeight;
}
}

useResetVariableListSizeCache(extendedState, listRef, data);
useFixScrollbarContainer(variableSizeListScrollbarRef, tableDivRef);

Expand All @@ -247,8 +259,17 @@ export const Table = memo((props: Props) => {

const expandedRowStyle = state.expanded[row.index] ? css({ '&:hover': { background: 'inherit' } }) : {};

if (rowHighlightIndex !== undefined && row.index === rowHighlightIndex) {
style = { ...style, backgroundColor: theme.components.table.rowHoverBackground };
}

return (
<div {...row.getRowProps({ style })} className={cx(tableStyles.row, expandedRowStyle)}>
<div
{...row.getRowProps({ style })}
className={cx(tableStyles.row, expandedRowStyle)}
onMouseEnter={() => (onRowHover ? onRowHover(index, data) : null)}
onMouseLeave={() => (onRowLeave ? onRowLeave() : null)}
>
{/*add the nested data to the DOM first to prevent a 1px border CSS issue on the last cell of the row*/}
{nestedDataField && state.expanded[row.index] && (
<ExpandedRow
Expand Down Expand Up @@ -279,13 +300,17 @@ export const Table = memo((props: Props) => {
rows,
prepareRow,
state.expanded,
rowHighlightIndex,
tableStyles,
nestedDataField,
width,
cellHeight,
theme.components.table.rowHoverBackground,
onRowHover,
data,
onRowLeave,
onCellFilterAdded,
timeRange,
data,
]
);

Expand Down Expand Up @@ -356,7 +381,7 @@ export const Table = memo((props: Props) => {
)}
{itemCount > 0 ? (
<div data-testid={selectors.components.Panels.Visualization.Table.body} ref={variableSizeListScrollbarRef}>
<CustomScrollbar onScroll={handleScroll} hideHorizontalTrack={true}>
<CustomScrollbar onScroll={handleScroll} hideHorizontalTrack={true} scrollTop={scrollTop}>
<VariableSizeList
// This component needs an unmount/remount when row height or page changes
key={tableStyles.rowHeight + state.pageIndex}
Expand Down
4 changes: 4 additions & 0 deletions packages/grafana-ui/src/components/Table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ export interface Props {
cellHeight?: schema.TableCellHeight;
/** @alpha Used by SparklineCell when provided */
timeRange?: TimeRange;
onRowHover?: (idx: number, frame: DataFrame) => void;
onRowLeave?: () => void;
/** Used to highlight rows with the given time value. Used with DataHoverEvent */
rowHighlightIndex?: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be used standalone to highlight a row in some other scenarios. If set it overwrites the one received through the data hover event. Not sure if could be useful or if it's too much.

}

/**
Expand Down
34 changes: 34 additions & 0 deletions packages/grafana-ui/src/components/Table/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,37 @@ export function getAlignmentFactor(
return alignmentFactor;
}
}

export function hasTimeField(data: DataFrame): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should probably be in @grafana/data as we have functions like this there already

return data.fields.some((field) => field.type === FieldType.time);
}

// since the conversion from timeseries panel crosshair to time is pixel based, we need
// to set a threshold where the table row highlights when the crosshair is hovered over a certain point
// because multiple pixels (converted to times) may represent the same point/row in table
export function isPointTimeValAroundTableTimeVal(pointTime: number, rowTime: number, threshold: number) {
return Math.abs(Math.floor(pointTime) - rowTime) < threshold;
}

// calculate the threshold for which we consider a point in a chart
// to match a row in a table based on a time value
export function calculateAroundPointThreshold(timeField: Field): number {
let max = -Number.MAX_VALUE;
let min = Number.MAX_VALUE;

if (timeField.values.length < 2) {
return 0;
}

for (let i = 0; i < timeField.values.length; i++) {
const value = timeField.values[i];
if (value > max) {
max = value;
}
if (value < min) {
min = value;
}
}

return (max - min) / timeField.values.length;
}
Comment on lines +546 to +565
Copy link
Contributor

Choose a reason for hiding this comment

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

This works a lot better! 🥇

7 changes: 7 additions & 0 deletions pkg/services/featuremgmt/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,13 @@ var (
FrontendOnly: false,
Owner: grafanaPluginsPlatformSquad,
},
{
Name: "tableSharedCrosshair",
Description: "Enables shared crosshair in table panel",
FrontendOnly: true,
Stage: FeatureStageExperimental,
Owner: grafanaBiSquad,
},
}
)

Expand Down
1 change: 1 addition & 0 deletions pkg/services/featuremgmt/toggles_gen.csv
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,4 @@ datatrails,experimental,@grafana/dashboards-squad,false,false,false,true
alertingSimplifiedRouting,experimental,@grafana/alerting-squad,false,false,false,false
logRowsPopoverMenu,experimental,@grafana/observability-logs,false,false,false,true
pluginsSkipHostEnvVars,experimental,@grafana/plugins-platform-backend,false,false,false,false
tableSharedCrosshair,experimental,@grafana/grafana-bi-squad,false,false,false,true
4 changes: 4 additions & 0 deletions pkg/services/featuremgmt/toggles_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,4 +590,8 @@ const (
// FlagPluginsSkipHostEnvVars
// Disables passing host environment variable to plugin processes
FlagPluginsSkipHostEnvVars = "pluginsSkipHostEnvVars"

// FlagTableSharedCrosshair
// Enables shared crosshair in table panel
FlagTableSharedCrosshair = "tableSharedCrosshair"
)