From 06f69abad9871920e9ca471887b18cd5c90bb679 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 10 Nov 2022 07:34:43 -0500 Subject: [PATCH] Don't switch to the home page needlessly after leaving a room (#9477) Currently, if you leave a room that you're not currently viewing, you'll get sent back to the home page for no reason. This creates needless friction when trying to leave multiple rooms belonging to a space from the room list. In addition to that fix, this improves the behavior when leaving a subspace, by making it take you to the parent space rather than all the way back home. --- src/utils/leave-behaviour.ts | 37 ++++++--- test/utils/leave-behaviour-test.ts | 124 +++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 test/utils/leave-behaviour-test.ts diff --git a/src/utils/leave-behaviour.ts b/src/utils/leave-behaviour.ts index 83054ce1b4..f330e9b873 100644 --- a/src/utils/leave-behaviour.ts +++ b/src/utils/leave-behaviour.ts @@ -128,17 +128,32 @@ export async function leaveRoomBehaviour(roomId: string, retry = true, spinner = return; } - if (!isMetaSpace(SpaceStore.instance.activeSpace) && - SpaceStore.instance.activeSpace !== roomId && - SdkContextClass.instance.roomViewStore.getRoomId() === roomId - ) { - dis.dispatch({ - action: Action.ViewRoom, - room_id: SpaceStore.instance.activeSpace, - metricsTrigger: undefined, // other - }); - } else { - dis.dispatch({ action: Action.ViewHomePage }); + if (SdkContextClass.instance.roomViewStore.getRoomId() === roomId) { + // We were viewing the room that was just left. In order to avoid + // accidentally viewing the next room in the list and clearing its + // notifications, switch to a neutral ground such as the home page or + // space landing page. + if (isMetaSpace(SpaceStore.instance.activeSpace)) { + dis.dispatch({ action: Action.ViewHomePage }); + } else if (SpaceStore.instance.activeSpace === roomId) { + // View the parent space, if there is one + const parent = SpaceStore.instance.getCanonicalParent(roomId); + if (parent !== null) { + dis.dispatch({ + action: Action.ViewRoom, + room_id: parent.roomId, + metricsTrigger: undefined, // other + }); + } else { + dis.dispatch({ action: Action.ViewHomePage }); + } + } else { + dis.dispatch({ + action: Action.ViewRoom, + room_id: SpaceStore.instance.activeSpace, + metricsTrigger: undefined, // other + }); + } } } diff --git a/test/utils/leave-behaviour-test.ts b/test/utils/leave-behaviour-test.ts new file mode 100644 index 0000000000..48618bd1f6 --- /dev/null +++ b/test/utils/leave-behaviour-test.ts @@ -0,0 +1,124 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { mocked, Mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { Room } from "matrix-js-sdk/src/models/room"; + +import { MatrixClientPeg } from "../../src/MatrixClientPeg"; +import { mkRoom, resetAsyncStoreWithClient, setupAsyncStoreWithClient, stubClient } from "../test-utils"; +import defaultDispatcher from "../../src/dispatcher/dispatcher"; +import { ViewRoomPayload } from "../../src/dispatcher/payloads/ViewRoomPayload"; +import { Action } from "../../src/dispatcher/actions"; +import { leaveRoomBehaviour } from "../../src/utils/leave-behaviour"; +import { SdkContextClass } from "../../src/contexts/SDKContext"; +import DMRoomMap from "../../src/utils/DMRoomMap"; +import SpaceStore from "../../src/stores/spaces/SpaceStore"; +import { MetaSpace } from "../../src/stores/spaces"; +import { ActionPayload } from "../../src/dispatcher/payloads"; + +describe("leaveRoomBehaviour", () => { + SdkContextClass.instance.constructEagerStores(); // Initialize RoomViewStore + + let client: Mocked; + let room: Mocked; + let space: Mocked; + + beforeEach(async () => { + stubClient(); + client = mocked(MatrixClientPeg.get()); + DMRoomMap.makeShared(); + + room = mkRoom(client, "!1:example.org"); + space = mkRoom(client, "!2:example.org"); + space.isSpaceRoom.mockReturnValue(true); + client.getRoom.mockImplementation(roomId => { + switch (roomId) { + case room.roomId: return room; + case space.roomId: return space; + default: return null; + } + }); + + await setupAsyncStoreWithClient(SpaceStore.instance, client); + }); + + afterEach(async () => { + SpaceStore.instance.setActiveSpace(MetaSpace.Home); + await resetAsyncStoreWithClient(SpaceStore.instance); + jest.restoreAllMocks(); + }); + + const viewRoom = (room: Room) => defaultDispatcher.dispatch({ + action: Action.ViewRoom, + room_id: room.roomId, + metricsTrigger: undefined, + }, true); + + const expectDispatch = async (payload: T) => { + const dispatcherSpy = jest.fn(); + const dispatcherRef = defaultDispatcher.register(dispatcherSpy); + await new Promise(resolve => setImmediate(resolve)); // Flush the dispatcher + expect(dispatcherSpy).toHaveBeenCalledWith(payload); + defaultDispatcher.unregister(dispatcherRef); + }; + + it("returns to the home page after leaving a room outside of a space that was being viewed", async () => { + viewRoom(room); + + await leaveRoomBehaviour(room.roomId); + await expectDispatch({ action: Action.ViewHomePage }); + }); + + it("returns to the parent space after leaving a room inside of a space that was being viewed", async () => { + jest.spyOn(SpaceStore.instance, "getCanonicalParent").mockImplementation( + roomId => roomId === room.roomId ? space : null, + ); + viewRoom(room); + SpaceStore.instance.setActiveSpace(space.roomId, false); + + await leaveRoomBehaviour(room.roomId); + await expectDispatch({ + action: Action.ViewRoom, + room_id: space.roomId, + metricsTrigger: undefined, + }); + }); + + it("returns to the home page after leaving a top-level space that was being viewed", async () => { + viewRoom(space); + SpaceStore.instance.setActiveSpace(space.roomId, false); + + await leaveRoomBehaviour(space.roomId); + await expectDispatch({ action: Action.ViewHomePage }); + }); + + it("returns to the parent space after leaving a subspace that was being viewed", async () => { + room.isSpaceRoom.mockReturnValue(true); + jest.spyOn(SpaceStore.instance, "getCanonicalParent").mockImplementation( + roomId => roomId === room.roomId ? space : null, + ); + viewRoom(room); + SpaceStore.instance.setActiveSpace(room.roomId, false); + + await leaveRoomBehaviour(room.roomId); + await expectDispatch({ + action: Action.ViewRoom, + room_id: space.roomId, + metricsTrigger: undefined, + }); + }); +});