From 96b213e7cbefd0d4f1c58fa8274283c7d2bda2e8 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Mon, 3 Jun 2019 17:51:40 +0100 Subject: [PATCH] Advance read receipts into trailing events without tiles This changes read receipt sending logic to allow it advance further into events without tiles (such as edits or reactions) that may exist after the last displayed event. By allowing the read receipt to advance past such events, this also marks as read any related notifications. For example, edits trigger notifications by default since they are `m.room.message` events, and with this change, such edit notifications can finally be marked read. Part of https://github.com/vector-im/riot-web/issues/9745 --- src/components/structures/TimelinePanel.js | 35 ++++++++++++++++++++-- src/components/views/rooms/EventTile.js | 3 ++ src/shouldHideEvent.js | 2 ++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index c939d31f94..17e25bfbfc 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -648,6 +648,7 @@ const TimelinePanel = React.createClass({ const lastReadEventIndex = this._getLastDisplayedEventIndex({ ignoreOwn: true, + allowEventsWithoutTiles: true, }); if (lastReadEventIndex === null) { shouldSendRR = false; @@ -1111,14 +1112,18 @@ const TimelinePanel = React.createClass({ const ignoreOwn = opts.ignoreOwn || false; const ignoreEchoes = opts.ignoreEchoes || false; const allowPartial = opts.allowPartial || false; + const allowEventsWithoutTiles = opts.allowEventsWithoutTiles || false; const messagePanel = this.refs.messagePanel; if (messagePanel === undefined) return null; + const EventTile = sdk.getComponent('rooms.EventTile'); + const wrapperRect = ReactDOM.findDOMNode(messagePanel).getBoundingClientRect(); const myUserId = MatrixClientPeg.get().credentials.userId; - for (let i = this.state.events.length-1; i >= 0; --i) { + let lastDisplayedIndex = null; + for (let i = this.state.events.length - 1; i >= 0; --i) { const ev = this.state.events[i]; if (ignoreOwn && ev.sender && ev.sender.userId == myUserId) { @@ -1136,10 +1141,34 @@ const TimelinePanel = React.createClass({ const boundingRect = node.getBoundingClientRect(); if ((allowPartial && boundingRect.top < wrapperRect.bottom) || (!allowPartial && boundingRect.bottom < wrapperRect.bottom)) { - return i; + lastDisplayedIndex = i; + break; } } - return null; + + if (lastDisplayedIndex === null) { + return null; + } + + // If events without tiles are allowed, then we walk forward from the + // the last displayed event and advance the index for any events without + // tiles that immediately follow it. + // XXX: We could track the last event without a tile after the last + // displayed event in the loop above so that we only do a single pass + // through the loop, which would be more efficient. Using two passes is + // easier to reason about, so let's start there and optimise later if + // needed. + if (allowEventsWithoutTiles) { + for (let i = lastDisplayedIndex + 1; i < this.state.events.length; i++) { + const ev = this.state.events[i]; + if (EventTile.haveTileForEvent(ev)) { + break; + } + lastDisplayedIndex = i; + } + } + + return lastDisplayedIndex; }, /** diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 78e98a8133..62938cccae 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -819,6 +819,9 @@ module.exports.haveTileForEvent = function(e) { // Only messages have a tile (black-rectangle) if redacted if (e.isRedacted() && !isMessageEvent(e)) return false; + // No tile for replacement events since they update the original tile + if (e.isRelation("m.replace")) return false; + const handler = getHandlerTile(e); if (handler === undefined) return false; if (handler === 'messages.TextualEvent') { diff --git a/src/shouldHideEvent.js b/src/shouldHideEvent.js index 3a1e51c610..5ecb91b305 100644 --- a/src/shouldHideEvent.js +++ b/src/shouldHideEvent.js @@ -45,6 +45,8 @@ export default function shouldHideEvent(ev) { // Hide redacted events if (ev.isRedacted() && !isEnabled('showRedactions')) return true; + + // Hide replacement events since they update the original tile if (ev.isRelation("m.replace")) return true; const eventDiff = memberEventDiff(ev);