From e65409861aac1b6d7b7d3f2d1eb7cfe16ea7bacd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 6 Jul 2022 14:07:10 +0200 Subject: [PATCH] Don't show the same user twice in Spotlight (#8978) Co-authored-by: Janne Mareike Koschinski --- .../12-spotlight/spotlight.spec.ts | 32 +++++++++++++++++++ .../dialogs/spotlight/SpotlightDialog.tsx | 26 ++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/cypress/integration/12-spotlight/spotlight.spec.ts b/cypress/integration/12-spotlight/spotlight.spec.ts index e5507af7e6..9b2f290cfa 100644 --- a/cypress/integration/12-spotlight/spotlight.spec.ts +++ b/cypress/integration/12-spotlight/spotlight.spec.ts @@ -55,6 +55,7 @@ declare global { roomHeaderName( options?: Partial ): Chainable>; + startDM(name: string): Chainable; } } } @@ -109,6 +110,20 @@ Cypress.Commands.add("roomHeaderName", ( return cy.get(".mx_RoomHeader_nametext", options); }); +Cypress.Commands.add("startDM", (name: string) => { + cy.openSpotlightDialog().within(() => { + cy.spotlightFilter(Filter.People); + cy.spotlightSearch().clear().type(name); + cy.get(".mx_Spinner").should("not.exist"); + cy.spotlightResults().should("have.length", 1); + cy.spotlightResults().eq(0).should("contain", name); + cy.spotlightResults().eq(0).click(); + }).then(() => { + cy.roomHeaderName().should("contain", name); + cy.get(".mx_RoomSublist[aria-label=People]").should("contain", name); + }); +}); + describe("Spotlight", () => { let synapse: SynapseInstance; @@ -319,6 +334,23 @@ describe("Spotlight", () => { }); }); + it("should close spotlight after starting a DM", () => { + cy.startDM(bot1Name); + cy.get(".mx_SpotlightDialog").should("have.length", 0); + }); + + it("should show the same user only once", () => { + cy.startDM(bot1Name); + cy.visit("/#/home"); + + cy.openSpotlightDialog().within(() => { + cy.spotlightFilter(Filter.People); + cy.spotlightSearch().clear().type(bot1Name); + cy.get(".mx_Spinner").should("not.exist"); + cy.spotlightResults().should("have.length", 1); + }); + }); + it("should be able to navigate results via keyboard", () => { cy.openSpotlightDialog().within(() => { cy.spotlightFilter(Filter.People); diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index 0bdb5b1cc9..6013b5721f 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -320,8 +320,25 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n ); const possibleResults = useMemo( () => { - const roomMembers = findVisibleRoomMembers(cli); - const roomMemberIds = new Set(roomMembers.map(item => item.userId)); + const roomResults = findVisibleRooms(cli).map(toRoomResult); + // If we already have a DM with the user we're looking for, we will + // show that DM instead of the user themselves + const alreadyAddedUserIds = roomResults.reduce((userIds, result) => { + const userId = DMRoomMap.shared().getUserIdForRoomId(result.room.roomId); + if (!userId) return userIds; + if (result.room.getJoinedMemberCount() > 2) return userIds; + userIds.add(userId); + return userIds; + }, new Set()); + const userResults = []; + for (const user of [...findVisibleRoomMembers(cli), ...users]) { + // Make sure we don't have any user more than once + if (alreadyAddedUserIds.has(user.userId)) continue; + alreadyAddedUserIds.add(user.userId); + + userResults.push(toMemberResult(user)); + } + return [ ...SpaceStore.instance.enabledMetaSpaces.map(spaceKey => ({ section: Section.Spaces, @@ -335,9 +352,8 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n SpaceStore.instance.setActiveSpace(spaceKey); }, })), - ...findVisibleRooms(cli).map(toRoomResult), - ...roomMembers.map(toMemberResult), - ...users.filter(item => !roomMemberIds.has(item.userId)).map(toMemberResult), + ...roomResults, + ...userResults, ...(profile ? [new DirectoryMember(profile)] : []).map(toMemberResult), ...publicRooms.map(toPublicRoomResult), ].filter(result => filter === null || result.filter.includes(filter));