Merge pull request #11934 from nextcloud/fix/noid/focusing-on-message

chore(MessagesList): Refactor scrolling
nextcloud · Mar 27, 2024 · 0afabde · 0afabde
2 parents 58f6d61 + 9184ceb
commit 0afabde
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 103 deletions.
diff --git a/src/components/MessagesList/MessagesGroup/Message/Message.vue b/src/components/MessagesList/MessagesGroup/Message/Message.vue
@@ -314,8 +314,6 @@ export default {
 		}
 	},
 
-	expose: ['highlightMessage'],
-
 	data() {
 		return {
 			isHovered: false,
@@ -470,15 +468,25 @@ export default {
 		},
 	},
 
+	mounted() {
+		EventBus.$on('highlight-message', this.highlightMessage)
+	},
+
+	beforeDestroy() {
+		EventBus.$off('highlight-message', this.highlightMessage)
+	},
+
 	methods: {
 		lastReadMessageVisibilityChanged(isVisible) {
 			if (isVisible) {
 				this.seen = true
 			}
 		},
 
-		highlightMessage() {
-			this.isHighlighted = true
+		highlightMessage(messageId) {
+			if (this.id === messageId) {
+				this.isHighlighted = true
+			}
 		},
 
 		handleMouseover() {

diff --git a/src/components/MessagesList/MessagesGroup/MessagesGroup.vue b/src/components/MessagesList/MessagesGroup/MessagesGroup.vue
@@ -38,7 +38,6 @@
 			</li>
 			<Message v-for="(message, index) of messages"
 				:key="message.id"
-				ref="message"
 				v-bind="message"
 				:token="token"
 				:is-temporary="message.timestamp === 0"
@@ -101,8 +100,6 @@ export default {
 		}
 	},
 
-	expose: ['highlightMessage'],
-
 	computed: {
 		actorId() {
 			return this.messages[0].actorId
@@ -163,16 +160,6 @@ export default {
 		},
 	},
 
-	methods: {
-		highlightMessage(messageId) {
-			for (const message of this.$refs.message) {
-				if (message.id === messageId) {
-					message.highlightMessage()
-					break
-				}
-			}
-		},
-	},
 }
 </script>
 

diff --git a/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue b/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue
@@ -92,8 +92,6 @@ export default {
 		},
 	},
 
-	expose: ['highlightMessage'],
-
 	setup() {
 		const { createCombinedSystemMessage } = useCombinedSystemMessage()
 
@@ -246,15 +244,6 @@ export default {
 			const prevMessage = this.messages[this.messages.findIndex(searchedMessage => searchedMessage.id === message.id) - 1]
 			return prevMessage?.id || this.previousMessageId
 		},
-
-		highlightMessage(messageId) {
-			for (const message of this.$refs.message) {
-				if (message.id === messageId) {
-					message.highlightMessage()
-					break
-				}
-			}
-		},
 	},
 }
 </script>

diff --git a/src/components/MessagesList/MessagesList.spec.js b/src/components/MessagesList/MessagesList.spec.js
@@ -141,9 +141,6 @@ describe('MessagesList.vue', () => {
 			expect(group.props('nextMessageId')).toBe(0)
 
 			expect(messagesListMock).toHaveBeenCalledWith(TOKEN)
-
-			const placeholder = wrapper.findAllComponents({ name: 'LoadingPlaceholder' })
-			expect(placeholder.exists()).toBe(false)
 		})
 
 		test('displays a date separator between days', () => {
@@ -254,7 +251,7 @@ describe('MessagesList.vue', () => {
 				},
 			})
 
-			const groups = wrapper.findAllComponents({ ref: 'messagesGroup' })
+			const groups = wrapper.findAll('.messages-group')
 
 			expect(groups.exists()).toBe(true)
 
@@ -279,7 +276,7 @@ describe('MessagesList.vue', () => {
 				},
 			})
 
-			const groups = wrapper.findAllComponents({ ref: 'messagesGroup' })
+			const groups = wrapper.findAll('.messages-group')
 
 			expect(groups.exists()).toBeTruthy()
 

diff --git a/src/components/MessagesList/MessagesList.vue b/src/components/MessagesList/MessagesList.vue
@@ -48,7 +48,6 @@
 			<component :is="messagesGroupComponent(group)"
 				v-for="group in list"
 				:key="group.id"
-				ref="messagesGroup"
 				class="messages-group"
 				:token="token"
 				:messages="group.messages"
@@ -58,6 +57,7 @@
 
 		<template v-if="showLoadingAnimation">
 			<LoadingPlaceholder type="messages"
+				class="messages-list__placeholder"
 				:count="15" />
 		</template>
 		<NcEmptyContent v-else-if="showEmptyContent"
@@ -165,11 +165,6 @@ export default {
 
 			isFocusingMessage: false,
 
-			/**
-			 * Quick edit option to fall back to the loading history and then new messages
-			 */
-			loadChatInLegacyMode: getCapabilities()?.spreed?.config?.chat?.legacy || false,
-
 			destroying: false,
 
 			expirationInterval: null,
@@ -211,7 +206,6 @@ export default {
 
 		showLoadingAnimation() {
 			return !this.$store.getters.isMessageListPopulated(this.token)
-				&& !this.messagesList.length
 		},
 
 		showEmptyContent() {
@@ -615,8 +609,7 @@ export default {
 			return null
 		},
 
-		scrollToFocusedMessage() {
-			const focusMessageId = this.getMessageIdFromHash()
+		scrollToFocusedMessage(focusMessageId) {
 			let isFocused = null
 			if (focusMessageId) {
 				// scroll to message in URL anchor
@@ -662,7 +655,7 @@ export default {
 				if (this.$store.getters.getFirstKnownMessageId(this.token) === null) {
 					let startingMessageId = 0
 					// first time load, initialize important properties
-					if (this.loadChatInLegacyMode || focusMessageId === null) {
+					if (focusMessageId === null) {
 						// Start from unread marker
 						this.$store.dispatch('setFirstKnownMessageId', {
 							token: this.token,
@@ -686,37 +679,28 @@ export default {
 						})
 					}
 
-					if (this.loadChatInLegacyMode) {
-						// get history before last read message
-						await this.getOldMessages(true)
-						// at this stage, the read marker will appear at the bottom of the view port since
-						// we haven't fetched the messages that come after it yet
-						// TODO: should we still show a spinner at this stage ?
+					// Get chat messages before last read message and after it
+					await this.getMessageContext(startingMessageId)
+					const startingMessageFound = this.focusMessage(startingMessageId, false, focusMessageId !== null)
 
-					} else {
-						// 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)
-						}
+					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 (this.loadChatInLegacyMode || focusMessageId === null) {
+				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()
+							this.scrollToFocusedMessage(focusMessageId)
 						})
 					}
 				}
@@ -726,11 +710,11 @@ export default {
 				// get new messages
 				await this.lookForNewMessages()
 
-				if (this.loadChatInLegacyMode || focusMessageId === null) {
+				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()
+						this.scrollToFocusedMessage(focusMessageId)
 					}
 				}
 			} else {
@@ -905,16 +889,14 @@ export default {
 				return
 			}
 
-			if (!this.loadChatInLegacyMode) {
-				if (this.isInitialisingMessages) {
-					console.debug('Ignore handleScroll as we are initialising the message history')
-					return
-				}
+			if (this.isInitialisingMessages) {
+				console.debug('Ignore handleScroll as we are initialising the message history')
+				return
+			}
 
-				if (this.isFocusingMessage) {
-					console.debug('Ignore handleScroll as we are programmatically scrolling to focus a message')
-					return
-				}
+			if (this.isFocusingMessage) {
+				console.debug('Ignore handleScroll as we are programmatically scrolling to focus a message')
+				return
 			}
 
 			const { scrollHeight, scrollTop, clientHeight } = this.$refs.scroller
@@ -1152,7 +1134,7 @@ export default {
 
 			this.$nextTick(async () => {
 				// FIXME: this doesn't wait for the smooth scroll to end
-				await element.scrollIntoView({
+				element.scrollIntoView({
 					behavior: smooth ? 'smooth' : 'auto',
 					block: 'center',
 					inline: 'nearest',
@@ -1162,12 +1144,7 @@ export default {
 					this.$refs.scroller.scrollTop += this.$refs.scroller.offsetHeight / 4
 				}
 				if (highlightAnimation) {
-					for (const group of this.$refs.messagesGroup) {
-						if (group.messages.some(message => message.id === messageId)) {
-							group.highlightMessage(messageId)
-							break
-						}
-					}
+					EventBus.$emit('highlight-message', messageId)
 				}
 				this.isFocusingMessage = false
 				await this.handleScroll()
@@ -1290,9 +1267,16 @@ export default {
 }
 
 .messages-list {
-  &__empty-content {
-    height: 100%;
-  }
+	&__placeholder {
+		display: flex;
+		flex-direction: column-reverse;
+		overflow: hidden;
+		height: 100%;
+	}
+
+	&__empty-content {
+		height: 100%;
+	}
 }
 
 .messages-group {

diff --git a/src/store/messagesStore.js b/src/store/messagesStore.js
@@ -224,33 +224,33 @@ const getters = {
 			return null
 		}
 
-		const displayableMessages = getters.messagesList(token).filter(message => {
+		return getters.messagesList(token).find(message => {
 			return message.id >= readMessageId
-				&& !('' + message.id).startsWith('temp-')
-		})
-
-		if (displayableMessages.length) {
-			return displayableMessages.shift().id
-		}
-
-		return null
+				&& !String(message.id).startsWith('temp-')
+				&& message.systemMessage !== 'reaction'
+				&& message.systemMessage !== 'reaction_deleted'
+				&& message.systemMessage !== 'reaction_revoked'
+				&& message.systemMessage !== 'poll_voted'
+				&& message.systemMessage !== 'message_deleted'
+				&& message.systemMessage !== 'message_edited'
+		})?.id
 	},
 
 	getFirstDisplayableMessageIdBeforeReadMarker: (state, getters) => (token, readMessageId) => {
 		if (!state.messages[token]) {
 			return null
 		}
 
-		const displayableMessages = getters.messagesList(token).filter(message => {
+		return getters.messagesList(token).findLast(message => {
 			return message.id < readMessageId
-				&& !('' + message.id).startsWith('temp-')
-		})
-
-		if (displayableMessages.length) {
-			return displayableMessages.pop().id
-		}
-
-		return null
+				&& !String(message.id).startsWith('temp-')
+				&& message.systemMessage !== 'reaction'
+				&& message.systemMessage !== 'reaction_deleted'
+				&& message.systemMessage !== 'reaction_revoked'
+				&& message.systemMessage !== 'poll_voted'
+				&& message.systemMessage !== 'message_deleted'
+				&& message.systemMessage !== 'message_edited'
+		})?.id
 	},
 
 	isSendingMessages: (state) => {