Skip to content

Commit e42f3d1

Browse files
JohnMcLearclaude
andcommitted
fix: page down/up now scrolls by viewport height, not line count
The previous implementation counted logical lines in the viewport, which failed when long wrapped lines consumed the entire viewport. Now scrolls by actual pixel height for correct behavior. Fixes #4562 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4137109 commit e42f3d1

1 file changed

Lines changed: 34 additions & 38 deletions

File tree

src/static/js/ace2_inner.ts

Lines changed: 34 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,51 +2879,47 @@ function Ace2Inner(editorInfo, cssManagers) {
28792879
// This is required, browsers will try to do normal default behavior on
28802880
// page up / down and the default behavior SUCKS
28812881
evt.preventDefault();
2882-
const oldVisibleLineRange = scroll.getVisibleLineRange(rep);
2883-
let topOffset = rep.selStart[0] - oldVisibleLineRange[0];
2884-
if (topOffset < 0) {
2885-
topOffset = 0;
2886-
}
28872882

28882883
const isPageDown = evt.which === 34;
28892884
const isPageUp = evt.which === 33;
28902885

2891-
scheduler.setTimeout(() => {
2892-
// the visible lines IE 1,10
2893-
const newVisibleLineRange = scroll.getVisibleLineRange(rep);
2894-
// total count of lines in pad IE 10
2895-
const linesCount = rep.lines.length();
2896-
// How many lines are in the viewport right now?
2897-
const numberOfLinesInViewport = newVisibleLineRange[1] - newVisibleLineRange[0];
2898-
2899-
if (isPageUp && padShortcutEnabled.pageUp) {
2900-
rep.selStart[0] -= numberOfLinesInViewport;
2901-
rep.selEnd[0] -= numberOfLinesInViewport;
2886+
if ((isPageDown && padShortcutEnabled.pageDown) ||
2887+
(isPageUp && padShortcutEnabled.pageUp)) {
2888+
// Scroll by actual viewport height in pixels, not by line count.
2889+
// This fixes the case where very long wrapped lines consume the
2890+
// entire viewport, making line-count-based scrolling useless.
2891+
const viewportHeight = outerWin.document.documentElement.clientHeight;
2892+
// Keep a small overlap so the user doesn't lose context
2893+
const scrollAmount = viewportHeight - 40;
2894+
const currentScrollY = scroll.getScrollY();
2895+
2896+
if (isPageDown) {
2897+
scroll.setScrollY(currentScrollY + scrollAmount);
2898+
} else {
2899+
scroll.setScrollY(Math.max(0, currentScrollY - scrollAmount));
29022900
}
29032901

2904-
if (isPageDown && padShortcutEnabled.pageDown) {
2905-
rep.selStart[0] += numberOfLinesInViewport;
2906-
rep.selEnd[0] += numberOfLinesInViewport;
2907-
}
2902+
// Move cursor into the new visible area
2903+
scheduler.setTimeout(() => {
2904+
const linesCount = rep.lines.length();
2905+
const newVisibleRange = scroll.getVisibleLineRange(rep);
29082906

2909-
// clamp to valid line range
2910-
rep.selStart[0] = Math.max(0, Math.min(rep.selStart[0], linesCount - 1));
2911-
rep.selEnd[0] = Math.max(0, Math.min(rep.selEnd[0], linesCount - 1));
2912-
updateBrowserSelectionFromRep();
2913-
// get the current caret selection, can't use rep. here because that only gives
2914-
// us the start position not the current
2915-
const myselection = targetDoc.getSelection();
2916-
// get the carets selection offset in px IE 214
2917-
let caretOffsetTop = myselection.focusNode.parentNode.offsetTop ||
2918-
myselection.focusNode.offsetTop;
2919-
2920-
// sometimes the first selection is -1 which causes problems
2921-
// (Especially with ep_page_view)
2922-
// so use focusNode.offsetTop value.
2923-
if (caretOffsetTop === -1) caretOffsetTop = myselection.focusNode.offsetTop;
2924-
// set the scrollY offset of the viewport on the document
2925-
scroll.setScrollY(caretOffsetTop);
2926-
}, 200);
2907+
if (isPageDown) {
2908+
// Place cursor at the first line of the new viewport
2909+
rep.selStart[0] = newVisibleRange[0];
2910+
rep.selEnd[0] = newVisibleRange[0];
2911+
} else {
2912+
// Place cursor at the last line of the new viewport
2913+
rep.selEnd[0] = newVisibleRange[1];
2914+
rep.selStart[0] = newVisibleRange[1];
2915+
}
2916+
2917+
// clamp to valid line range
2918+
rep.selStart[0] = Math.max(0, Math.min(rep.selStart[0], linesCount - 1));
2919+
rep.selEnd[0] = Math.max(0, Math.min(rep.selEnd[0], linesCount - 1));
2920+
updateBrowserSelectionFromRep();
2921+
}, 0);
2922+
}
29272923
}
29282924
}
29292925

0 commit comments

Comments
 (0)