Skip to content

Commit 8a52bb0

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 - Remove duplication in conditions (the "no search result" part) Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
1 parent bf6a9c9 commit 8a52bb0

4 files changed

Lines changed: 103 additions & 86 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: 73 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -114,76 +114,87 @@
114114
@scroll="debounceHandleScroll">
115115
<NcAppNavigationCaption :class="{'hidden-visually': !isSearching}"
116116
:title="t('spreed', 'Conversations')" />
117-
<Conversation v-for="item of conversationsList"
118-
:key="item.id"
119-
:ref="`conversation-${item.token}`"
120-
:item="item" />
121-
<template v-if="!initialisedConversations">
122-
<LoadingPlaceholder type="conversations" />
117+
118+
<!-- Initial Loading -->
119+
<LoadingPlaceholder v-if="!initialisedConversations" type="conversations" />
120+
121+
<!-- Conversations List -->
122+
<template v-else-if="!isSearching">
123+
<Conversation v-for="item of filteredConversationsList"
124+
:key="item.id"
125+
:ref="`conversation-${item.token}`"
126+
:item="item" />
127+
<Hint v-if="filteredConversationsList.length === 0" :hint="t('spreed', 'No matches')" />
123128
</template>
124-
<Hint v-else-if="noMatchFound"
125-
:hint="t('spreed', 'No matches found')" />
126-
<template v-if="isSearching">
127-
<template v-if="!listedConversationsLoading && searchResultsListedConversations.length > 0">
129+
130+
<!-- Search results -->
131+
<template v-else-if="isSearching">
132+
<!-- Search results: user's conversations -->
133+
<Conversation v-for="item of searchResultsConversationList"
134+
:key="item.id"
135+
:ref="`conversation-${item.token}`"
136+
:item="item" />
137+
<Hint v-if="searchResultsConversationList.length === 0" :hint="t('spreed', 'No matches')" />
138+
139+
<!-- Search results: listed (open) conversations -->
140+
<template v-if="!listedConversationsLoading && searchResultsListedConversations.length !== 0">
128141
<NcAppNavigationCaption :title="t('spreed', 'Open conversations')" />
129142
<Conversation v-for="item of searchResultsListedConversations"
130143
:key="item.id"
131144
:item="item"
132145
is-search-result />
133146
</template>
147+
148+
<!-- Search results: users -->
134149
<template v-if="searchResultsUsers.length !== 0">
135150
<NcAppNavigationCaption :title="t('spreed', 'Users')" />
136151
<NcListItem v-for="item of searchResultsUsers"
137152
:key="item.id"
138153
:title="item.label"
139154
@click="createAndJoinConversation(item)">
140155
<template #icon>
141-
<ConversationIcon :item="iconData(item)"
142-
:disable-menu="true" />
143-
</template>
144-
</NcListItem>
145-
</template>
146-
<template v-if="!showStartConversationsOptions">
147-
<NcAppNavigationCaption v-if="searchResultsUsers.length === 0"
148-
:title="t('spreed', 'Users')" />
149-
<Hint v-if="contactsLoading" :hint="t('spreed', 'Loading')" />
150-
<Hint v-else :hint="t('spreed', 'No search results')" />
151-
</template>
152-
</template>
153-
<template v-if="showStartConversationsOptions">
154-
<template v-if="searchResultsGroups.length !== 0">
155-
<NcAppNavigationCaption :title="t('spreed', 'Groups')" />
156-
<NcListItem v-for="item of searchResultsGroups"
157-
:key="item.id"
158-
:title="item.label"
159-
@click="createAndJoinConversation(item)">
160-
<template #icon>
161-
<ConversationIcon :item="iconData(item)"
162-
:disable-menu="true" />
156+
<ConversationIcon :item="iconData(item)" disable-menu />
163157
</template>
164158
</NcListItem>
165159
</template>
166160

167-
<template v-if="searchResultsCircles.length !== 0">
168-
<NcAppNavigationCaption :title="t('spreed', 'Circles')" />
169-
<NcListItem v-for="item of searchResultsCircles"
170-
:key="item.id"
171-
:title="item.label"
172-
@click="createAndJoinConversation(item)">
173-
<template #icon>
174-
<ConversationIcon :item="iconData(item)"
175-
:disable-menu="true" />
176-
</template>
177-
</NcListItem>
161+
<!-- Search results: new conversations -->
162+
<template v-if="canStartConversations">
163+
<!-- New conversations: Groups -->
164+
<template v-if="searchResultsGroups.length !== 0">
165+
<NcAppNavigationCaption :title="t('spreed', 'Groups')" />
166+
<NcListItem v-for="item of searchResultsGroups"
167+
:key="item.id"
168+
:title="item.label"
169+
@click="createAndJoinConversation(item)">
170+
<template #icon>
171+
<ConversationIcon :item="iconData(item)" disable-menu />
172+
</template>
173+
</NcListItem>
174+
</template>
175+
176+
<!-- New conversations: Circles -->
177+
<template v-if="searchResultsCircles.length !== 0">
178+
<NcAppNavigationCaption :title="t('spreed', 'Circles')" />
179+
<NcListItem v-for="item of searchResultsCircles"
180+
:key="item.id"
181+
:title="item.label"
182+
@click="createAndJoinConversation(item)">
183+
<template #icon>
184+
<ConversationIcon :item="iconData(item)" disable-menu />
185+
</template>
186+
</NcListItem>
187+
</template>
178188
</template>
179189

