diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index 1c4043f150..8223d84dbb 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -137,15 +137,22 @@ const InnerSpacePanel = React.memo(({ children, isPanelCo const [invites, spaces, activeSpace] = useSpaces(); const activeSpaces = activeSpace ? [activeSpace] : []; - const homeNotificationState = SpaceStore.spacesTweakAllRoomsEnabled - ? RoomNotificationStateStore.instance.globalState : SpaceStore.instance.getNotificationState(HOME_SPACE); + let homeTooltip: string; + let homeNotificationState: NotificationState; + if (SpaceStore.instance.allRoomsInHome) { + homeTooltip = _t("All rooms"); + homeNotificationState = RoomNotificationStateStore.instance.globalState; + } else { + homeTooltip = _t("Home"); + homeNotificationState = SpaceStore.instance.getNotificationState(HOME_SPACE); + } return
SpaceStore.instance.setActiveSpace(null)} selected={!activeSpace} - tooltip={SpaceStore.spacesTweakAllRoomsEnabled ? _t("All rooms") : _t("Home")} + tooltip={homeTooltip} notificationState={homeNotificationState} isNarrow={isPanelCollapsed} /> diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 5aa49df8a1..54153b3d75 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -187,8 +187,7 @@ export const SETTINGS: {[setting: string]: ISetting} = { "feature_spaces.all_rooms": { displayName: _td("Show all rooms in Home"), supportedLevels: LEVELS_FEATURE, - default: true, - controller: new ReloadOnChangeController(), + default: false, }, "feature_dnd": { isFeature: true, diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index d064b01257..42ecc25651 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -37,9 +37,8 @@ import { EnhancedMap, mapDiff } from "../utils/maps"; import { setHasDiff } from "../utils/sets"; import RoomViewStore from "./RoomViewStore"; import { Action } from "../dispatcher/actions"; -import { arrayHasDiff } from "../utils/arrays"; +import { arrayHasDiff, arrayHasOrderChange } from "../utils/arrays"; import { objectDiff } from "../utils/objects"; -import { arrayHasOrderChange } from "../utils/arrays"; import { reorderLexicographically } from "../utils/stringOrderField"; import { TAG_ORDER } from "../components/views/rooms/RoomList"; import { shouldShowSpaceSettings } from "../utils/space"; @@ -48,6 +47,7 @@ import { _t } from "../languageHandler"; import GenericToast from "../components/views/toasts/GenericToast"; import Modal from "../Modal"; import InfoDialog from "../components/views/dialogs/InfoDialog"; +import { SettingUpdatedPayload } from "../dispatcher/payloads/SettingUpdatedPayload"; type SpaceKey = string | symbol; @@ -61,6 +61,7 @@ export const SUGGESTED_ROOMS = Symbol("suggested-rooms"); export const UPDATE_TOP_LEVEL_SPACES = Symbol("top-level-spaces"); export const UPDATE_INVITED_SPACES = Symbol("invited-spaces"); export const UPDATE_SELECTED_SPACE = Symbol("selected-space"); +export const UPDATE_HOME_BEHAVIOUR = Symbol("home-behaviour"); // Space Room ID/HOME_SPACE will be emitted when a Space's children change export interface ISuggestedRoom extends ISpaceSummaryRoom { @@ -69,12 +70,10 @@ export interface ISuggestedRoom extends ISpaceSummaryRoom { const MAX_SUGGESTED_ROOMS = 20; -// All of these settings cause the page to reload and can be costly if read frequently, so read them here only +// This setting causes the page to reload and can be costly if read frequently, so read it here only const spacesEnabled = SettingsStore.getValue("feature_spaces"); -const spacesTweakAllRoomsEnabled = SettingsStore.getValue("feature_spaces.all_rooms"); -const homeSpaceKey = spacesTweakAllRoomsEnabled ? "ALL_ROOMS" : "HOME_SPACE"; -const getSpaceContextKey = (space?: Room) => `mx_space_context_${space?.roomId || homeSpaceKey}`; +const getSpaceContextKey = (space?: Room) => `mx_space_context_${space?.roomId || "HOME_SPACE"}`; const partitionSpacesAndRooms = (arr: Room[]): [Room[], Room[]] => { // [spaces, rooms] return arr.reduce((result, room: Room) => { @@ -102,10 +101,6 @@ const getRoomFn: FetchRoomFn = (room: Room) => { }; export class SpaceStoreClass extends AsyncStoreWithClient { - constructor() { - super(defaultDispatcher, {}); - } - // The spaces representing the roots of the various tree-like hierarchies private rootSpaces: Room[] = []; // The list of rooms not present in any currently joined spaces @@ -122,6 +117,13 @@ export class SpaceStoreClass extends AsyncStoreWithClient { private _invitedSpaces = new Set(); private spaceOrderLocalEchoMap = new Map(); private _restrictedJoinRuleSupport?: IRoomCapability; + private _allRoomsInHome: boolean = SettingsStore.getValue("feature_spaces.all_rooms"); + + constructor() { + super(defaultDispatcher, {}); + + SettingsStore.monitorSetting("feature_spaces.all_rooms", null); + } public get invitedSpaces(): Room[] { return Array.from(this._invitedSpaces); @@ -139,13 +141,16 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return this._suggestedRooms; } + public get allRoomsInHome(): boolean { + return this._allRoomsInHome; + } + public async setActiveRoomInSpace(space: Room | null): Promise { if (space && !space.isSpaceRoom()) return; if (space !== this.activeSpace) await this.setActiveSpace(space); if (space) { - const notificationState = this.getNotificationState(space.roomId); - const roomId = notificationState.getFirstRoomWithNotifications(); + const roomId = this.getNotificationState(space.roomId).getFirstRoomWithNotifications(); defaultDispatcher.dispatch({ action: "view_room", room_id: roomId, @@ -200,7 +205,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // else if the last viewed room in this space is joined then view that // else view space home or home depending on what is being clicked on if (space?.getMyMembership() !== "invite" && - this.matrixClient?.getRoom(roomId)?.getMyMembership() === "join" + this.matrixClient?.getRoom(roomId)?.getMyMembership() === "join" && + this.getSpaceFilteredRoomIds(space).has(roomId) ) { defaultDispatcher.dispatch({ action: "view_room", @@ -377,7 +383,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public getSpaceFilteredRoomIds = (space: Room | null): Set => { - if (!space && spacesTweakAllRoomsEnabled) { + if (!space && this.allRoomsInHome) { return new Set(this.matrixClient.getVisibleRooms().map(r => r.roomId)); } return this.spaceFilteredRooms.get(space?.roomId || HOME_SPACE) || new Set(); @@ -474,7 +480,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }; private showInHomeSpace = (room: Room) => { - if (spacesTweakAllRoomsEnabled) return true; + if (this.allRoomsInHome) return true; if (room.isSpaceRoom()) return false; return !this.parentMap.get(room.roomId)?.size // put all orphaned rooms in the Home Space || DMRoomMap.shared().getUserIdForRoomId(room.roomId) // put all DMs in the Home Space @@ -506,7 +512,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldFilteredRooms = this.spaceFilteredRooms; this.spaceFilteredRooms = new Map(); - if (!spacesTweakAllRoomsEnabled) { + if (!this.allRoomsInHome) { // put all room invites in the Home Space const invites = visibleRooms.filter(r => !r.isSpaceRoom() && r.getMyMembership() === "invite"); this.spaceFilteredRooms.set(HOME_SPACE, new Set(invites.map(room => room.roomId))); @@ -562,8 +568,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); this.spaceFilteredRooms.forEach((roomIds, s) => { + if (this.allRoomsInHome && s === HOME_SPACE) return; // we'll be using the global notification state, skip + // Update NotificationStates - this.getNotificationState(s)?.setRooms(visibleRooms.filter(room => { + this.getNotificationState(s).setRooms(visibleRooms.filter(room => { if (!roomIds.has(room.roomId)) return false; if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { @@ -663,7 +671,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // TODO confirm this after implementing parenting behaviour if (room.isSpaceRoom()) { this.onSpaceUpdate(); - } else if (!spacesTweakAllRoomsEnabled) { + } else if (!this.allRoomsInHome) { this.onRoomUpdate(room); } this.emit(room.roomId); @@ -687,7 +695,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (order !== lastOrder) { this.notifyIfOrderChanged(); } - } else if (ev.getType() === EventType.Tag && !spacesTweakAllRoomsEnabled) { + } else if (ev.getType() === EventType.Tag && !this.allRoomsInHome) { // If the room was in favourites and now isn't or the opposite then update its position in the trees const oldTags = lastEv?.getContent()?.tags || {}; const newTags = ev.getContent()?.tags || {}; @@ -698,7 +706,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }; private onAccountData = (ev: MatrixEvent, lastEvent: MatrixEvent) => { - if (ev.getType() === EventType.Direct) { + if (!this.allRoomsInHome && ev.getType() === EventType.Direct) { const lastContent = lastEvent.getContent(); const content = ev.getContent(); @@ -733,9 +741,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.matrixClient.removeListener("Room.myMembership", this.onRoom); this.matrixClient.removeListener("Room.accountData", this.onRoomAccountData); this.matrixClient.removeListener("RoomState.events", this.onRoomState); - if (!spacesTweakAllRoomsEnabled) { - this.matrixClient.removeListener("accountData", this.onAccountData); - } + this.matrixClient.removeListener("accountData", this.onAccountData); } await this.reset(); } @@ -746,9 +752,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.matrixClient.on("Room.myMembership", this.onRoom); this.matrixClient.on("Room.accountData", this.onRoomAccountData); this.matrixClient.on("RoomState.events", this.onRoomState); - if (!spacesTweakAllRoomsEnabled) { - this.matrixClient.on("accountData", this.onAccountData); - } + this.matrixClient.on("accountData", this.onAccountData); this.matrixClient.getCapabilities().then(capabilities => { this._restrictedJoinRuleSupport = capabilities @@ -779,7 +783,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // as it will cause you to end up in the wrong room this.setActiveSpace(room, false); } else if ( - (!spacesTweakAllRoomsEnabled || this.activeSpace) && + (!this.allRoomsInHome || this.activeSpace) && !this.getSpaceFilteredRoomIds(this.activeSpace).has(roomId) ) { this.switchToRelatedSpace(roomId); @@ -791,17 +795,33 @@ export class SpaceStoreClass extends AsyncStoreWithClient { window.localStorage.setItem(getSpaceContextKey(this.activeSpace), payload.room_id); break; } + case "after_leave_room": if (this._activeSpace && payload.room_id === this._activeSpace.roomId) { this.setActiveSpace(null, false); } break; + case Action.SwitchSpace: if (payload.num === 0) { this.setActiveSpace(null); } else if (this.spacePanelSpaces.length >= payload.num) { this.setActiveSpace(this.spacePanelSpaces[payload.num - 1]); } + break; + + case Action.SettingUpdated: { + const settingUpdatedPayload = payload as SettingUpdatedPayload; + if (settingUpdatedPayload.settingName === "feature_spaces.all_rooms") { + const newValue = SettingsStore.getValue("feature_spaces.all_rooms"); + if (this.allRoomsInHome !== newValue) { + this._allRoomsInHome = newValue; + this.emit(UPDATE_HOME_BEHAVIOUR, this.allRoomsInHome); + this.rebuild(); // rebuild everything + } + } + break; + } } } @@ -872,7 +892,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { export default class SpaceStore { public static spacesEnabled = spacesEnabled; - public static spacesTweakAllRoomsEnabled = spacesTweakAllRoomsEnabled; private static internalInstance = new SpaceStoreClass(); diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 8b809be95d..09005e3d84 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -21,6 +21,7 @@ import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import "./SpaceStore-setup"; // enable space lab import "../skinned-sdk"; // Must be first for skinning to work import SpaceStore, { + UPDATE_HOME_BEHAVIOUR, UPDATE_INVITED_SPACES, UPDATE_SELECTED_SPACE, UPDATE_TOP_LEVEL_SPACES, @@ -30,6 +31,8 @@ import { mkEvent, stubClient } from "../test-utils"; import DMRoomMap from "../../src/utils/DMRoomMap"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import defaultDispatcher from "../../src/dispatcher/dispatcher"; +import SettingsStore from "../../src/settings/SettingsStore"; +import { SettingLevel } from "../../src/settings/SettingLevel"; jest.useFakeTimers(); @@ -79,8 +82,16 @@ describe("SpaceStore", () => { jest.runAllTimers(); }; + const setShowAllRooms = async (value: boolean) => { + if (store.allRoomsInHome === value) return; + const emitProm = testUtils.emitPromise(store, UPDATE_HOME_BEHAVIOUR); + await SettingsStore.setValue("feature_spaces.all_rooms", null, SettingLevel.DEVICE, value); + jest.runAllTimers(); // run async dispatch + await emitProm; + }; + beforeEach(() => { - jest.runAllTimers(); + jest.runAllTimers(); // run async dispatch client.getVisibleRooms.mockReturnValue(rooms = []); }); afterEach(async () => { @@ -346,10 +357,16 @@ describe("SpaceStore", () => { expect(store.getSpaceFilteredRoomIds(null).has(invite2)).toBeTruthy(); }); - it("home space does contain rooms/low priority even if they are also shown in a space", () => { + it("all rooms space does contain rooms/low priority even if they are also shown in a space", async () => { + await setShowAllRooms(true); expect(store.getSpaceFilteredRoomIds(null).has(room1)).toBeTruthy(); }); + it("home space doesn't contain rooms/low priority if they are also shown in a space", async () => { + await setShowAllRooms(false); + expect(store.getSpaceFilteredRoomIds(null).has(room1)).toBeFalsy(); + }); + it("space contains child rooms", () => { const space = client.getRoom(space1); expect(store.getSpaceFilteredRoomIds(space).has(fav1)).toBeTruthy(); @@ -592,20 +609,30 @@ describe("SpaceStore", () => { }); describe("context switching tests", () => { - const fn = jest.spyOn(defaultDispatcher, "dispatch"); + let dispatcherRef; + let currentRoom = null; beforeEach(async () => { [room1, room2, orphan1].forEach(mkRoom); mkSpace(space1, [room1, room2]); mkSpace(space2, [room2]); await run(); + + dispatcherRef = defaultDispatcher.register(payload => { + if (payload.action === "view_room" || payload.action === "view_home_page") { + currentRoom = payload.room_id || null; + } + }); }); afterEach(() => { - fn.mockClear(); localStorage.clear(); + defaultDispatcher.unregister(dispatcherRef); }); - const getCurrentRoom = () => fn.mock.calls.reverse().find(([p]) => p.action === "view_room")?.[0].room_id; + const getCurrentRoom = () => { + jest.runAllTimers(); + return currentRoom; + }; it("last viewed room in target space is the current viewed and in both spaces", async () => { await store.setActiveSpace(client.getRoom(space1)); @@ -642,6 +669,14 @@ describe("SpaceStore", () => { expect(getCurrentRoom()).toBe(space2); }); + it("last viewed room is target space is no longer in that space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room1); + localStorage.setItem(`mx_space_context_${space2}`, room1); + await store.setActiveSpace(client.getRoom(space2)); + expect(getCurrentRoom()).toBe(space2); // Space home instead of room1 + }); + it("no last viewed room in target space", async () => { await store.setActiveSpace(client.getRoom(space1)); viewRoom(room1); @@ -653,7 +688,7 @@ describe("SpaceStore", () => { await store.setActiveSpace(client.getRoom(space1)); viewRoom(room1); await store.setActiveSpace(null); - expect(fn.mock.calls[fn.mock.calls.length - 1][0]).toStrictEqual({ action: "view_home_page" }); + expect(getCurrentRoom()).toBeNull(); // Home }); }); @@ -707,6 +742,7 @@ describe("SpaceStore", () => { }); it("when switching rooms in the all rooms home space don't switch to related space", async () => { + await setShowAllRooms(true); viewRoom(room2); await store.setActiveSpace(null, false); viewRoom(room1);