fix: Scrolling horizontally in Linker mode renders empty cells

[emilyhuxng]

Fixes #1146

Changed assertNotNull to getOrThrow and update hoverSelectColumn on scroll so the highlighted column will change on scroll.

[codecov]

Codecov Report

Merging #1160 (a3bf053) into main (76b3bd5) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head a3bf053 differs from pull request most recent head 0405851. Consider uploading reports for the commit 0405851 to get more accurate results

@@           Coverage Diff           @@
##             main    #1160   +/-   ##
=======================================
  Coverage   44.13%   44.13%           
=======================================
  Files         447      447           
  Lines       33266    33266           
  Branches     8356     8357    +1     
=======================================
+ Hits        14681    14682    +1     
+ Misses      18536    18535    -1     
  Partials       49       49           

Flag	Coverage Δ
unit	44.13% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/iris-grid/src/IrisGridRenderer.ts	24.75% <0.00%> (+0.04%)	⬆️
.../mousehandlers/IrisGridColumnSelectMouseHandler.ts	5.97% <0.00%> (-0.19%)	⬇️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mofojed]

Need to update the PR description to link it to the ticket it fixes (#1146).
I'd also like an investigation how we're getting into this state, as it indicates there are possibly other issues.

[mofojed]

I'm concerned how we got into this state. The reason there were asserts there is it should never be the case that we have a hoverSelectColumn but no matching columnX/columnWidth in the metrics. How are we getting into this state?

[emilyhuxng]

onWheel in IrisGridColumnSelectMouseHandler doesn't update hoverSelectColumn so if you were hovering column 0 for example, as you scroll, hoverSelectColumn stays 0 but eventually allColumnXs and allColumnWidths will not have 0 as a key anymore as column 0 isn't in view. Should hoverSelectColumn be updated in onWheel as well? This would result in it highlighting while the user is scrolling rather than needing to move cursor afterwards.

[mofojed]

Yes - we're updating the cursor, we should update the hover column as well.

[mofojed]

We should leave this assertion here (or just use getOrThrow instead):

const x = getOrThrow(allColumnXs, hoverSelectColumn);
const columnWidth = getOrThrow(allColumnWidths, hoverSelectColumn);

This isn't an exceptional case we should have error handling before; we expect the metrics for the hoverSelectColumn to always exist, otherwise there is a problem with our code. In this case, the assertion helped us catch the root cause of the issue (not updating the column while wheeling).

[mofojed]

We should leave this assertion here (or just use getOrThrow instead):

const x = getOrThrow(allColumnXs, hoverSelectColumn);
const columnWidth = getOrThrow(allColumnWidths, hoverSelectColumn);

This isn't an exceptional case we should have error handling before; we expect the metrics for the hoverSelectColumn to always exist, otherwise there is a problem with our code. In this case, the assertion helped us catch the root cause of the issue (not updating the column while wheeling).

[mofojed]

PR description will need to be updated before merging (it no longer matches the actual change).