180-
<NcAppNavigationCaption v-if="sourcesWithoutResults"
181-
:title="sourcesWithoutResultsList" />
190+
<!-- Search results: no results (yet) -->
191+
<NcAppNavigationCaption v-if="sourcesWithoutResults" :title="sourcesWithoutResultsList" />
182192
<Hint v-if="contactsLoading" :hint="t('spreed', 'Loading')" />
183193
<Hint v-else :hint="t('spreed', 'No search results')" />
184194
</template>
185195
</ul>
186196
</li>
197+
187198
<NcButton v-if="!preventFindingUnread && unreadNum > 0"
188199
class="unread-mention-button"
189200
type="primary"
@@ -306,37 +317,36 @@ export default {
306317
307318
computed: {
308319
conversationsList() {
309-
let conversations = this.$store.getters.conversationsList
320+
return this.$store.getters.conversationsList
321+
},
322+
323+
searchResultsConversationList() {
310324
if (this.searchText !== '') {
311325
const lowerSearchText = this.searchText.toLowerCase()
312-
conversations = conversations.filter(conversation =>
326+
return this.conversationsList.filter(conversation =>
313327
conversation.displayName.toLowerCase().includes(lowerSearchText)
314-
|| conversation.name.toLowerCase().includes(lowerSearchText)
328+
|| conversation.name.toLowerCase().includes(lowerSearchText)
315329
)
316-
} else if (this.isFiltered === 'unread') {
317-
conversations = conversations.filter(conversation => conversation.unreadMessages > 0)
330+
} else {
331+
return []
332+
}
333+
},
334+
335+
filteredConversationsList() {
336+
if (this.isFiltered === 'unread') {
337+
return this.conversationsList.filter(conversation => conversation.unreadMessages > 0)
318338
} else if (this.isFiltered === 'mentions') {
319-
conversations = conversations.filter(conversation => conversation.unreadMention || (conversation.unreadMessages > 0
339+
return this.conversationsList.filter(conversation => conversation.unreadMention || (conversation.unreadMessages > 0
320340
&& (conversation.type === CONVERSATION.TYPE.ONE_TO_ONE || conversation.type === CONVERSATION.TYPE.ONE_TO_ONE_FORMER)))
341+
} else {
342+
return this.conversationsList
321343
}
322-
323-
// FIXME: this modifies the original array,
324-
// maybe should act on a copy or sort already within the store ?
325-
return conversations.sort(this.sortConversations)
326344
},
327345
328346
isSearching() {
329347
return this.searchText !== ''
330348
},
331349
332-
noMatchFound() {
333-
return (this.searchText || this.isFiltered) && !this.conversationsList.length
334-
},
335-
336-
showStartConversationsOptions() {
337-
return this.isSearching && this.canStartConversations
338-
},
339-
340350
sourcesWithoutResults() {
341351
return !this.searchResultsUsers.length
342352
|| !this.searchResultsGroups.length
@@ -563,14 +573,6 @@ export default {
563573
emit('show-settings')
564574
},
565575
566-
sortConversations(conversation1, conversation2) {
567-
if (conversation1.isFavorite !== conversation2.isFavorite) {
568-
return conversation1.isFavorite ? -1 : 1
569-
}
570-
571-
return conversation2.lastActivity - conversation1.lastActivity
572-
},
573-
574576
/**
575577
* @param {object} [options] Options for conversation refreshing
576578
* @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
@@ -92,10 +92,24 @@ const state = {
9292

9393
const getters = {
9494
conversations: state => state.conversations,
95-
conversationsList: state => Object.values(state.conversations).filter(conversation => {
96-
// Filter out breakout rooms from left sidebar
97-
return conversation.objectType !== 'room'
98-
}),
95+
/**
96+
* List of all conversations sorted by isFavorite and lastActivity without breakout rooms
97+
*
98+
* @param {object} state state
99+
* @return {object[]} sorted conversations list
100+
*/
101+
conversationsList: state => {
102+
return Object.values(state.conversations)
103+
// Filter out breakout rooms
104+
.filter(conversation => conversation.objectType !== 'room')
105+
// Sort by isFavorite and lastActivity
106+
.sort((conversation1, conversation2) => {
107+
if (conversation1.isFavorite !== conversation2.isFavorite) {
108+
return conversation1.isFavorite ? -1 : 1
109+
}
110+
return conversation2.lastActivity - conversation1.lastActivity
111+
})
112+
},
99113
/**
100114
* Get a conversation providing its token
101115
*

‎src/store/conversationsStore.spec.js‎

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

319319
expect(fetchConversations).toHaveBeenCalledWith({ })
320320
// conversationsList is actual to the response
321-
expect(store.getters.conversationsList).toEqual([oldConversation, newConversation])
321+
expect(store.getters.conversationsList).toEqual([newConversation, oldConversation])
322322
// Only old conversation with new activity should be actually replaced with new objects
323323
expect(store.state.conversationsStore.conversations[oldConversation.token]).toStrictEqual(oldConversation)
324324
expect(store.state.conversationsStore.conversations[newConversation.token]).toStrictEqual(newConversation)
@@ -368,8 +368,11 @@ describe('conversationsStore', () => {
368368
await store.dispatch('fetchConversations', { modifiedSince })
369369

370370
expect(fetchConversations).toHaveBeenCalledWith({ params: { modifiedSince } })
371-
// conversationsList is actual to the response
372-
expect(store.getters.conversationsList).toEqual([newConversation1, newConversation2])
371+
// conversations are actual to the response
372+
expect(store.state.conversationsStore.conversations).toEqual({
373+
[newConversation1.token]: newConversation1,
374+
[newConversation2.token]: newConversation2,
375+
})
373376
// Only old conversation with new activity should be actually replaced with new objects
374377
expect(store.state.conversationsStore.conversations[oldConversation1.token]).toStrictEqual(oldConversation1)
375378
expect(store.state.conversationsStore.conversations[oldConversation2.token]).toStrictEqual(newConversation2)

0 commit comments

Comments
 (0)