fix(app): prevent scrollend position rollback
Discard virtualizer measurement corrections suppressed during an active user gesture instead of replaying stale offsets at scrollend. Preserve post-scroll live anchoring and add unit plus Electron coverage for ordinary scrolling without updates.
This commit is contained in:
@@ -1,13 +1,8 @@
|
||||
export function createSessionTimelineScrollPolicy({ isUserScrolling, writeScroll }) {
|
||||
let explicitDepth = 0;
|
||||
let deferredAdjustment = 0;
|
||||
let deferredInstance = null;
|
||||
|
||||
function scrollToFn(offset, options = {}, instance) {
|
||||
if (explicitDepth === 0 && isUserScrolling()) {
|
||||
const adjustment = Number(options.adjustments);
|
||||
if (Number.isFinite(adjustment)) deferredAdjustment += adjustment;
|
||||
deferredInstance = instance;
|
||||
return;
|
||||
}
|
||||
writeScroll(offset, options, instance);
|
||||
@@ -22,19 +17,8 @@ export function createSessionTimelineScrollPolicy({ isUserScrolling, writeScroll
|
||||
}
|
||||
}
|
||||
|
||||
function flushDeferredAdjustment(instance = deferredInstance) {
|
||||
if (isUserScrolling() || deferredAdjustment === 0 || !instance?.scrollElement) return false;
|
||||
const adjustment = deferredAdjustment;
|
||||
deferredAdjustment = 0;
|
||||
deferredInstance = null;
|
||||
const offset = Number(instance.scrollElement.scrollTop) || 0;
|
||||
writeScroll(offset, { behavior: 'auto', adjustments: adjustment }, instance);
|
||||
return true;
|
||||
}
|
||||
|
||||
return {
|
||||
scrollToFn,
|
||||
runExplicit,
|
||||
flushDeferredAdjustment,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -133,10 +133,6 @@ export function useSessionTimelineViewport({
|
||||
tailFollowReady.value = true;
|
||||
}
|
||||
|
||||
function settleUserScroll() {
|
||||
return scrollPolicy.flushDeferredAdjustment(virtualizer.value);
|
||||
}
|
||||
|
||||
return {
|
||||
virtualRows,
|
||||
totalSize,
|
||||
@@ -145,7 +141,6 @@ export function useSessionTimelineViewport({
|
||||
scrollToIndex,
|
||||
scrollToEnd,
|
||||
isFollowingTail,
|
||||
settleUserScroll,
|
||||
resetForInitialSnapshot,
|
||||
completeInitialSnapshot,
|
||||
};
|
||||
|
||||
@@ -63,7 +63,6 @@ const liveReloadCoordinator = createSessionLiveReloadCoordinator({
|
||||
});
|
||||
|
||||
function handleUserScrollEnd() {
|
||||
timelineViewport.settleUserScroll();
|
||||
if (active.value) void liveReloadCoordinator.flush();
|
||||
}
|
||||
|
||||
|
||||
@@ -323,6 +323,45 @@ async function run() {
|
||||
assert(disclosure.unmounted, 'the expanded tool row unmounts outside overscan');
|
||||
assert(disclosure.restored, 'disclosure state survives unmount and remount');
|
||||
|
||||
const passiveScrollSettlement = await win.webContents.executeJavaScript(`new Promise(resolve => {
|
||||
const wrap = document.querySelector('.detail-wrap');
|
||||
const originalScrollTo = wrap.scrollTo.bind(wrap);
|
||||
const blockAutomaticScrollEnd = event => event.stopImmediatePropagation();
|
||||
let phase = 'scrolling';
|
||||
let postScrollEndWrites = 0;
|
||||
wrap.addEventListener('scrollend', blockAutomaticScrollEnd, true);
|
||||
wrap.scrollTo = (...args) => {
|
||||
if (phase === 'settled') postScrollEndWrites++;
|
||||
return originalScrollTo(...args);
|
||||
};
|
||||
wrap.dispatchEvent(new WheelEvent('wheel', { deltaY: 70, bubbles: true }));
|
||||
const startedAt = performance.now();
|
||||
function frame(now) {
|
||||
wrap.scrollTop += 55;
|
||||
if (now - startedAt < 600) {
|
||||
requestAnimationFrame(frame);
|
||||
return;
|
||||
}
|
||||
const beforeScrollEnd = wrap.scrollTop;
|
||||
phase = 'settled';
|
||||
wrap.removeEventListener('scrollend', blockAutomaticScrollEnd, true);
|
||||
wrap.dispatchEvent(new Event('scrollend'));
|
||||
setTimeout(() => {
|
||||
const afterScrollEnd = wrap.scrollTop;
|
||||
wrap.scrollTo = originalScrollTo;
|
||||
resolve({ beforeScrollEnd, afterScrollEnd, postScrollEndWrites });
|
||||
}, 120);
|
||||
}
|
||||
requestAnimationFrame(frame);
|
||||
})`, true);
|
||||
assert(
|
||||
passiveScrollSettlement.postScrollEndWrites === 0
|
||||
&& Math.abs(passiveScrollSettlement.afterScrollEnd - passiveScrollSettlement.beforeScrollEnd) < 1,
|
||||
`ordinary scrolling settles without rollback (${JSON.stringify(passiveScrollSettlement)})`,
|
||||
);
|
||||
await win.webContents.executeJavaScript(`document.querySelector('button[title="First"]')?.click()`, true);
|
||||
await delay(350);
|
||||
|
||||
await win.webContents.executeJavaScript(`window.location.hash = '#/sessions'`, true);
|
||||
await waitFor(win.webContents, `!document.querySelector('.virtual-timeline')`, 'session detail deactivation');
|
||||
await win.webContents.executeJavaScript(`(async () => {
|
||||
|
||||
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
||||
|
||||
import { createSessionTimelineScrollPolicy } from '../app/src/renderer/src/session-timeline-scroll-policy.mjs';
|
||||
|
||||
test('virtualizer scroll writes are deferred throughout a user scroll', () => {
|
||||
test('virtualizer scroll writes are discarded throughout a user scroll', () => {
|
||||
let scrolling = true;
|
||||
const element = { scrollTop: 100 };
|
||||
const writes = [];
|
||||
@@ -21,12 +21,13 @@ test('virtualizer scroll writes are deferred throughout a user scroll', () => {
|
||||
assert.deepEqual(writes, [], 'momentum is never interrupted by a programmatic write');
|
||||
|
||||
scrolling = false;
|
||||
policy.flushDeferredAdjustment(instance);
|
||||
assert.deepEqual(writes, [{ offset: 100, behavior: 'auto', adjustments: 24 }]);
|
||||
assert.equal(element.scrollTop, 124, 'the accumulated correction restores the reader anchor once');
|
||||
|
||||
policy.flushDeferredAdjustment(instance);
|
||||
assert.equal(writes.length, 1, 'settlement is idempotent');
|
||||
assert.equal(element.scrollTop, 100, 'scrollend never replays a suppressed correction');
|
||||
policy.scrollToFn(100, { behavior: 'auto', adjustments: 8 }, instance);
|
||||
assert.deepEqual(
|
||||
writes,
|
||||
[{ offset: 100, behavior: 'auto', adjustments: 8 }],
|
||||
'new corrections produced after scrollend remain available for live commits',
|
||||
);
|
||||
});
|
||||
|
||||
test('explicit UUID and pagination navigation can bypass the user-scroll guard', () => {
|
||||
@@ -44,3 +45,24 @@ test('explicit UUID and pagination navigation can bypass the user-scroll guard',
|
||||
|
||||
assert.deepEqual(writes, [{ offset: 720, behavior: 'auto' }]);
|
||||
});
|
||||
|
||||
test('settlement never replays a measurement correction from before the latest user scroll', () => {
|
||||
let scrolling = true;
|
||||
const element = { scrollTop: 100 };
|
||||
const writes = [];
|
||||
const instance = { scrollElement: element };
|
||||
const policy = createSessionTimelineScrollPolicy({
|
||||
isUserScrolling: () => scrolling,
|
||||
writeScroll: (offset, options) => {
|
||||
writes.push({ offset, ...options });
|
||||
element.scrollTop = offset + (options.adjustments || 0);
|
||||
},
|
||||
});
|
||||
|
||||
policy.scrollToFn(100, { behavior: 'auto', adjustments: -24 }, instance);
|
||||
element.scrollTop = 300;
|
||||
scrolling = false;
|
||||
|
||||
assert.deepEqual(writes, [], 'the completed gesture owns its final scroll position');
|
||||
assert.equal(element.scrollTop, 300, 'scrollend must not roll the viewport backward');
|
||||
});
|
||||
|
||||
@@ -49,7 +49,7 @@ test('timeline viewport owns measurement and anchoring while SessionDetail alone
|
||||
assert.match(viewportModule, /scrollToFn:\s*scrollPolicy\.scrollToFn/);
|
||||
assert.match(viewportModule, /useScrollendEvent:\s*true/);
|
||||
assert.match(viewportModule, /isScrollingResetDelay:\s*450/);
|
||||
assert.match(viewportModule, /settleUserScroll/);
|
||||
assert.doesNotMatch(viewportModule + sessionDetail, /settleUserScroll|flushDeferredAdjustment/);
|
||||
assert.match(viewportModule, /useAnimationFrameWithResizeObserver:\s*true/);
|
||||
assert.match(viewportModule, /scrollPaddingEnd/);
|
||||
assert.match(viewportModule, /scrollToIndex/);
|
||||
|
||||
Reference in New Issue
Block a user