fix: non-contiguous table row selection background colour (#1623)

Fixes #1619

Draw selection was calling the fill/stroke inside the loop, which
implicitly calls closePath, resulting the fill to be drawed over top of
itself multiple times. Move the fill/stroke outside the loop the creates
the selection rects.

Adds an e2e test that makes a non-contiguous selection to prevent
regressions.

packages/grid/src/GridRenderer.ts

@@ -2109,30 +2109,36 @@ export class GridRenderer {
 
         context.rect(x, y, endX - x, endY - y);
       }
+    }
 
-      // draw the inner transparent fill
-      context.fillStyle = theme.selectionColor;
-      context.fill();
+    /**
+     * Create the path, then draw it once. Fill and
+     * stroke must be outside the beginPath loop otherwise
+     * the fill/stroke will be drawn multiple times.
+     */
 
-      /**
-       * draw an "inner stroke" that's clipped to just inside of the rects
-       * to act as a casing to the outer stroke. 3px width because 1px is outside
-       * the rect (but clipped), 1px is "on" the rect (technically this pixel is
-       * a half pixel clip as well due to rects offset, but we are immediately painting
-       * over it), and then the 1px inside (which is the desired pixel).
-       */
-      context.save();
-      context.clip();
-      context.strokeStyle = theme.selectionOutlineCasingColor;
-      context.lineWidth = 3;
-      context.stroke();
-      context.restore();
+    // draw the inner transparent fill
+    context.fillStyle = theme.selectionColor;
+    context.fill();
 
-      // draw the outerstroke border on top of the inner stroke
-      context.strokeStyle = theme.selectionOutlineColor;
-      context.lineWidth = 1;
-      context.stroke();
-    }
+    /**
+     * draw an "inner stroke" that's clipped to just inside of the rects
+     * to act as a casing to the outer stroke. 3px width because 1px is outside
+     * the rect (but clipped), 1px is "on" the rect (technically this pixel is
+     * a half pixel clip as well due to rects offset, but we are immediately painting
+     * over it), and then the 1px inside (which is the desired pixel).
+     */
+    context.save();
+    context.clip();
+    context.strokeStyle = theme.selectionOutlineCasingColor;
+    context.lineWidth = 3;
+    context.stroke();
+    context.restore();
+
+    // draw the outerstroke border on top of the inner stroke
+    context.strokeStyle = theme.selectionOutlineColor;
+    context.lineWidth = 1;
+    context.stroke();
 
     if (isCursorVisible && column != null && row != null) {
       context.restore();

tests/table.spec.ts

@@ -38,6 +38,36 @@ test('can open a simple table', async ({ page }) => {
   await expect(page.locator('.iris-grid-panel .iris-grid')).toHaveScreenshot();
 });
 
+test('can make a non-contiguous table row selection', async ({ page }) => {
+  await page.goto('');
+  await openSimpleTable(page);
+
+  const grid = await page.locator('.iris-grid-panel .iris-grid');
+  const gridLocation = await grid.boundingBox();
+  expect(gridLocation).not.toBeNull();
+  if (gridLocation === null) return;
+
+  // Based on default row and header height in IrisGridTheme
+  // Ideally this would be calculated from the current theme
+  const rowHeight = 19;
+  const columnHeaderHeight = 30;
+
+  // ctrl+click on every other row for 9 rows, starting at row 0 after the header
+  /* eslint-disable no-await-in-loop */
+  for (let i = 0; i < 9; i += 1) {
+    await page.keyboard.down('Control');
+    await page.mouse.click(
+      gridLocation.x + 1, // plus one so we click on the first pixel not 0
+      gridLocation.y + 1 + columnHeaderHeight + rowHeight * i * 2
+      // times 2 because we're skipping every other row
+    );
+    await page.keyboard.up('Control');
+  }
+  /* eslint-enable no-await-in-loop */
+
+  await expect(page.locator('.iris-grid-panel .iris-grid')).toHaveScreenshot();
+});
+
 test('can open a table with column header groups', async ({ page }) => {
   await page.goto('');
   const consoleInput = page.locator('.console-input');