From ba37a4db951d926a17456c6cf32d4f078b9880f1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 3 Nov 2022 00:43:00 -0500 Subject: [PATCH] Jump forward seamlessly Fix https://github.com/matrix-org/matrix-public-archive/issues/120 --- server/routes/room-routes.js | 49 ++++++++++++++----- shared/hydrogen-vm-render-script.js | 4 +- shared/lib/url-creator.js | 19 ++----- shared/viewmodels/ArchiveRoomViewModel.js | 8 +-- .../JumpToNextActivitySummaryTileViewModel.js | 1 - shared/views/ArchiveRoomView.js | 5 +- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/server/routes/room-routes.js b/server/routes/room-routes.js index 4686fabc..ee021ca8 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -152,6 +152,7 @@ router.get( router.get( '/jump', + // eslint-disable-next-line max-statements asyncHandler(async function (req, res) { const roomIdOrAlias = getRoomIdOrAliasFromReq(req); @@ -159,16 +160,15 @@ router.get( assert(!Number.isNaN(ts), '?ts query parameter must be a number'); const dir = req.query.dir; assert(['f', 'b'].includes(dir), '?dir query parameter must be [f|b]'); - const scrollStartPosition = req.query.continue; // We have to wait for the room join to happen first before we can use the jump to // date endpoint const roomId = await ensureRoomJoined(matrixAccessToken, roomIdOrAlias, req.query.via); let originServerTs; - let eventIdForTimestamp; - + let scrollStartEventId; try { + let eventIdForTimestamp; // Find the closest day to today with messages ({ eventId: eventIdForTimestamp, originServerTs } = await timestampToEvent({ accessToken: matrixAccessToken, @@ -178,14 +178,15 @@ router.get( })); // The goal is to go forward 100 messages, so that when we view the room at that - // point going backwards 100 message, we end up at the perfect sam continuation spot - // in the room. + // point going backwards 100 messages, we end up at the perfect sam continuation + // spot in the room. // - // XXX: This is flawed in the fact that when we go `/messages?dir=b` it could - // backfill messages which will fill up the response before we perfectly connect and - // continue from the position they were jumping from before. When `/messages?dir=f` - // backfills, we won't have this problem anymore because any messages backfilled in - // the forwards direction would be picked up the same going backwards. + // XXX: This is flawed in the fact that when we go `/messages?dir=b` later, it + // could backfill messages which will fill up the response before we perfectly + // connect and continue from the position they were jumping from before. When + // `/messages?dir=f` backfills, we won't have this problem anymore because any + // messages backfilled in the forwards direction would be picked up the same going + // backwards. if (dir === 'f') { // Use `/messages?dir=f` and get the `end` pagination token to paginate from. And // then start the scroll from the top of the page so they can continue. @@ -198,7 +199,31 @@ router.get( limit: archiveMessageLimit, }); - originServerTs = messageResData.chunk[messageResData.chunk.length - 1]?.origin_server_ts; + if (!messageResData.chunk?.length) { + throw new StatusError( + 404, + `/messages response didn't contain any more messages to jump to` + ); + } + + const timestampOfLastMessage = + messageResData.chunk[messageResData.chunk.length - 1].origin_server_ts; + const dateOfLastMessage = new Date(timestampOfLastMessage); + + // Back track from the last message timestamp to the date boundary. This will + // gurantee some overlap with the previous page we jumped from so we don't lose + // any messages in the gap. + const utcMidnightOfDayBefore = Date.UTC( + dateOfLastMessage.getUTCFullYear(), + dateOfLastMessage.getUTCMonth(), + dateOfLastMessage.getUTCDate() + ); + // We minus 1 from UTC midnight to get to the day before + const endOfDayBeforeDate = new Date(utcMidnightOfDayBefore - 1); + + originServerTs = endOfDayBeforeDate; + // Start the scroll at the next event from where they jumped from (seamless navigation) + scrollStartEventId = eventIdForTimestamp; } } catch (err) { const is404Error = err instanceof HTTPResponseError && err.response.status === 404; @@ -227,7 +252,7 @@ router.get( // TODO: Add query parameter that causes the client to start the scroll at the top // when jumping forwards so they can continue reading where they left off. matrixPublicArchiveURLCreator.archiveUrlForDate(roomIdOrAlias, new Date(originServerTs), { - scrollStartPosition, + scrollStartEventId, }) ); }) diff --git a/shared/hydrogen-vm-render-script.js b/shared/hydrogen-vm-render-script.js index 91be577e..6d1d5fc4 100644 --- a/shared/hydrogen-vm-render-script.js +++ b/shared/hydrogen-vm-render-script.js @@ -56,7 +56,7 @@ async function mountHydrogen() { const appElement = document.querySelector('#app'); const qs = new URLSearchParams(window?.location?.search); - const scrollStartPosition = qs.get('continue'); + const scrollStartEventId = qs.get('continue'); const platformConfig = {}; const assetPaths = {}; @@ -126,7 +126,7 @@ async function mountHydrogen() { // The timestamp from the URL that was originally visited dayTimestampFrom: fromTimestamp, dayTimestampTo: toTimestamp, - scrollStartPosition, + scrollStartEventId, events, stateEventMap, shouldIndex, diff --git a/shared/lib/url-creator.js b/shared/lib/url-creator.js index c34b94e8..f91ae263 100644 --- a/shared/lib/url-creator.js +++ b/shared/lib/url-creator.js @@ -65,7 +65,7 @@ class URLCreator { return `${urlJoin(this._basePath, `${urlPath}`)}${qsToUrlPiece(qs)}`; } - archiveUrlForDate(roomIdOrAlias, date, { viaServers = [], scrollStartPosition } = {}) { + archiveUrlForDate(roomIdOrAlias, date, { viaServers = [], scrollStartEventId } = {}) { assert(roomIdOrAlias); assert(date); @@ -73,8 +73,8 @@ class URLCreator { [].concat(viaServers).forEach((viaServer) => { qs.append('via', viaServer); }); - if (scrollStartPosition) { - qs.append('continue', scrollStartPosition); + if (scrollStartEventId) { + qs.append('continue', scrollStartEventId); } const urlPath = this._getArchiveUrlPathForRoomIdOrAlias(roomIdOrAlias); @@ -86,15 +86,7 @@ class URLCreator { return `${urlJoin(this._basePath, `${urlPath}/date/${urlDate}`)}${qsToUrlPiece(qs)}`; } - archiveJumpUrlForRoom( - roomIdOrAlias, - { - ts, - dir, - // where the scroll position should continue from ['top'|'bottom'] - scrollStartPosition, - } - ) { + archiveJumpUrlForRoom(roomIdOrAlias, { ts, dir }) { assert(roomIdOrAlias); assert(ts); assert(dir); @@ -102,9 +94,6 @@ class URLCreator { let qs = new URLSearchParams(); qs.append('ts', ts); qs.append('dir', dir); - if (scrollStartPosition) { - qs.append('continue', scrollStartPosition); - } const urlPath = this._getArchiveUrlPathForRoomIdOrAlias(roomIdOrAlias); diff --git a/shared/viewmodels/ArchiveRoomViewModel.js b/shared/viewmodels/ArchiveRoomViewModel.js index ff56d0dc..ff52684f 100644 --- a/shared/viewmodels/ArchiveRoomViewModel.js +++ b/shared/viewmodels/ArchiveRoomViewModel.js @@ -64,7 +64,7 @@ class ArchiveRoomViewModel extends ViewModel { room, dayTimestampFrom, dayTimestampTo, - scrollStartPosition, + scrollStartEventId, events, stateEventMap, shouldIndex, @@ -82,7 +82,7 @@ class ArchiveRoomViewModel extends ViewModel { this._room = room; this._dayTimestampFrom = dayTimestampFrom; this._dayTimestampTo = dayTimestampTo; - this._scrollStartPosition = scrollStartPosition === 'top' ? 'top' : 'bottom'; + this._scrollStartEventId = scrollStartEventId; this._currentTopPositionEventEntry = null; this._matrixPublicArchiveURLCreator = new MatrixPublicArchiveURLCreator(basePath); this._basePath = basePath; @@ -250,8 +250,8 @@ class ArchiveRoomViewModel extends ViewModel { return this._currentTopPositionEventEntry; } - get scrollStartPosition() { - return this._scrollStartPosition; + get scrollStartEventId() { + return this._scrollStartEventId; } get shouldShowRightPanel() { diff --git a/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js b/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js index 3cf7f4e5..5b2c9034 100644 --- a/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js +++ b/shared/viewmodels/JumpToNextActivitySummaryTileViewModel.js @@ -38,7 +38,6 @@ class JumpToNextActivitySummaryTileViewModel extends SimpleTile { { ts: this.rangeEndTimestamp, dir: 'f', - scrollStartPosition: 'top', } ); } diff --git a/shared/views/ArchiveRoomView.js b/shared/views/ArchiveRoomView.js index 85c27ea0..eb95f3a8 100644 --- a/shared/views/ArchiveRoomView.js +++ b/shared/views/ArchiveRoomView.js @@ -115,6 +115,7 @@ class DisabledComposerView extends TemplateView { class ArchiveRoomView extends TemplateView { render(t, vm) { + console.log('vm.scrollStartEventId', vm.scrollStartEventId); const rootElement = t.div( { className: { @@ -149,7 +150,9 @@ class ArchiveRoomView extends TemplateView { t.view( new TimelineView(vm.timelineViewModel, { viewClassForTile: customViewClassForTile, - stickToBottom: vm.scrollStartPosition === 'bottom', + stickToBottom: !vm.scrollStartEventId, + anchorPosition: vm.scrollStartEventId ? 'top' : 'bottom', + initialAnchorEventId: vm.scrollStartEventId, }) ), t.view(new DisabledComposerView(vm)),