Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(MessagesList): Refactor scrolling #11934

Merged
merged 4 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/components/MessagesList/MessagesGroup/Message/Message.vue
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,6 @@ export default {
}
},

expose: ['highlightMessage'],

data() {
return {
isHovered: false,
Expand Down Expand Up @@ -450,15 +448,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() {
Expand Down
13 changes: 0 additions & 13 deletions src/components/MessagesList/MessagesGroup/MessagesGroup.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -101,8 +100,6 @@ export default {
}
},

expose: ['highlightMessage'],

computed: {
actorId() {
return this.messages[0].actorId
Expand Down Expand Up @@ -163,16 +160,6 @@ export default {
},
},

methods: {
highlightMessage(messageId) {
for (const message of this.$refs.message) {
if (message.id === messageId) {
message.highlightMessage()
break
}
}
},
},
}
</script>

Expand Down
11 changes: 0 additions & 11 deletions src/components/MessagesList/MessagesGroup/MessagesSystemGroup.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ export default {
},
},

expose: ['highlightMessage'],

setup() {
const { createCombinedSystemMessage } = useCombinedSystemMessage()

Expand Down Expand Up @@ -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
}
}
},
},
}
</script>
Expand Down
7 changes: 2 additions & 5 deletions src/components/MessagesList/MessagesList.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -254,7 +251,7 @@ describe('MessagesList.vue', () => {
},
})

const groups = wrapper.findAllComponents({ ref: 'messagesGroup' })
const groups = wrapper.findAll('.messages-group')

expect(groups.exists()).toBe(true)

Expand All @@ -279,7 +276,7 @@ describe('MessagesList.vue', () => {
},
})

const groups = wrapper.findAllComponents({ ref: 'messagesGroup' })
const groups = wrapper.findAll('.messages-group')

expect(groups.exists()).toBeTruthy()

Expand Down
88 changes: 36 additions & 52 deletions src/components/MessagesList/MessagesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -58,6 +57,7 @@

<template v-if="showLoadingAnimation">
<LoadingPlaceholder type="messages"
class="messages-list__placeholder"
:count="15" />
</template>
<NcEmptyContent v-else-if="showEmptyContent"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -211,7 +206,6 @@ export default {

showLoadingAnimation() {
return !this.$store.getters.isMessageListPopulated(this.token)
&& !this.messagesList.length
},

showEmptyContent() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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)
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved

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)
})
}
}
Expand All @@ -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()
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved
this.scrollToFocusedMessage(focusMessageId)
}
}
} else {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
36 changes: 18 additions & 18 deletions src/store/messagesStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved
&& message.systemMessage !== 'message_deleted'
&& message.systemMessage !== 'message_edited'
})?.id
},

isSendingMessages: (state) => {
Expand Down
Loading