[stable29] Follow-up: chat scrolling refactoring

src/components/MessagesList/MessagesList.vue

@@ -78,7 +78,6 @@ import uniqueId from 'lodash/uniqueId.js'
 import Message from 'vue-material-design-icons/Message.vue'
 
 import Axios from '@nextcloud/axios'
-import { getCapabilities } from '@nextcloud/capabilities'
 import { subscribe, unsubscribe } from '@nextcloud/event-bus'
 import moment from '@nextcloud/moment'
 
@@ -241,7 +240,7 @@ export default {
 				return false
 			}
 
-			return !!this.$store.getters.findParticipant(this.token, this.$store.getters.getParticipantIdentifier())
+			return !!this.$store.getters.findParticipant(this.token, this.conversation)
 		},
 
 		isInLobby() {
@@ -614,36 +613,40 @@ export default {
 			let isFocused = null
 			if (focusMessageId) {
 				// scroll to message in URL anchor
-				isFocused = this.focusMessage(focusMessageId, false)
+				this.focusMessage(focusMessageId)
+				return
 			}
 
-			if (!isFocused && this.visualLastReadMessageId) {
+			if (this.visualLastReadMessageId) {
 				// scroll to last read message if visible in the current pages
 				isFocused = this.focusMessage(this.visualLastReadMessageId, false, false)
 			}
 
-			// TODO: in case the element is not in a page but does exist in the DB,
-			// we need to scroll up / down to the page where it would exist after
-			// loading said pages
-
 			if (!isFocused) {
-				// if no anchor was present or the message to focus on did not exist,
-				// scroll to bottom
-				this.scrollToBottom({ force: true })
-			}
+				// Safeguard 1: scroll to first visible message before the read marker
+				const fallbackLastReadMessageId = this.$store.getters.getFirstDisplayableMessageIdBeforeReadMarker(this.token, this.visualLastReadMessageId)
+				if (fallbackLastReadMessageId) {
+					isFocused = this.focusMessage(fallbackLastReadMessageId, false, false)
+				}
 
-			// if no scrollbars, clear read marker directly as scrolling is not possible for the user to clear it
-			// also clear in case lastReadMessage is zero which is due to an older bug
-			if (this.visualLastReadMessageId === 0
-				|| (this.$refs.scroller && this.$refs.scroller.scrollHeight <= this.$refs.scroller.offsetHeight)) {
-				// clear after a delay, unless scrolling can resume in-between
-				this.debounceUpdateReadMarkerPosition()
+				if (!isFocused) {
+					// Safeguard 2: in case the fallback message is not found too
+					// scroll to bottom
+					this.scrollToBottom({ force: true })
+				} else {
+					this.$store.dispatch('setVisualLastReadMessageId', {
+						token: this.token,
+						id: fallbackLastReadMessageId,
+					})
+				}
 			}
+
+			// Update read marker in all cases except when the message is from URL anchor
+			this.debounceUpdateReadMarkerPosition()
 		},
 
 		async handleStartGettingMessagesPreconditions() {
 			if (this.token && this.isParticipant && !this.isInLobby) {
-
 				// prevent sticky mode before we have loaded anything
 				this.isInitialisingMessages = true
 				const focusMessageId = this.getMessageIdFromHash()
@@ -654,70 +657,27 @@ export default {
 				})
 
 				if (this.$store.getters.getFirstKnownMessageId(this.token) === null) {
-					let startingMessageId = 0
-					// first time load, initialize important properties
-					if (focusMessageId === null) {
-						// Start from unread marker
-						this.$store.dispatch('setFirstKnownMessageId', {
-							token: this.token,
-							id: this.conversation.lastReadMessage,
-						})
-						startingMessageId = this.conversation.lastReadMessage
-						this.$store.dispatch('setLastKnownMessageId', {
-							token: this.token,
-							id: this.conversation.lastReadMessage,
-						})
-					} else {
-						// Start from message hash
-						this.$store.dispatch('setFirstKnownMessageId', {
-							token: this.token,
-							id: focusMessageId,
-						})
-						startingMessageId = focusMessageId
-						this.$store.dispatch('setLastKnownMessageId', {
-							token: this.token,
-							id: focusMessageId,
-						})
-					}
+					// Start from message hash or unread marker
+					const startingMessageId = focusMessageId !== null ? focusMessageId : this.conversation.lastReadMessage
+					// First time load, initialize important properties
+					this.$store.dispatch('setFirstKnownMessageId', { token: this.token, id: startingMessageId })
+					this.$store.dispatch('setLastKnownMessageId', { token: this.token, id: startingMessageId })
 
 					// Get chat messages before last read message and after it
 					await this.getMessageContext(startingMessageId)
-					const startingMessageFound = this.focusMessage(startingMessageId, false, focusMessageId !== null)
-
-					if (!startingMessageFound) {
-						const fallbackStartingMessageId = this.$store.getters.getFirstDisplayableMessageIdBeforeReadMarker(this.token, startingMessageId)
-						this.$store.dispatch('setVisualLastReadMessageId', {
-							token: this.token,
-							id: fallbackStartingMessageId,
-						})
-						this.focusMessage(fallbackStartingMessageId, false, false)
-					}
 				}
 
-				let hasScrolled = false
-				if (focusMessageId === null) {
-					// if lookForNewMessages will long poll instead of returning existing messages,
-					// scroll right away to avoid delays
-					if (!this.hasMoreMessagesToLoad) {
-						hasScrolled = true
-						this.$nextTick(() => {
-							this.scrollToFocusedMessage(focusMessageId)
-						})
-					}
-				}
+				this.$nextTick(() => {
+					// basically scrolling to either the last read message or the message in the URL anchor
+					// and there is a fallback to scroll to the bottom if the message is not found
+					this.scrollToFocusedMessage(focusMessageId)
+				})
 
 				this.isInitialisingMessages = false
 
 				// get new messages
 				await this.lookForNewMessages()
 
-				if (focusMessageId === null) {
-					// don't scroll if lookForNewMessages was polling as we don't want
-					// to scroll back to the read marker after receiving new messages later
-					if (!hasScrolled) {
-						this.scrollToFocusedMessage(focusMessageId)
-					}
-				}
 			} else {
 				this.$store.dispatch('cancelLookForNewMessages', { requestId: this.chatIdentifier })
 			}
@@ -750,6 +710,12 @@ export default {
 				if (Axios.isCancel(exception)) {
 					console.debug('The request has been canceled', exception)
 				}
+
+				if (exception?.response?.status === 304 && exception?.response?.data === '') {
+					// 304 - Not modified
+					// Empty chat, no messages to load
+					this.$store.dispatch('loadedMessagesOfConversation', { token: this.token })
+				}
 			}
 			this.loadingOldMessages = false
 		},
@@ -1083,7 +1049,7 @@ export default {
 		 */
 		scrollToBottom(options = {}) {
 			this.$nextTick(() => {
-				if (!this.$refs.scroller) {
+				if (!this.$refs.scroller || this.isFocusingMessage) {
 					return
 				}
 
@@ -1106,7 +1072,6 @@ export default {
 					newTop = this.$refs.scroller.scrollHeight
 					this.setChatScrolledToBottom(true)
 				}
-
 				this.$refs.scroller.scrollTo({
 					top: newTop,
 					behavior: options?.smooth ? 'smooth' : 'auto',
@@ -1123,35 +1088,40 @@ export default {
 		 * @return {boolean} true if element was found, false otherwise
 		 */
 		focusMessage(messageId, smooth = true, highlightAnimation = true) {
-			const element = document.getElementById(`message_${messageId}`)
+			let element = document.getElementById(`message_${messageId}`)
 			if (!element) {
+				// Message id doesn't exist
 				// TODO: in some cases might need to trigger a scroll up if this is an older message
+				// https://github.com/nextcloud/spreed/pull/10084
 				console.warn('Message to focus not found in DOM', messageId)
-				return false
+				return false // element not found
+			}
+
+			if (element.offsetParent === null) {
+				console.debug('Message to focus is hidden, scrolling to its nearest visible parent', messageId)
+				element = element.closest('ul[style="display: none;"]').parentElement
 			}
 
 			console.debug('Scrolling to a focused message programmatically')
 			this.isFocusingMessage = true
 
-			this.$nextTick(async () => {
-				// FIXME: this doesn't wait for the smooth scroll to end
-				element.scrollIntoView({
-					behavior: smooth ? 'smooth' : 'auto',
-					block: 'center',
-					inline: 'nearest',
-				})
-				if (this.$refs.scroller && !smooth) {
-					// scroll the viewport slightly further to make sure the element is about 1/3 from the top
-					this.$refs.scroller.scrollTop += this.$refs.scroller.offsetHeight / 4
-				}
-				if (highlightAnimation) {
-					EventBus.$emit('highlight-message', messageId)
-				}
-				this.isFocusingMessage = false
-				await this.handleScroll()
+			element.scrollIntoView({
+				behavior: smooth ? 'smooth' : 'auto',
+				block: 'center',
+				inline: 'nearest',
 			})
 
-			return true
+			if (this.$refs.scroller && !smooth) {
+				// scroll the viewport slightly further to make sure the element is about 1/3 from the top
+				this.$refs.scroller.scrollTop += this.$refs.scroller.offsetHeight / 4
+			}
+
+			if (highlightAnimation) {
+				EventBus.$emit('highlight-message', messageId)
+			}
+			this.isFocusingMessage = false
+
+			return true // element found
 		},
 
 		/**

src/store/messagesStore.js

@@ -89,6 +89,17 @@ function hasMentionToSelf(context, message) {
 	return false
 }
 
+/**
+ * Returns whether the given message is presented in DOM and visible (none of the ancestors has `display: none`)
+ *
+ * @param {string} messageId store context
+ * @return {boolean} whether the message is visible in the UI
+ */
+function isMessageVisible(messageId) {
+	const element = document.getElementById(`message_${messageId}`)
+	return element !== null && element.offsetParent !== null
+}
+
 const state = {
 	/**
 	 * Map of conversation token to message list
@@ -243,6 +254,7 @@ const getters = {
 
 		return getters.messagesList(token).findLast(message => {
 			return message.id < readMessageId
+				&& isMessageVisible(message.id)
 				&& !String(message.id).startsWith('temp-')
 				&& message.systemMessage !== 'reaction'
 				&& message.systemMessage !== 'reaction_deleted'
@@ -1410,6 +1422,10 @@ const actions = {
 	async easeMessageList(context, { token }) {
 		context.commit('easeMessageList', { token })
 	},
+
+	loadedMessagesOfConversation(context, { token }) {
+		context.commit('loadedMessagesOfConversation', { token })
+	}
 }
 
 export default { state, mutations, getters, actions }

src/store/participantsStore.js

@@ -230,29 +230,16 @@ const getters = {
 		}
 
 		if (participantIdentifier.attendeeId) {
-			if (state.attendees[token][participantIdentifier.attendeeId]) {
-				return state.attendees[token][participantIdentifier.attendeeId]
-			}
-			return null
-		}
-
-		let foundAttendee = null
-		Object.keys(state.attendees[token]).forEach((attendeeId) => {
-			if (participantIdentifier.actorType && participantIdentifier.actorId
-				&& state.attendees[token][attendeeId].actorType === participantIdentifier.actorType
-				&& state.attendees[token][attendeeId].actorId === participantIdentifier.actorId) {
-				foundAttendee = attendeeId
-			}
-			if (participantIdentifier.sessionId && state.attendees[token][attendeeId].sessionIds.includes(participantIdentifier.sessionId)) {
-				foundAttendee = attendeeId
-			}
-		})
-
-		if (!foundAttendee) {
-			return null
+			return state.attendees[token][participantIdentifier.attendeeId] ?? null
 		}
 
-		return state.attendees[token][foundAttendee]
+		// Fallback, sometimes actorId and actorType are set before the attendeeId
+		return Object.entries(state.attendees[token]).find(([attendeeId, attendee]) => {
+			return (participantIdentifier.actorType && participantIdentifier.actorId
+					&& attendee.actorType === participantIdentifier.actorType
+					&& attendee.actorId === participantIdentifier.actorId)
+				|| (participantIdentifier.sessionId && attendee.sessionIds.includes(participantIdentifier.sessionId))
+		})?.[1] ?? null
 	},
 	getPeer: (state) => (token, sessionId, userId) => {
 		if (state.peers[token]) {