Skip to content

Commit ca42625

Browse files
ShGKmeShGKme
committed
refactor(conversations): separate search results and conversations
- Fix "// TODO": instead of mutating the getter result as computed side effect, move sorting conversations into conversationsList getter - Divide storing all conversations in the LeftSidebar into: "All conversations list" + "Filtered conversations list" + "Search result conversations list" It allows to easily add v-show (KeepAlive) or VirtualScrolling in future - Organize rendering in LeftSIdebar and reduce duplications Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
1 parent 7bcb9bc commit ca42625

4 files changed

Lines changed: 122 additions & 101 deletions

File tree

‎src/components/LeftSidebar/LeftSidebar.spec.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,6 @@ describe('LeftSidebar.vue', () => {
132132
const wrapper = mountComponent()
133133

134134
expect(fetchConversationsAction).toHaveBeenCalledWith(expect.anything(), expect.anything())
135-
expect(conversationsListMock).toHaveBeenCalled()
136-
137-
const conversationListItems = wrapper.findAllComponents({ name: 'Conversation' })
138-
expect(conversationListItems).toHaveLength(conversationsList.length)
139135

140136
expect(wrapper.vm.searchText).toBe('')
141137
expect(wrapper.vm.initialisedConversations).toBeFalsy()
@@ -146,9 +142,12 @@ describe('LeftSidebar.vue', () => {
146142
await flushPromises()
147143

148144
expect(wrapper.vm.initialisedConversations).toBeTruthy()
149-
expect(conversationListItems.at(0).props('item')).toStrictEqual(conversationsList[2])
150-
expect(conversationListItems.at(1).props('item')).toStrictEqual(conversationsList[0])
151-
expect(conversationListItems.at(2).props('item')).toStrictEqual(conversationsList[1])
145+
146+
const conversationListItems = wrapper.findAllComponents({ name: 'Conversation' })
147+
expect(conversationListItems).toHaveLength(conversationsList.length)
148+
expect(conversationListItems.at(0).props('item')).toStrictEqual(conversationsList[0])
149+
expect(conversationListItems.at(1).props('item')).toStrictEqual(conversationsList[1])
150+
expect(conversationListItems.at(2).props('item')).toStrictEqual(conversationsList[2])
152151

153152
expect(conversationsReceivedEvent).toHaveBeenCalledWith({
154153
singleConversation: false,
@@ -304,7 +303,6 @@ describe('LeftSidebar.vue', () => {
304303
const wrapper = mountComponent()
305304

306305
expect(fetchConversationsAction).toHaveBeenCalledWith(expect.anything(), expect.anything())
307-
expect(conversationsListMock).toHaveBeenCalled()
308306

309307
const searchBox = wrapper.findComponent({ name: 'SearchBox' })
310308
expect(searchBox.exists()).toBeTruthy()

‎src/components/LeftSidebar/LeftSidebar.vue‎

Lines changed: 92 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -113,89 +113,98 @@
113113
<template #list>
114114
<li ref="container" class="left-sidebar__list" @scroll="debounceHandleScroll">
115115
<ul class="scroller">
116-
<NcListItem v-if="noMatchFound && searchText && canStartConversations"
117-
:title="t('spreed', 'Create a new conversation')"
118-
@click="createConversation(searchText)">
119-
<template #icon>
120-
<ChatPlus :size="30" />
121-
</template>
122-
<template #subtitle>
123-
{{ searchText }}
124-
</template>
125-
</NcListItem>
126-
127-
<NcAppNavigationCaption :class="{'hidden-visually': !isSearching}"
128-
:title="t('spreed', 'Conversations')" />
129-
<Conversation v-for="item of conversationsList"
130-
:key="item.id"
131-
:ref="`conversation-${item.token}`"
132-
:item="item" />
133-
<template v-if="!initialisedConversations">
134-
<LoadingPlaceholder type="conversations" />
116+
<!-- Conversations List -->
117+
<template v-if="!isSearching">
118+
<NcAppNavigationCaption :title="t('spreed', 'Conversations')" class="hidden-visually" />
119+
<Conversation v-for="item of filteredConversationsList"
120+
:key="`conversation_${item.id}`"
121+
:ref="`conversation-${item.token}`"
122+
:item="item" />
123+
<LoadingPlaceholder v-if="!initialisedConversations" type="conversations" />
124+
<Hint v-else-if="filteredConversationsList.length === 0" :hint="t('spreed', 'No matches found')" />
135125
</template>
136-
<Hint v-else-if="noMatchFound"
137-
:hint="t('spreed', 'No matches found')" />
138-
<template v-if="isSearching">
139-
<template v-if="!listedConversationsLoading && searchResultsListedConversations.length > 0">
126+
127+
<!-- Search results -->
128+
<template v-else-if="isSearching">
129+
<!-- Create a new conversation -->
130+
<NcListItem v-if="searchResultsConversationList.length === 0 && canStartConversations"
131+
:title="t('spreed', 'Create a new conversation')"
132+
@click="createConversation(searchText)">
133+
<template #icon>
134+
<ChatPlus :size="30" />
135+
</template>
136+
<template #subtitle>
137+
{{ searchText }}
138+
</template>
139+
</NcListItem>
140+
141+
<!-- Search results: user's conversations -->
142+
<NcAppNavigationCaption :title="t('spreed', 'Conversations')" />
143+
<Conversation v-for="item of searchResultsConversationList"
144+
:key="`conversation_${item.id}`"
145+
:ref="`conversation-${item.token}`"
146+
:item="item" />
147+
<Hint v-if="searchResultsConversationList.length === 0" :hint="t('spreed', 'No matches found')" />
148+
149+
<!-- Search results: listed (open) conversations -->
150+
<template v-if="!listedConversationsLoading && searchResultsListedConversations.length !== 0">
140151
<NcAppNavigationCaption :title="t('spreed', 'Open conversations')" />
141152
<Conversation v-for="item of searchResultsListedConversations"
142-
:key="item.id"
153+
:key="`open-conversation_${item.id}`"
143154
:item="item"
144155
is-search-result />
145156
</template>
157+
158+
<!-- Search results: users -->
146159
<template v-if="searchResultsUsers.length !== 0">
147160
<NcAppNavigationCaption :title="t('spreed', 'Users')" />
148161
<NcListItem v-for="item of searchResultsUsers"
149-
:key="item.id"
162+
:key="`user_${item.id}`"
150163
:title="item.label"
151164
@click="createAndJoinConversation(item)">
152165
<template #icon>
153-
<ConversationIcon :item="iconData(item)"
154-
:disable-menu="true" />
155-
</template>
156-
</NcListItem>
157-
</template>
158-
<template v-if="!showStartConversationsOptions">
159-
<NcAppNavigationCaption v-if="searchResultsUsers.length === 0"
160-
:title="t('spreed', 'Users')" />
161-
<Hint v-if="contactsLoading" :hint="t('spreed', 'Loading')" />
162-
<Hint v-else :hint="t('spreed', 'No matches found')" />
163-
</template>
164-
</template>
165-
<template v-if="showStartConversationsOptions">
166-
<template v-if="searchResultsGroups.length !== 0">
167-
<NcAppNavigationCaption :title="t('spreed', 'Groups')" />
168-
<NcListItem v-for="item of searchResultsGroups"
169-
:key="item.id"
170-
:title="item.label"
171-
@click="createAndJoinConversation(item)">
172-
<template #icon>
173-
<ConversationIcon :item="iconData(item)"
174-
:disable-menu="true" />
166+
<ConversationIcon :item="iconData(item)" disable-menu />
175167
</template>
176168
</NcListItem>
177169
</template>
178170

179-
<template v-if="searchResultsCircles.length !== 0">
180-
<NcAppNavigationCaption :title="t('spreed', 'Circles')" />
181-
<NcListItem v-for="item of searchResultsCircles"
182-
:key="item.id"
183-
:title="item.label"
184-
@click="createAndJoinConversation(item)">
185-
<template #icon>
186-
<ConversationIcon :item="iconData(item)"
187-
:disable-menu="true" />
188-
</template>
189-
</NcListItem>
171+
<!-- Search results: new conversations -->
172+
<template v-if="canStartConversations">
173+
<!-- New conversations: Groups -->
174+
<template v-if="searchResultsGroups.length !== 0">
175+
<NcAppNavigationCaption :title="t('spreed', 'Groups')" />
176+
<NcListItem v-for="item of searchResultsGroups"
177+
:key="`group_${item.id}`"
178+
:title="item.label"
179+
@click="createAndJoinConversation(item)">
180+
<template #icon>
181+
<ConversationIcon :item="iconData(item)" disable-menu />
182+
</template>
183+
</NcListItem>
184+
</template>
185+
186+
<!-- New conversations: Circles -->
187+
<template v-if="searchResultsCircles.length !== 0">
188+
<NcAppNavigationCaption :title="t('spreed', 'Circles')" />
189+
<NcListItem v-for="item of searchResultsCircles"
190+
:key="`circle_${item.id}`"
191+
:title="item.label"
192+
@click="createAndJoinConversation(item)">
193+
<template #icon>
194+
<ConversationIcon :item="iconData(item)" disable-menu />
195+
</template>
196+
</NcListItem>
197+
</template>
190198
</template>
191199

192-
<NcAppNavigationCaption v-if="sourcesWithoutResults"
193-
:title="sourcesWithoutResultsList" />
200+
<!-- Search results: no results (yet) -->
201+
<NcAppNavigationCaption v-if="sourcesWithoutResults" :title="sourcesWithoutResultsList" />
194202
<Hint v-if="contactsLoading" :hint="t('spreed', 'Loading')" />
195203
<Hint v-else :hint="t('spreed', 'No search results')" />
196204
</template>
197205
</ul>
198206
</li>
207+
199208
<NcButton v-if="!preventFindingUnread && unreadNum > 0"
200209
class="unread-mention-button"
201210
type="primary"
@@ -337,37 +346,42 @@ export default {
337346
338347
computed: {
339348
conversationsList() {
340-
let conversations = this.$store.getters.conversationsList
349+
return this.$store.getters.conversationsList
350+
},
351+
352+
searchResultsConversationList() {
341353
if (this.searchText !== '' || this.isFocused) {
342354
const lowerSearchText = this.searchText.toLowerCase()
343-
conversations = conversations.filter(conversation =>
355+
return this.conversationsList.filter(conversation =>
344356
conversation.displayName.toLowerCase().includes(lowerSearchText)
345-
|| conversation.name.toLowerCase().includes(lowerSearchText)
357+
|| conversation.name.toLowerCase().includes(lowerSearchText)
346358
)
347-
} else if (this.isFiltered === 'unread') {
348-
conversations = conversations.filter(conversation => conversation.unreadMessages > 0)
349-
} else if (this.isFiltered === 'mentions') {
350-
conversations = conversations.filter(conversation => conversation.unreadMention || (conversation.unreadMessages > 0
359+
} else {
360+
return []
361+
}
362+
},
363+
364+
filteredConversationsList() {
365+
if (this.isFocused) {
366+
return this.conversationsList
367+
}
368+
369+
if (this.isFiltered === 'unread') {
370+
return this.conversationsList.filter(conversation => conversation.unreadMessages > 0)
371+
}
372+
373+
if (this.isFiltered === 'mentions') {
374+
return this.conversationsList.filter(conversation => conversation.unreadMention || (conversation.unreadMessages > 0
351375
&& (conversation.type === CONVERSATION.TYPE.ONE_TO_ONE || conversation.type === CONVERSATION.TYPE.ONE_TO_ONE_FORMER)))
352376
}
353377
354-
// FIXME: this modifies the original array,
355-
// maybe should act on a copy or sort already within the store ?
356-
return conversations.sort(this.sortConversations)
378+
return this.conversationsList
357379
},
358380
359381
isSearching() {
360382
return this.searchText !== ''
361383
},
362384
363-
noMatchFound() {
364-
return (this.searchText || this.isFiltered) && !this.conversationsList.length
365-
},
366-
367-
showStartConversationsOptions() {
368-
return this.isSearching && this.canStartConversations
369-
},
370-
371385
sourcesWithoutResults() {
372386
return !this.searchResultsUsers.length
373387
|| !this.searchResultsGroups.length
@@ -601,14 +615,6 @@ export default {
601615
emit('show-settings')
602616
},
603617
604-
sortConversations(conversation1, conversation2) {
605-
if (conversation1.isFavorite !== conversation2.isFavorite) {
606-
return conversation1.isFavorite ? -1 : 1
607-
}
608-
609-
return conversation2.lastActivity - conversation1.lastActivity
610-
},
611-
612618
/**
613619
* @param {object} [options] Options for conversation refreshing
614620
* @param {string} [options.token] The conversation token that got update

‎src/store/conversationsStore.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,24 @@ const state = {
106106

107107
const getters = {
108108
conversations: state => state.conversations,
109-
conversationsList: state => Object.values(state.conversations).filter(conversation => {
110-
// Filter out breakout rooms from left sidebar
111-
return conversation.objectType !== 'room'
112-
}),
109+
/**
110+
* List of all conversations sorted by isFavorite and lastActivity without breakout rooms
111+
*
112+
* @param {object} state state
113+
* @return {object[]} sorted conversations list
114+
*/
115+
conversationsList: state => {
116+
return Object.values(state.conversations)
117+
// Filter out breakout rooms
118+
.filter(conversation => conversation.objectType !== 'room')
119+
// Sort by isFavorite and lastActivity
120+
.sort((conversation1, conversation2) => {
121+
if (conversation1.isFavorite !== conversation2.isFavorite) {
122+
return conversation1.isFavorite ? -1 : 1
123+
}
124+
return conversation2.lastActivity - conversation1.lastActivity
125+
})
126+
},
113127
/**
114128
* Get a conversation providing its token
115129
*

‎src/store/conversationsStore.spec.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ describe('conversationsStore', () => {
289289

290290
expect(fetchConversations).toHaveBeenCalledWith({ })
291291
// conversationsList is actual to the response
292-
expect(store.getters.conversationsList).toEqual([oldConversation, newConversation])
292+
expect(store.getters.conversationsList).toEqual([newConversation, oldConversation])
293293
// Only old conversation with new activity should be actually replaced with new objects
294294
expect(store.state.conversationsStore.conversations[oldConversation.token]).toStrictEqual(oldConversation)
295295
expect(store.state.conversationsStore.conversations[newConversation.token]).toStrictEqual(newConversation)
@@ -581,8 +581,11 @@ describe('conversationsStore', () => {
581581
await store.dispatch('fetchConversations', { modifiedSince })
582582

583583
expect(fetchConversations).toHaveBeenCalledWith({ params: { modifiedSince } })
584-
// conversationsList is actual to the response
585-
expect(store.getters.conversationsList).toEqual([newConversation1, newConversation2])
584+
// conversations are actual to the response
585+
expect(store.state.conversationsStore.conversations).toEqual({
586+
[newConversation1.token]: newConversation1,
587+
[newConversation2.token]: newConversation2,
588+
})
586589
// Only old conversation with new activity should be actually replaced with new objects
587590
expect(store.state.conversationsStore.conversations[oldConversation1.token]).toStrictEqual(oldConversation1)
588591
expect(store.state.conversationsStore.conversations[oldConversation2.token]).toStrictEqual(newConversation2)

0 commit comments

Comments
 (0)