Alarm Table Panel: Incorrect context menu when items are scrolled

Description

Reported by a customer in

When scrolling is enabled, and there are more entries in the table than fit in the panel (i.e., you need to scroll down in order to see all entries), if you double-click on any given entry the "Alarm Detail" window that pops up is not the correct one (does not correlate with the entry that was doubled-clicked).

For example, when clicking on an entry at the bottom of the table after scrolling down that has an alarm ID 80, the Alarm Detail window that pops up is for alarm ID 10. For the entries at the top of the table that fit in the panel without having to scroll down, this doesn't happen and it works fine.

Found using Grafana 10.0.2 and plugin version 9.0.13.

Activity

Show:

Scott Theleman May 23, 2024 at 5:58 PM
Edited

I worked on this, see branch:

Adding notes here so I or someone can continue working on this, as it is a real bug.

The main issue seems to be, we are saving an array of booleans, the order of items in the array matches the rows currently on the current page, whether or not they are actually visible. For example, if there are 20 items total, and user sets rows per page at 10, but panel is only big enough to view 7 items, the vertical scrollbar will appear. The selectedIndexes will be size 10, booleans are true for when a row/item has been selected.

When a user clicks a row, we use the e.target from the MouseEvent to find the cell, then get parent row. Then use CSS selector of div[role="row"] to get the table rows, then loop through these and do a reference equality check of e.target and row. Then map that index back into the currently-paged filteredProps DataFrame data.

However, if scrolling is involved, it looks like the row elements don’t always stay in same order, could be some lower level thing where row HTMLElement objects are mapped to different set of cells, etc. So if you have to scroll to see an item and click on it, often we select the wrong item, so Alarm Details or any actions will operate on the wrong alarm.

My solution is to attach some data to each row with the Alarm ID. However, I couldn’t find a way to do this. As far as I could tell, the Grafana Table plugin just doesn’t give access to the underlying react-table in any way that would allow, for example, to add a data-alarm-id attribute to each row, etc.

So instead, if we mandate that the ID (alarm id) column has to be present, we can get the Alarm ID out of the text. So we save a set of selected Alarm ID values. We have to do mapping between those and the data to find the order in the data rows for the page, etc. This is implemented in the listed branch, but still has some issues. Selecting individual items now works correctly even when scrolling, but selecting multiple items has some issues, mostly with applying styles for the selection dashed box.

Unfortunately, doing all this was a pretty significant change. Created a draft PR as well. I’ll try to work on this in the background.

Details

Assignee

Reporter

Fix versions

Affects versions

Sprint

Priority

PagerDuty

Created May 6, 2024 at 2:58 PM
Updated May 23, 2024 at 6:03 PM