From 7c0c782dedf6061bdaab58db5e1e75479c34a8d8 Mon Sep 17 00:00:00 2001 From: Cammille Carinan Date: Tue, 28 Jul 2026 16:17:09 +0200 Subject: [PATCH 1/4] chore: skip patch files in the whitespace hooks --- .pre-commit-config.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8d5861e9..468f60ee 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,12 +11,14 @@ repos: - id: check-merge-conflict - id: check-toml - id: check-yaml + # Patch files carry significant whitespace: a blank context line is a + # single space, and normalising it stops pnpm applying the patch. - id: end-of-file-fixer stages: [pre-commit] - exclude: ^frontend/apps/demo/public/ + exclude: ^frontend/(apps/demo/public/|patches/) - id: trailing-whitespace stages: [pre-commit] - exclude: ^frontend/apps/demo/public/ + exclude: ^frontend/(apps/demo/public/|patches/) # Recorded HTTP cassettes are scrubbed at record time; this gate catches # anything the scrubbing missed before it can be committed. Known-benign From 9d70591b014170c36a57e47cfcb57014853bd0fc Mon Sep 17 00:00:00 2001 From: Cammille Carinan Date: Tue, 28 Jul 2026 16:17:15 +0200 Subject: [PATCH 2/4] fix(frontend/table): stop the grid autoscrolling on a scrollbar drag --- .../@glideapps__glide-data-grid@6.0.3.patch | 60 +++++++++++++++++++ frontend/patches/README.md | 30 ++++++++++ frontend/pnpm-lock.yaml | 15 +++-- frontend/pnpm-workspace.yaml | 3 + 4 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 frontend/patches/@glideapps__glide-data-grid@6.0.3.patch create mode 100644 frontend/patches/README.md diff --git a/frontend/patches/@glideapps__glide-data-grid@6.0.3.patch b/frontend/patches/@glideapps__glide-data-grid@6.0.3.patch new file mode 100644 index 00000000..23b6e746 --- /dev/null +++ b/frontend/patches/@glideapps__glide-data-grid@6.0.3.patch @@ -0,0 +1,60 @@ +diff --git a/dist/cjs/internal/data-grid/data-grid.js b/dist/cjs/internal/data-grid/data-grid.js +index 412d81c8748b979a9d988f2e3cfa8eddc1bfc555..1d7ebc9ce3dbfd793fc009b318f3e1e6c663d1a5 100644 +--- a/dist/cjs/internal/data-grid/data-grid.js ++++ b/dist/cjs/internal/data-grid/data-grid.js +@@ -562,6 +562,13 @@ const DataGrid = (p, forwardedRef) => { + const eventTarget = eventTargetRef?.current; + if (canvas === null || (ev.target !== canvas && ev.target !== eventTarget)) + return; ++ // A scrollbar press targets the scroller from outside its client box, ++ // and autoscrolls the grid if let through. Overlay bars slip past. ++ if (ev instanceof MouseEvent && eventTarget !== null && ++ ev.target === eventTarget && ++ (ev.offsetX > eventTarget.clientWidth || ++ ev.offsetY > eventTarget.clientHeight)) ++ return; + mouseDown.current = true; + let clientX; + let clientY; +@@ -573,11 +580,6 @@ const DataGrid = (p, forwardedRef) => { + clientX = ev.touches[0].clientX; + clientY = ev.touches[0].clientY; + } +- if (ev.target === eventTarget && eventTarget !== null) { +- const bounds = eventTarget.getBoundingClientRect(); +- if (clientX > bounds.right || clientY > bounds.bottom) +- return; +- } + const args = getMouseArgsForPosition(canvas, clientX, clientY, ev); + downPosition.current = args.location; + if (args.isTouch) { +diff --git a/dist/esm/internal/data-grid/data-grid.js b/dist/esm/internal/data-grid/data-grid.js +index c32e94b805eeb5291d0c3aa3b44b8dc43e9f99b4..11f7dfeeb7bfbe314a82a9afb9229ae3949456f4 100644 +--- a/dist/esm/internal/data-grid/data-grid.js ++++ b/dist/esm/internal/data-grid/data-grid.js +@@ -535,6 +535,13 @@ const DataGrid = (p, forwardedRef) => { + const eventTarget = eventTargetRef?.current; + if (canvas === null || (ev.target !== canvas && ev.target !== eventTarget)) + return; ++ // A scrollbar press targets the scroller from outside its client box, ++ // and autoscrolls the grid if let through. Overlay bars slip past. ++ if (ev instanceof MouseEvent && eventTarget !== null && ++ ev.target === eventTarget && ++ (ev.offsetX > eventTarget.clientWidth || ++ ev.offsetY > eventTarget.clientHeight)) ++ return; + mouseDown.current = true; + let clientX; + let clientY; +@@ -546,11 +553,6 @@ const DataGrid = (p, forwardedRef) => { + clientX = ev.touches[0].clientX; + clientY = ev.touches[0].clientY; + } +- if (ev.target === eventTarget && eventTarget !== null) { +- const bounds = eventTarget.getBoundingClientRect(); +- if (clientX > bounds.right || clientY > bounds.bottom) +- return; +- } + const args = getMouseArgsForPosition(canvas, clientX, clientY, ev); + downPosition.current = args.location; + if (args.isTouch) { diff --git a/frontend/patches/README.md b/frontend/patches/README.md new file mode 100644 index 00000000..7fbf2f66 --- /dev/null +++ b/frontend/patches/README.md @@ -0,0 +1,30 @@ +# Patches + +Local fixes to third-party packages, applied by pnpm through the +`patchedDependencies` map in `pnpm-workspace.yaml`. A version bump makes +`pnpm install` fail until the patch is regenerated: + +```sh +pnpm patch @ # edit the printed directory, then: +pnpm patch-commit +``` + +The package's `exports` map resolves to `dist/`, so `src/` edits are inert. +Patch both `dist/cjs` and `dist/esm`. + +## `@glideapps/glide-data-grid` + +Pressing a scrollbar starts a cell drag, which autoscrolls the grid along the +other axis. The grid's mousedown guard rejects presses past the scroller's +border box, but scrollbars sit inside it, between the client box and the border +box, so a press on one is never rejected. The patch measures the client box. + +Still present on upstream `main`, and reported there as +[#1034](https://github.com/glideapps/glide-data-grid/issues/1034), which reaches +the same cause from the other end: the press lands on whatever sits under the +scrollbar. Drop the patch once that is fixed. + +The patch only recognises scrollbars that take layout space. Overlay scrollbars +sit over the client box, so a press on one looks exactly like a press on a cell +and still gets through. Covered by +`e2e/tests/app/table/scrollbar-drag.spec.ts`. diff --git a/frontend/pnpm-lock.yaml b/frontend/pnpm-lock.yaml index baed7d96..3707afd1 100644 --- a/frontend/pnpm-lock.yaml +++ b/frontend/pnpm-lock.yaml @@ -167,6 +167,11 @@ catalogs: specifier: 2.6.3 version: 2.6.3 +patchedDependencies: + '@glideapps/glide-data-grid@6.0.3': + hash: 47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324 + path: patches/@glideapps__glide-data-grid@6.0.3.patch + importers: .: @@ -194,7 +199,7 @@ importers: version: link:../../packages/ui '@glideapps/glide-data-grid': specifier: catalog:components - version: 6.0.3(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) + version: 6.0.3(patch_hash=47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) '@glideapps/glide-data-grid-cells': specifier: catalog:components version: 6.0.3(@types/react@18.2.45)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) @@ -306,7 +311,7 @@ importers: version: link:../../packages/ui '@glideapps/glide-data-grid': specifier: catalog:components - version: 6.0.3(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) + version: 6.0.3(patch_hash=47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) '@glideapps/glide-data-grid-cells': specifier: catalog:components version: 6.0.3(@types/react@18.2.45)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) @@ -555,7 +560,7 @@ importers: version: link:../config '@glideapps/glide-data-grid': specifier: catalog:components - version: 6.0.3(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) + version: 6.0.3(patch_hash=47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) '@glideapps/glide-data-grid-cells': specifier: catalog:components version: 6.0.3(@types/react@18.2.45)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) @@ -4421,7 +4426,7 @@ snapshots: '@glideapps/glide-data-grid-cells@6.0.3(@types/react@18.2.45)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0)': dependencies: - '@glideapps/glide-data-grid': 6.0.3(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) + '@glideapps/glide-data-grid': 6.0.3(patch_hash=47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0) '@linaria/react': 4.5.4(react@18.2.0) '@toast-ui/editor': 3.1.10 '@toast-ui/react-editor': 3.1.10(react@18.2.0) @@ -4435,7 +4440,7 @@ snapshots: - react-responsive-carousel - supports-color - '@glideapps/glide-data-grid@6.0.3(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0)': + '@glideapps/glide-data-grid@6.0.3(patch_hash=47a75ec9ba71a108ae1897c5759252d01efe6d55dfe0152a37e4b8379a288324)(lodash@4.17.23)(marked@14.0.0)(react-dom@18.2.0(react@18.2.0))(react-responsive-carousel@3.2.23)(react@18.2.0)': dependencies: '@linaria/react': 4.5.4(react@18.2.0) canvas-hypertxt: 1.0.3 diff --git a/frontend/pnpm-workspace.yaml b/frontend/pnpm-workspace.yaml index e5681675..bf8d1c1e 100644 --- a/frontend/pnpm-workspace.yaml +++ b/frontend/pnpm-workspace.yaml @@ -65,3 +65,6 @@ catalogs: '@types/react': 18.2.45 '@types/react-dom': 18.2.18 '@types/react-plotly.js': 2.6.3 + +patchedDependencies: + '@glideapps/glide-data-grid@6.0.3': patches/@glideapps__glide-data-grid@6.0.3.patch From f00d945c103606d09c9522b456834f47319b1bbd Mon Sep 17 00:00:00 2001 From: Cammille Carinan Date: Tue, 28 Jul 2026 16:17:23 +0200 Subject: [PATCH 3/4] chore(frontend/docker): copy patches before installing dependencies --- frontend/apps/app/Dockerfile | 4 +++- frontend/apps/demo/Dockerfile | 4 +++- frontend/apps/site/Dockerfile | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frontend/apps/app/Dockerfile b/frontend/apps/app/Dockerfile index 6c0b228c..a0a4f5ab 100644 --- a/frontend/apps/app/Dockerfile +++ b/frontend/apps/app/Dockerfile @@ -12,8 +12,10 @@ ARG APP_DIR RUN corepack enable WORKDIR /usr/src/app -# Prepare workspace +# Prepare workspace. pnpm hashes every file named by patchedDependencies before +# it resolves anything, so patches/ must land before the source tree does. COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ +COPY patches/ patches/ COPY ${APP_DIR}/package.json ${APP_DIR}/ COPY packages/ui/package.json packages/ui/ COPY packages/config/package.json packages/config/ diff --git a/frontend/apps/demo/Dockerfile b/frontend/apps/demo/Dockerfile index 5671f9f4..9524c8cf 100644 --- a/frontend/apps/demo/Dockerfile +++ b/frontend/apps/demo/Dockerfile @@ -12,8 +12,10 @@ ARG APP_DIR RUN corepack enable WORKDIR /usr/src/app -# Prepare workspace +# Prepare workspace. pnpm hashes every file named by patchedDependencies before +# it resolves anything, so patches/ must land before the source tree does. COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ +COPY patches/ patches/ COPY ${APP_DIR}/package.json ${APP_DIR}/ COPY packages/ui/package.json packages/ui/ COPY packages/config/package.json packages/config/ diff --git a/frontend/apps/site/Dockerfile b/frontend/apps/site/Dockerfile index db8c9fdb..fc1150c4 100644 --- a/frontend/apps/site/Dockerfile +++ b/frontend/apps/site/Dockerfile @@ -12,8 +12,10 @@ ARG APP_DIR RUN corepack enable WORKDIR /usr/src/app -# Prepare workspace +# Prepare workspace. pnpm hashes every file named by patchedDependencies before +# it resolves anything, so patches/ must land before the source tree does. COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./ +COPY patches/ patches/ COPY ${APP_DIR}/package.json ${APP_DIR}/ COPY packages/ui/package.json packages/ui/ COPY packages/config/package.json packages/config/ From 1ebda35a17df7f739d4d888a923a0761a43b163d Mon Sep 17 00:00:00 2001 From: Cammille Carinan Date: Tue, 28 Jul 2026 16:17:30 +0200 Subject: [PATCH 4/4] test(frontend/e2e): cover scrollbar drags not scrolling the grid --- frontend/e2e/support/grid.ts | 57 +++++++++++++++- .../tests/app/table/scrollbar-drag.spec.ts | 66 +++++++++++++++++++ frontend/e2e/tsconfig.json | 3 + 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 frontend/e2e/tests/app/table/scrollbar-drag.spec.ts diff --git a/frontend/e2e/support/grid.ts b/frontend/e2e/support/grid.ts index 8ab5ca80..3b609e44 100644 --- a/frontend/e2e/support/grid.ts +++ b/frontend/e2e/support/grid.ts @@ -1,4 +1,4 @@ -import { expect, type Page } from '@playwright/test' +import { expect, type Locator, type Page } from '@playwright/test' // The grid is a Glide . Actions that must reach Glide's canvas mouse // handler (hover, context menu, cell/column selection, activation) are driven by @@ -19,6 +19,7 @@ export const ROW_HEIGHT = 34 export type Box = { x: number; y: number; width: number; height: number } export type Cell = { col: number; row: number } +export type Point = { x: number; y: number } // Opening a plot switches to the Plots tab, which unmounts the table, so the // canvas may be absent when a later grid action runs. Wait for it before reading @@ -56,3 +57,57 @@ export function cellPoint(box: Box, { col, row }: Cell) { y: box.y + HEADER_HEIGHT + row * ROW_HEIGHT + ROW_HEIGHT / 2, } } + +export type Axis = 'horizontal' | 'vertical' + +// The element that carries the grid's scrollbars, which occupy the strip +// between its client box and its border box. One owner: the class tracks Glide. +export function gridScroller(page: Page): Locator { + return page.locator('.dvn-scroller') +} + +// The grid mounts its scroller before the padders give it a scroll range, so an +// early read can land on a grid that cannot scroll yet. Poll this. +export async function canScroll(page: Page, axis: Axis): Promise { + const track = await readTrack(gridScroller(page), axis) + return track.scroll > track.client +} + +// The centre of a scrollbar thumb, in page coordinates, ready for page.mouse. +export async function scrollbarThumb(page: Page, axis: Axis): Promise { + const scroller = gridScroller(page) + await expect(scroller).toBeVisible() + await expect.poll(() => canScroll(page, axis)).toBe(true) + + const track = await readTrack(scroller, axis) + if (track.thickness === 0) { + throw new Error( + `the ${axis} scrollbar takes no layout space, so it cannot be pressed; launch the browser without --hide-scrollbars` + ) + } + + const length = (track.client * track.client) / track.scroll + const along = (track.offset / track.scroll) * track.client + length / 2 + return axis === 'horizontal' + ? { x: track.x + along, y: track.y + track.height - track.thickness / 2 } + : { x: track.x + track.width - track.thickness / 2, y: track.y + along } +} + +function readTrack(scroller: Locator, axis: Axis) { + return scroller.evaluate((element: HTMLElement, along: Axis) => { + const sideways = along === 'horizontal' + const { x, y, width, height } = element.getBoundingClientRect() + return { + x, + y, + width, + height, + thickness: sideways + ? element.offsetHeight - element.clientHeight + : element.offsetWidth - element.clientWidth, + client: sideways ? element.clientWidth : element.clientHeight, + scroll: sideways ? element.scrollWidth : element.scrollHeight, + offset: sideways ? element.scrollLeft : element.scrollTop, + } + }, axis) +} diff --git a/frontend/e2e/tests/app/table/scrollbar-drag.spec.ts b/frontend/e2e/tests/app/table/scrollbar-drag.spec.ts new file mode 100644 index 00000000..03bb413a --- /dev/null +++ b/frontend/e2e/tests/app/table/scrollbar-drag.spec.ts @@ -0,0 +1,66 @@ +import { type Page } from '@playwright/test' + +import { test, expect } from '#fixtures' +import { + canScroll, + gridScroller, + scrollbarThumb, + type Point, +} from '#support/grid' +import { openProposal } from '#support/table' + +// Small enough that the example's columns overflow sideways and its rows +// overflow downwards, so the grid has both scrollbars and somewhere to scroll. +test.use({ + viewport: { width: 900, height: 400 }, + // Headless Chromium hides scrollbars by default, leaving only overlay ones, + // which take no layout space and so cannot be pressed. + launchOptions: { ignoreDefaultArgs: ['--hide-scrollbars'] }, +}) + +test.beforeEach(async ({ page, example }) => { + await openProposal(page, example) + + // Dragging one scrollbar only proves something about the other axis if the + // grid could have scrolled that way in the first place. + await expect.poll(() => canScroll(page, 'horizontal')).toBe(true) + await expect.poll(() => canScroll(page, 'vertical')).toBe(true) +}) + +test('dragging along the horizontal scrollbar leaves the vertical scroll alone', async ({ + page, +}) => { + const scroller = gridScroller(page) + + await dragThumb(page, { + from: await scrollbarThumb(page, 'horizontal'), + by: { x: 150, y: 0 }, + }) + + await expect(scroller).not.toHaveJSProperty('scrollLeft', 0) + await expect(scroller).toHaveJSProperty('scrollTop', 0) +}) + +test('dragging along the vertical scrollbar leaves the horizontal scroll alone', async ({ + page, +}) => { + const scroller = gridScroller(page) + + await dragThumb(page, { + from: await scrollbarThumb(page, 'vertical'), + by: { x: 0, y: 60 }, + }) + + await expect(scroller).not.toHaveJSProperty('scrollTop', 0) + await expect(scroller).toHaveJSProperty('scrollLeft', 0) +}) + +// Glide autoscrolls a drag that strays outside the grid, and speeds up the +// longer the button is held. Hold before releasing so a regression shows. +async function dragThumb(page: Page, { from, by }: { from: Point; by: Point }) { + await page.mouse.move(from.x, from.y) + await page.mouse.down() + await page.mouse.move(from.x + by.x, from.y + by.y, { steps: 10 }) + await page.waitForTimeout(300) + await page.mouse.up() +} diff --git a/frontend/e2e/tsconfig.json b/frontend/e2e/tsconfig.json index 639f3d52..b503383b 100644 --- a/frontend/e2e/tsconfig.json +++ b/frontend/e2e/tsconfig.json @@ -2,6 +2,9 @@ "extends": "@damnit-frontend/config/tsconfig/node", "compilerOptions": { "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.tsbuildinfo", + // Specs run in node, but page.evaluate bodies run in the browser and need + // to name DOM types. + "lib": ["ES2023", "DOM"], "types": ["node", "@playwright/test"] }, "include": [