fix(page-controller): apply scroll direction to pixels parameter
Two bugs in the scroll direction logic: 1. Vertical scroll with `pixels` ignores the `down` boolean because the `??` operator bypasses the direction multiplier when pixels is provided. Fix: move the direction multiplier outside the `??` so it applies to both the pixels and numPages paths. 2. Horizontal scroll with `pixels` applies direction twice - once in PageController.ts and again in actions.ts - causing a double negation that reverses the intended direction. Fix: remove the redundant direction logic from actions.ts since PageController already signs the scroll amount. Also removes the now-unused `down` and `right` parameters from the scrollVertically() and scrollHorizontally() action functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -321,11 +321,11 @@ export class PageController extends EventTarget {
|
||||
|
||||
this.assertIndexed()
|
||||
|
||||
const scrollAmount = pixels ?? numPages * (down ? 1 : -1) * window.innerHeight
|
||||
const scrollAmount = (pixels ?? numPages * window.innerHeight) * (down ? 1 : -1)
|
||||
|
||||
const element = index !== undefined ? getElementByIndex(this.selectorMap, index) : null
|
||||
|
||||
const message = await scrollVertically(down, scrollAmount, element)
|
||||
const message = await scrollVertically(scrollAmount, element)
|
||||
|
||||
return {
|
||||
success: true,
|
||||
@@ -356,7 +356,7 @@ export class PageController extends EventTarget {
|
||||
|
||||
const element = index !== undefined ? getElementByIndex(this.selectorMap, index) : null
|
||||
|
||||
const message = await scrollHorizontally(right, scrollAmount, element)
|
||||
const message = await scrollHorizontally(scrollAmount, element)
|
||||
|
||||
return {
|
||||
success: true,
|
||||
|
||||
@@ -231,11 +231,7 @@ export async function scrollIntoViewIfNeeded(element: Element) {
|
||||
}
|
||||
}
|
||||
|
||||
export async function scrollVertically(
|
||||
down: boolean,
|
||||
scroll_amount: number,
|
||||
element?: HTMLElement | null
|
||||
) {
|
||||
export async function scrollVertically(scroll_amount: number, element?: HTMLElement | null) {
|
||||
// Element-specific scrolling if element is provided
|
||||
if (element) {
|
||||
const targetElement = element
|
||||
@@ -359,11 +355,7 @@ export async function scrollVertically(
|
||||
}
|
||||
}
|
||||
|
||||
export async function scrollHorizontally(
|
||||
right: boolean,
|
||||
scroll_amount: number,
|
||||
element?: HTMLElement | null
|
||||
) {
|
||||
export async function scrollHorizontally(scroll_amount: number, element?: HTMLElement | null) {
|
||||
// Element-specific scrolling if element is provided
|
||||
if (element) {
|
||||
const targetElement = element
|
||||
@@ -372,7 +364,7 @@ export async function scrollHorizontally(
|
||||
let scrolledElement: HTMLElement | null = null
|
||||
let scrollDelta = 0
|
||||
let attempts = 0
|
||||
const dx = right ? scroll_amount : -scroll_amount
|
||||
const dx = scroll_amount
|
||||
|
||||
while (currentElement && attempts < 10) {
|
||||
const computedStyle = window.getComputedStyle(currentElement)
|
||||
@@ -420,7 +412,7 @@ export async function scrollHorizontally(
|
||||
|
||||
// Page-level scrolling (default or fallback)
|
||||
|
||||
const dx = right ? scroll_amount : -scroll_amount
|
||||
const dx = scroll_amount
|
||||
const bigEnough = (el: HTMLElement) => el.clientWidth >= window.innerWidth * 0.5
|
||||
const canScroll = (el: HTMLElement | null) =>
|
||||
el &&
|
||||
|
||||
Reference in New Issue
Block a user