From 2c21068183a9a48493c3dd3de120b6414c61c2be Mon Sep 17 00:00:00 2001 From: DorraJaouad Date: Fri, 22 Mar 2024 17:10:03 +0100 Subject: [PATCH 1/4] chore(MessagesList): adjust loading placeholder to not to exceed view height Signed-off-by: DorraJaouad --- src/components/MessagesList/MessagesList.vue | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/components/MessagesList/MessagesList.vue b/src/components/MessagesList/MessagesList.vue index d6d8c92f2b2..47682630cb2 100644 --- a/src/components/MessagesList/MessagesList.vue +++ b/src/components/MessagesList/MessagesList.vue @@ -58,6 +58,7 @@ Date: Mon, 25 Mar 2024 14:08:35 +0100 Subject: [PATCH 2/4] fix: switch highlighting behavior to event-driven Signed-off-by: DorraJaouad --- .../MessagesGroup/Message/Message.vue | 16 ++++++++++++---- .../MessagesGroup/MessagesGroup.vue | 13 ------------- .../MessagesGroup/MessagesSystemGroup.vue | 11 ----------- .../MessagesList/MessagesList.spec.js | 7 ++----- src/components/MessagesList/MessagesList.vue | 17 +++++------------ 5 files changed, 19 insertions(+), 45 deletions(-) diff --git a/src/components/MessagesList/MessagesGroup/Message/Message.vue b/src/components/MessagesList/MessagesGroup/Message/Message.vue index 4da5892f01a..5db073c0695 100644 --- a/src/components/MessagesList/MessagesGroup/Message/Message.vue +++ b/src/components/MessagesList/MessagesGroup/Message/Message.vue @@ -303,8 +303,6 @@ export default { } }, - expose: ['highlightMessage'], - data() { return { isHovered: false, @@ -450,6 +448,14 @@ export default { }, }, + mounted() { + EventBus.$on('highlight-message', this.highlightMessage) + }, + + beforeDestroy() { + EventBus.$off('highlight-message', this.highlightMessage) + }, + methods: { lastReadMessageVisibilityChanged(isVisible) { if (isVisible) { @@ -457,8 +463,10 @@ export default { } }, - 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 index c899c02358d..53d68f60493 100644 --- a/src/components/MessagesList/MessagesGroup/MessagesGroup.vue +++ b/src/components/MessagesList/MessagesGroup/MessagesGroup.vue @@ -38,7 +38,6 @@ diff --git a/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue b/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue index 46d23ac6100..f46b4af0361 100644 --- a/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue +++ b/src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue @@ -89,8 +89,6 @@ export default { }, }, - expose: ['highlightMessage'], - setup() { const { createCombinedSystemMessage } = useCombinedSystemMessage() @@ -235,15 +233,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 - } - } - }, }, } diff --git a/src/components/MessagesList/MessagesList.spec.js b/src/components/MessagesList/MessagesList.spec.js index ac5211064ae..b8f198a3565 100644 --- 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 index 47682630cb2..2cf34f2af7b 100644 --- a/src/components/MessagesList/MessagesList.vue +++ b/src/components/MessagesList/MessagesList.vue @@ -48,7 +48,6 @@ { - this.scrollToFocusedMessage() + this.scrollToFocusedMessage(focusMessageId) }) } } @@ -731,7 +729,7 @@ export default { // 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 { @@ -1153,7 +1151,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', @@ -1163,12 +1161,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() From 785a5ee544bb4ebff2e6adeb8c745f32686c64b6 Mon Sep 17 00:00:00 2001 From: DorraJaouad Date: Wed, 27 Mar 2024 10:48:19 +0100 Subject: [PATCH 3/4] fix(MessagesList): remove legacy mode Signed-off-by: DorraJaouad --- src/components/MessagesList/MessagesList.vue | 57 +++++++------------- 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/src/components/MessagesList/MessagesList.vue b/src/components/MessagesList/MessagesList.vue index 2cf34f2af7b..027d1a08e28 100644 --- a/src/components/MessagesList/MessagesList.vue +++ b/src/components/MessagesList/MessagesList.vue @@ -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() { @@ -661,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, @@ -685,31 +679,22 @@ 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) { @@ -725,7 +710,7 @@ 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) { @@ -904,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 From 9184cebeaad51768909c24a291680eed7cba5a1b Mon Sep 17 00:00:00 2001 From: DorraJaouad Date: Wed, 27 Mar 2024 11:57:52 +0100 Subject: [PATCH 4/4] chore(MessagesStore): adjust fallback displayable messages Signed-off-by: DorraJaouad --- src/store/messagesStore.js | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/store/messagesStore.js b/src/store/messagesStore.js index b9d8b3895ee..906090771f5 100644 --- a/src/store/messagesStore.js +++ b/src/store/messagesStore.js @@ -224,16 +224,16 @@ 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) => { @@ -241,16 +241,16 @@ const getters = { 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) => {