Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-jump-and-idle-followups.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
default: patch
---

Keep jump-to-message targets anchored while surrounding history and media previews settle, and avoid aggressive foreground service worker claims after idle Safari PWA resumes.
186 changes: 84 additions & 102 deletions src/app/features/room/RoomTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,7 @@ export function RoomTimeline({
useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout> | undefined;
let retryIntervalId: ReturnType<typeof setInterval> | undefined;
let recenterTimeoutId: ReturnType<typeof setTimeout> | undefined;
let resizeObserver: ResizeObserver | undefined;
const recenterTimeoutIds: ReturnType<typeof setTimeout>[] = [];

if (timelineSync.focusItem) {
// Mark that the jump succeeded (focusItem was set). This prevents recovery
Expand All @@ -520,113 +519,89 @@ export function RoomTimeline({
});

let scrollSucceeded = false;
const attemptScroll = () => {
if (!timelineSync.focusItem?.scrollTo || !vListRef.current || scrollSucceeded) return false;

let processedIndex = getRawIndexToProcessedIndex(timelineSync.focusItem.index);
const resolveProcessedIndex = () => {
const currentFocusItem = timelineSyncRef.current.focusItem;
if (!currentFocusItem) return undefined;

// Fallback: if index lookup fails but we have an eventId, search by ID.
// This handles fragmented timelines from sliding sync where absolute indices
// don't align across different timeline contexts.
if (processedIndex === undefined && timelineSync.focusItem.eventId) {
let nextProcessedIndex = getRawIndexToProcessedIndex(currentFocusItem.index);
if (nextProcessedIndex === undefined && currentFocusItem.eventId) {
const found = processedEventsRef.current.findIndex(
(e) => e.mEvent.getId() === timelineSync.focusItem!.eventId
(e) => e.mEvent.getId() === currentFocusItem.eventId
);
if (found >= 0) {
processedIndex = found;
log.log(
`[PermalinkJump] Found event by ID search: eventId=${timelineSync.focusItem.eventId}, processedIndex=${found}`
);
} else {
log.log(
`[PermalinkJump] Event not found in processedEvents yet: eventId=${timelineSync.focusItem.eventId}, processedEvents.length=${processedEventsRef.current.length}`
);
nextProcessedIndex = found;
}
Comment on lines +546 to 553

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resolve jump anchors by event id before stale indices

When post-jump pagination loads history above the target, the target's absolute itemIndex shifts, but the old raw index can still resolve to a different row. Because this resolver accepts getRawIndexToProcessedIndex(currentFocusItem.index) before checking currentFocusItem.eventId, the delayed 600/1500/3000 ms re-centers can scroll to whatever event now occupies the stale index instead of the jumped message. Prefer validating the matched row's event id or searching by eventId first whenever it is available.

Useful? React with 👍 / 👎.

}

if (processedIndex !== undefined) {
log.log(
`[PermalinkJump] Scroll succeeded: processedIndex=${processedIndex}, eventId=${timelineSync.focusItem.eventId}`
);
Sentry.addBreadcrumb({
category: 'timeline.permalink',
message: 'Scroll succeeded',
level: 'info',
data: {
processedIndex,
eventId: timelineSync.focusItem.eventId,
roomId: room.roomId,
},
});
return nextProcessedIndex;
};

// An event-targeted jump should no longer be treated as bottom-pinned.
// If we leave atBottom=true from the room's previous state, the scroll
// handler can immediately "chase" the live bottom after this jump.
setAtBottom(false);
startJumpScrollBlock();
const attemptScroll = () => {
if (!timelineSync.focusItem?.scrollTo || !vListRef.current || scrollSucceeded) return false;

// Reveal timeline and scroll in the same frame to avoid flash
setIsReady(true);
vListRef.current.scrollToIndex(processedIndex, { align: 'center' });
timelineSync.setFocusItem((prev) => (prev ? { ...prev, scrollTo: false } : undefined));
scrollSucceeded = true;
const processedIndex = resolveProcessedIndex();

// Stop retry loop now that scroll succeeded
if (retryIntervalId !== undefined) {
clearInterval(retryIntervalId);
retryIntervalId = undefined;
if (processedIndex === undefined) {
if (timelineSync.focusItem.eventId) {
log.log(
`[PermalinkJump] Event not found in processedEvents yet: eventId=${timelineSync.focusItem.eventId}, processedEvents.length=${processedEventsRef.current.length}`
);
}
return false;
}

// Use ResizeObserver to wait for layout to stabilize (images loading, etc.)
// before re-centering. This prevents the scroll target from being pushed out
// of view when media loads above it.
if (messageListRef.current && 'ResizeObserver' in globalThis) {
let resizeDebounceTimer: ReturnType<typeof setTimeout> | undefined;

resizeObserver = new ResizeObserver(() => {
// Clear any pending re-center and schedule a new one after 100ms of no resize
if (resizeDebounceTimer !== undefined) {
clearTimeout(resizeDebounceTimer);
}
log.log(
`[PermalinkJump] Scroll succeeded: processedIndex=${processedIndex}, eventId=${timelineSync.focusItem.eventId}`
);
Sentry.addBreadcrumb({
category: 'timeline.permalink',
message: 'Scroll succeeded',
level: 'info',
data: {
processedIndex,
eventId: timelineSync.focusItem.eventId,
roomId: room.roomId,
},
});

resizeDebounceTimer = setTimeout(() => {
// Layout has settled (no resize for 100ms) — re-center now
if (vListRef.current && processedIndex !== undefined) {
log.log(
`[PermalinkJump] Re-centering after layout settled: processedIndex=${processedIndex}`
);
vListRef.current.scrollToIndex(processedIndex, { align: 'center' });
}

// Stop observing after first stable re-center
if (resizeObserver) {
resizeObserver.disconnect();
resizeObserver = undefined;
}
}, 100);
});

resizeObserver.observe(messageListRef.current);

// Fallback: stop observing after 2 seconds regardless
recenterTimeoutId = setTimeout(() => {
if (resizeObserver) {
resizeObserver.disconnect();
resizeObserver = undefined;
}
}, 2000);
} else {
// Fallback for browsers without ResizeObserver: use timeout
recenterTimeoutId = setTimeout(() => {
if (vListRef.current && processedIndex !== undefined) {
vListRef.current.scrollToIndex(processedIndex, { align: 'center' });
}
}, 600);
}
// An event-targeted jump should no longer be treated as bottom-pinned.
// If we leave atBottom=true from the room's previous state, the scroll
// handler can immediately "chase" the live bottom after this jump.
setAtBottom(false);
startJumpScrollBlock();

return true;
// Reveal timeline and scroll in the same frame to avoid flash
setIsReady(true);
vListRef.current.scrollToIndex(processedIndex, { align: 'center' });
timelineSync.setFocusItem((prev) => (prev ? { ...prev, scrollTo: false } : undefined));
scrollSucceeded = true;

// Stop retry loop now that scroll succeeded
if (retryIntervalId !== undefined) {
clearInterval(retryIntervalId);
retryIntervalId = undefined;
}
return false;

// Media loads and preview expansion can keep shifting layout after the
// first successful jump. Re-center a few bounded times and resolve the
// target row fresh each time so the event stays anchored while history
// and measured heights settle.
[150, 600, 1500, 3000].forEach((delay) => {
const recenterTimeoutId = setTimeout(() => {
Comment thread
Just-Insane marked this conversation as resolved.
Outdated
const delayedProcessedIndex = resolveProcessedIndex();
if (vListRef.current && delayedProcessedIndex !== undefined) {
log.log(
`[PermalinkJump] Re-centering after delayed settle: processedIndex=${delayedProcessedIndex}, delay=${delay}`
);
setAtBottom(false);
startJumpScrollBlock();
vListRef.current.scrollToIndex(delayedProcessedIndex, { align: 'center' });
}
}, delay);
recenterTimeoutIds.push(recenterTimeoutId);
});

return true;
};

// Try immediate scroll
Expand All @@ -642,18 +617,23 @@ export function RoomTimeline({
}, 200);
}

// Clear highlight after scroll settles. Use longer timeout for history jumps
// where pagination and rendering may take more time.
const highlightDuration = timelineSync.focusItem.highlight ? 4000 : 2000;
timeoutId = setTimeout(() => {
timelineSync.setFocusItem(undefined);
Comment thread
sentry[bot] marked this conversation as resolved.
}, highlightDuration);
const paginationLoading =
timelineSync.backwardStatus === 'loading' || timelineSync.forwardStatus === 'loading';

// Keep the highlight alive while surrounding context is still loading. Once
// pagination settles, leave the highlight around a bit longer so the target
// remains easy to re-find after previews/images finish reflowing.
if (!paginationLoading) {
const highlightDuration = timelineSync.focusItem.highlight ? 8000 : 4000;
timeoutId = setTimeout(() => {
timelineSync.setFocusItem(undefined);
}, highlightDuration);
}
}
return () => {
if (timeoutId !== undefined) clearTimeout(timeoutId);
if (retryIntervalId !== undefined) clearInterval(retryIntervalId);
if (recenterTimeoutId !== undefined) clearTimeout(recenterTimeoutId);
if (resizeObserver) resizeObserver.disconnect();
recenterTimeoutIds.forEach((id) => clearTimeout(id));
};
}, [
timelineSync.focusItem,
Expand All @@ -662,6 +642,8 @@ export function RoomTimeline({
getRawIndexToProcessedIndex,
setAtBottom,
startJumpScrollBlock,
timelineSync.backwardStatus,
timelineSync.forwardStatus,
room.roomId,
]);

Expand Down
22 changes: 20 additions & 2 deletions src/app/hooks/useAppVisibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const DEFAULT_HEARTBEAT_INTERVAL_MS = 10 * 60 * 1000;

type SessionSyncReason = 'heartbeat';

const requestServiceWorkerClaim = (reason: 'visible_foreground' | 'pageshow_restore') => {
const requestServiceWorkerClaim = (reason: 'pageshow_restore') => {
if (!('serviceWorker' in navigator)) return;
if (navigator.serviceWorker.controller) return;

Expand Down Expand Up @@ -181,7 +181,25 @@ export function useAppVisibility(mx: MatrixClient | undefined, activeSession?: S
);
appEvents.emitVisibilityChange(isVisible);
if (isVisible) {
requestServiceWorkerClaim('visible_foreground');
if ('serviceWorker' in navigator && !navigator.serviceWorker.controller) {
Sentry.addBreadcrumb({
category: 'service_worker.claim',
message: 'Foreground resume without service worker controller',
level: 'warning',
data: {
reason: 'visible_foreground',
visibilityState: document.visibilityState,
online: navigator.onLine,
},
});
Sentry.metrics.count('sable.sw.claim_skipped', 1, {
attributes: {
reason: 'visible_foreground',
visibility_state: document.visibilityState,
online: navigator.onLine,
},
});
}
}
if (!isVisible) {
appEvents.emitVisibilityHidden();
Expand Down
Loading