From 786af022e3bcaeb5b38824616e154bfeb7f2a761 Mon Sep 17 00:00:00 2001 From: ubermanu <1533514+ubermanu@users.noreply.github.com> Date: Mon, 18 May 2026 19:31:39 +0200 Subject: [PATCH 1/2] refactor: replace scrollOnLinks with drag-to-activate threshold --- .changeset/strict-schools-find.md | 5 + extensions/roller/e2e/scroll.test.ts | 88 +++---- extensions/roller/src/Roller.ts | 215 ++++++++++++------ .../roller/src/components/OptionsPage.svelte | 8 - extensions/roller/src/defaultOptions.ts | 1 - extensions/roller/src/helpers/scroll.ts | 33 --- extensions/roller/src/types.ts | 1 - 7 files changed, 200 insertions(+), 151 deletions(-) create mode 100644 .changeset/strict-schools-find.md diff --git a/.changeset/strict-schools-find.md b/.changeset/strict-schools-find.md new file mode 100644 index 0000000..f1fd678 --- /dev/null +++ b/.changeset/strict-schools-find.md @@ -0,0 +1,5 @@ +--- +@ubermanu/roller: minor +--- + +Replace `scrollOnLinks` option with drag-to-activate scrolling. diff --git a/extensions/roller/e2e/scroll.test.ts b/extensions/roller/e2e/scroll.test.ts index 19d726a..482e9ba 100644 --- a/extensions/roller/e2e/scroll.test.ts +++ b/extensions/roller/e2e/scroll.test.ts @@ -188,7 +188,6 @@ it('should NOT trigger sticky scrolling when stickyScroll is disabled', async () moveSpeed: 10, stickyScroll: false, innerScroll: true, - scrollOnLinks: true, sameSpeed: false, capSpeed: 10, shouldCap: false, @@ -229,7 +228,7 @@ it('should NOT trigger sticky scrolling when stickyScroll is disabled', async () expect(scrollY).toBeLessThan(5) }) -it('should NOT trigger scroll on links when scrollOnLinks is disabled, but should when enabled', async () => { +it('should trigger scrolling when dragging on a link past the drag threshold', async () => { // Reset all roller options to defaults await extensionRealm.evaluate(() => { Object.assign(window.roller.options, { @@ -238,7 +237,6 @@ it('should NOT trigger scroll on links when scrollOnLinks is disabled, but shoul moveSpeed: 10, stickyScroll: true, innerScroll: true, - scrollOnLinks: true, sameSpeed: false, capSpeed: 10, shouldCap: false, @@ -249,85 +247,87 @@ it('should NOT trigger scroll on links when scrollOnLinks is disabled, but shoul }) await new Promise((r) => setTimeout(r, 200)) - // Set up a page with a link and tall body for scrolling + // Set up a page with a link and tall body await page.evaluate(() => { document.body.innerHTML = ` Click me
` }) - // Wait a moment for page to settle await new Promise((r) => setTimeout(r, 200)) - // Get the link coordinates const linkRect = await page.evaluate(() => { const link = document.getElementById('test-link') const rect = link!.getBoundingClientRect() - return { - x: rect.left + rect.width / 2, - y: rect.top + rect.height / 2, - } + return { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 } }) - // Reset scroll and assert await page.evaluate(() => window.scrollTo(0, 0)) let scrollY = await page.evaluate(() => window.scrollY) expect(scrollY).toBe(0) - // Test with scrollOnLinks: false - await extensionRealm.evaluate(() => { - window.roller.options.scrollOnLinks = false - }) - await new Promise((r) => setTimeout(r, 200)) - - // Middle-click on the link + // Middle-click on the link and drag down past the threshold while holding await page.mouse.move(linkRect.x, linkRect.y) await page.mouse.down({ button: 'middle' }) - await new Promise((r) => setTimeout(r, 100)) - await page.mouse.up({ button: 'middle' }) - await new Promise((r) => setTimeout(r, 100)) - - // Move mouse down - await page.mouse.move(linkRect.x, linkRect.y + 100) + await page.mouse.move(linkRect.x, linkRect.y + 50) await new Promise((r) => setTimeout(r, 1500)) - // Read scrollY scrollY = await page.evaluate(() => window.scrollY) // Stop scrolling + await page.mouse.up({ button: 'middle' }) await page.mouse.down({ button: 'left' }) await page.mouse.up({ button: 'left' }) - // Assert scroll did NOT occur - expect(scrollY).toBeLessThan(5) + expect(scrollY).toBeGreaterThan(10) +}) - // Now test with scrollOnLinks: true +it('should NOT trigger scrolling when clicking a link without dragging past the threshold', async () => { + // Reset all roller options to defaults await extensionRealm.evaluate(() => { - window.roller.options.scrollOnLinks = true + Object.assign(window.roller.options, { + dragThreshold: 10, + moveThreshold: 10, + moveSpeed: 10, + stickyScroll: true, + innerScroll: true, + sameSpeed: false, + capSpeed: 10, + shouldCap: false, + ctrlClick: false, + middleClick: true, + disableOnWindows: true, + }) }) await new Promise((r) => setTimeout(r, 200)) - // Reset scroll back to 0 + await page.evaluate(() => { + document.body.innerHTML = ` + Click me +
+ ` + }) + await new Promise((r) => setTimeout(r, 200)) + + const linkRect = await page.evaluate(() => { + const link = document.getElementById('test-link') + const rect = link!.getBoundingClientRect() + return { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 } + }) + await page.evaluate(() => window.scrollTo(0, 0)) + let scrollY = await page.evaluate(() => window.scrollY) + expect(scrollY).toBe(0) - // Repeat the middle-click on the link + // Middle-click on the link and release immediately without dragging await page.mouse.move(linkRect.x, linkRect.y) await page.mouse.down({ button: 'middle' }) - await new Promise((r) => setTimeout(r, 100)) + await new Promise((r) => setTimeout(r, 50)) await page.mouse.up({ button: 'middle' }) - await new Promise((r) => setTimeout(r, 100)) - // Move mouse down - await page.mouse.move(linkRect.x, linkRect.y + 100) - await new Promise((r) => setTimeout(r, 1500)) + // Wait to confirm no scrolling starts + await new Promise((r) => setTimeout(r, 1000)) - // Read scrollY again scrollY = await page.evaluate(() => window.scrollY) - - // Stop scrolling - await page.mouse.down({ button: 'left' }) - await page.mouse.up({ button: 'left' }) - - // Assert scroll DID occur - expect(scrollY).toBeGreaterThan(10) + expect(scrollY).toBeLessThan(5) }) diff --git a/extensions/roller/src/Roller.ts b/extensions/roller/src/Roller.ts index 19f7ccf..9b69351 100644 --- a/extensions/roller/src/Roller.ts +++ b/extensions/roller/src/Roller.ts @@ -1,11 +1,6 @@ import Overlay from './components/Overlay' import defaultOptions from './defaultOptions' -import { - findScroll, - findScrollTop, - getDocumentContext, - isScrollable, -} from './helpers/scroll' +import { findScroll, findScrollTop, getDocumentContext } from './helpers/scroll' import * as utils from './helpers/utils' import type { RollerOptions, ScrollResult } from './types' @@ -38,6 +33,16 @@ export default class Roller { iframeOldX: number | null iframeOldY: number | null + // Pending (pre-threshold) state + pendingX: number | null + pendingY: number | null + pendingTarget: HTMLElement | null + pendingButton: number + pendingCtrl: boolean + pendingMeta: boolean + handlePendingMouseMove: ((e: MouseEvent) => void) | null + handlePendingMouseUp: ((e: MouseEvent) => void) | null + htmlNode: HTMLElement bodyNode: HTMLElement htmlScrollBehavior: string @@ -64,6 +69,15 @@ export default class Roller { this.iframeOldX = null this.iframeOldY = null + this.pendingX = null + this.pendingY = null + this.pendingTarget = null + this.pendingButton = 0 + this.pendingCtrl = false + this.pendingMeta = false + this.handlePendingMouseMove = null + this.handlePendingMouseUp = null + const { htmlNode, bodyNode } = getDocumentContext() this.htmlNode = htmlNode this.bodyNode = bodyNode @@ -221,6 +235,23 @@ export default class Roller { this.bodyNode.style.setProperty('scroll-behavior', this.bodyScrollBehavior) } + clearPending(): void { + if (this.handlePendingMouseMove) { + removeEventListener('mousemove', this.handlePendingMouseMove, true) + this.handlePendingMouseMove = null + } + if (this.handlePendingMouseUp) { + removeEventListener('mouseup', this.handlePendingMouseUp, true) + this.handlePendingMouseUp = null + } + this.pendingX = null + this.pendingY = null + this.pendingTarget = null + this.pendingButton = 0 + this.pendingCtrl = false + this.pendingMeta = false + } + start(o: ScrollResult, x: number, y: number): void { this.scrolling = true this.oldX = x @@ -374,76 +405,132 @@ export default class Roller { handleMouseDown(event: MouseEvent): void { if (this.scrolling) { utils.stopEvent(event, true) - } else { - const path = event.composedPath() - const target = path.find((node) => (node as Node).nodeType === 1) as - | HTMLElement - | null - | undefined - - if ( - target != null && - target.localName !== 'iframe' && - ((event.button === 1 && this.options.middleClick) || - (event.button === 0 && - (event.ctrlKey || event.metaKey) && - this.options.ctrlClick)) && - event.clientX < this.htmlNode.clientWidth && - event.clientY < this.htmlNode.clientHeight && - (this.options.scrollOnLinks || isScrollable(target)) - ) { - const elem: ScrollResult | null = - this.isInIframe && !this.options.innerScroll - ? null - : findScroll(target, this.options.innerScroll) - if (elem !== null) { - utils.stopEvent(event, true) - this.start(elem, event.clientX, event.clientY) - } else if (this.isInIframe && this.options.innerScroll) { - utils.stopEvent(event, true) - window.parent.postMessage( - { - type: 'roller-scroll', - action: 'start', - clientX: event.clientX, - clientY: event.clientY, - frameId: this.frameId, - }, - '*' - ) - this.iframeOldX = event.clientX - this.iframeOldY = event.clientY - - const handleIframeMouseMove = (e: MouseEvent): void => { - window.parent.postMessage( - { - type: 'roller-scroll', - action: 'move', - clientX: e.clientX, - clientY: e.clientY, - frameId: this.frameId, - }, - '*' - ) - } + return + } - const handleIframeMouseUp = (): void => { + const path = event.composedPath() + const target = path.find((node) => (node as Node).nodeType === 1) as + | HTMLElement + | null + | undefined + + if ( + target != null && + target.localName !== 'iframe' && + ((event.button === 1 && this.options.middleClick) || + (event.button === 0 && + (event.ctrlKey || event.metaKey) && + this.options.ctrlClick)) && + event.clientX < this.htmlNode.clientWidth && + event.clientY < this.htmlNode.clientHeight + ) { + // Prevent default immediately so links don't navigate before we know + // whether the user is dragging or just clicking. + event.preventDefault() + + const pendingX = event.clientX + const pendingY = event.clientY + const pendingButton = event.button + const pendingCtrl = event.ctrlKey + const pendingMeta = event.metaKey + + this.pendingX = pendingX + this.pendingY = pendingY + this.pendingTarget = target + this.pendingButton = pendingButton + this.pendingCtrl = pendingCtrl + this.pendingMeta = pendingMeta + + const onMove = (e: MouseEvent): void => { + const dx = e.clientX - pendingX + const dy = e.clientY - pendingY + + if (utils.hypot(dx, dy) > this.options.dragThreshold) { + this.clearPending() + utils.stopEvent(e, true) + + if (this.isInIframe && !this.options.innerScroll) { + // Bubble up to parent frame window.parent.postMessage( { type: 'roller-scroll', - action: 'stop', + action: 'start', + clientX: pendingX, + clientY: pendingY, frameId: this.frameId, }, '*' ) - removeEventListener('mousemove', handleIframeMouseMove, true) - removeEventListener('mouseup', handleIframeMouseUp, true) + this.iframeOldX = pendingX + this.iframeOldY = pendingY + + const handleIframeMouseMove = (ev: MouseEvent): void => { + window.parent.postMessage( + { + type: 'roller-scroll', + action: 'move', + clientX: ev.clientX, + clientY: ev.clientY, + frameId: this.frameId, + }, + '*' + ) + } + + const handleIframeMouseUp = (): void => { + window.parent.postMessage( + { + type: 'roller-scroll', + action: 'stop', + frameId: this.frameId, + }, + '*' + ) + removeEventListener('mousemove', handleIframeMouseMove, true) + removeEventListener('mouseup', handleIframeMouseUp, true) + } + + addEventListener('mousemove', handleIframeMouseMove, true) + addEventListener('mouseup', handleIframeMouseUp, true) + } else { + const elem = findScroll(target, this.options.innerScroll) + if (elem !== null) { + this.start(elem, pendingX, pendingY) + } } + } + } - addEventListener('mousemove', handleIframeMouseMove, true) - addEventListener('mouseup', handleIframeMouseUp, true) + const onUp = (e: MouseEvent): void => { + this.clearPending() + // User released without crossing the drag threshold — synthesize the + // native action that preventDefault() suppressed. + const link = + (e.target as HTMLElement | null)?.closest?.('a[href]') ?? + (target.localName === 'a' && (target as HTMLAnchorElement).href + ? target + : null) + if (link || pendingButton === 1) { + // Fire a synthetic click so links still work + target.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + cancelable: true, + button: pendingButton, + ctrlKey: pendingCtrl, + metaKey: pendingMeta, + clientX: pendingX, + clientY: pendingY, + }) + ) } } + + this.handlePendingMouseMove = onMove + this.handlePendingMouseUp = onUp + + addEventListener('mousemove', onMove, true) + addEventListener('mouseup', onUp, true) } } } diff --git a/extensions/roller/src/components/OptionsPage.svelte b/extensions/roller/src/components/OptionsPage.svelte index 992d6bc..2f76636 100644 --- a/extensions/roller/src/components/OptionsPage.svelte +++ b/extensions/roller/src/components/OptionsPage.svelte @@ -118,14 +118,6 @@ -
-
- -
-