From 8e58cfcbb86533c20b5db32861d4ec52baa59d66 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 1 Dec 2021 16:40:54 +0000 Subject: [PATCH 01/26] Space preferences for whether or not you see DMs in a Space --- res/css/_components.scss | 1 + res/css/structures/_SpacePanel.scss | 4 + res/css/views/dialogs/_SettingsDialog.scss | 5 +- .../dialogs/_SpacePreferencesDialog.scss | 34 +++++++ .../views/context_menus/SpaceContextMenu.tsx | 14 +++ .../views/dialogs/SpacePreferencesDialog.tsx | 97 +++++++++++++++++++ .../views/dialogs/UserSettingsDialog.tsx | 2 +- src/components/views/rooms/RoomList.tsx | 3 +- src/i18n/strings/en_EN.json | 2 + src/settings/Settings.tsx | 5 + src/stores/spaces/SpaceStore.ts | 17 +++- src/utils/space.tsx | 8 ++ 12 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 res/css/views/dialogs/_SpacePreferencesDialog.scss create mode 100644 src/components/views/dialogs/SpacePreferencesDialog.tsx diff --git a/res/css/_components.scss b/res/css/_components.scss index adfd98925a5..979a9caeec4 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -111,6 +111,7 @@ @import "./views/dialogs/_SettingsDialog.scss"; @import "./views/dialogs/_ShareDialog.scss"; @import "./views/dialogs/_SlashCommandHelpDialog.scss"; +@import "./views/dialogs/_SpacePreferencesDialog.scss"; @import "./views/dialogs/_SpaceSettingsDialog.scss"; @import "./views/dialogs/_TabbedIntegrationManagerDialog.scss"; @import "./views/dialogs/_TermsDialog.scss"; diff --git a/res/css/structures/_SpacePanel.scss b/res/css/structures/_SpacePanel.scss index 6749134a2c5..7dfb06bdecd 100644 --- a/res/css/structures/_SpacePanel.scss +++ b/res/css/structures/_SpacePanel.scss @@ -383,6 +383,10 @@ $activeBorderColor: $primary-content; mask-image: url('$(res)/img/element-icons/roomlist/search.svg'); } + .mx_SpacePanel_iconPreferences::before { + mask-image: url('$(res)/img/element-icons/settings/preference.svg'); + } + .mx_SpacePanel_noIcon { display: none; diff --git a/res/css/views/dialogs/_SettingsDialog.scss b/res/css/views/dialogs/_SettingsDialog.scss index b3b6802c3d7..b518328de26 100644 --- a/res/css/views/dialogs/_SettingsDialog.scss +++ b/res/css/views/dialogs/_SettingsDialog.scss @@ -15,7 +15,10 @@ limitations under the License. */ // Not actually a component but things shared by settings components -.mx_UserSettingsDialog, .mx_RoomSettingsDialog, .mx_SpaceSettingsDialog { +.mx_UserSettingsDialog, +.mx_RoomSettingsDialog, +.mx_SpaceSettingsDialog, +.mx_SpacePreferencesDialog { width: 90vw; max-width: 1000px; // set the height too since tabbed view scrolls itself. diff --git a/res/css/views/dialogs/_SpacePreferencesDialog.scss b/res/css/views/dialogs/_SpacePreferencesDialog.scss new file mode 100644 index 00000000000..370c0f845b1 --- /dev/null +++ b/res/css/views/dialogs/_SpacePreferencesDialog.scss @@ -0,0 +1,34 @@ +/* +Copyright 2021 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. +*/ + +.mx_SpacePreferencesDialog { + width: 700px; + height: 400px; + + .mx_TabbedView .mx_SettingsTab { + min-width: unset; + + .mx_SettingsTab_section { + font-size: $font-15px; + line-height: $font-24px; + + .mx_Checkbox + p { + color: $secondary-content; + margin: 0 20px 0 24px; + } + } + } +} diff --git a/src/components/views/context_menus/SpaceContextMenu.tsx b/src/components/views/context_menus/SpaceContextMenu.tsx index d143ddfd98c..6c7d6f6597f 100644 --- a/src/components/views/context_menus/SpaceContextMenu.tsx +++ b/src/components/views/context_menus/SpaceContextMenu.tsx @@ -29,6 +29,7 @@ import { showCreateNewRoom, showCreateNewSubspace, showSpaceInvite, + showSpacePreferences, showSpaceSettings, } from "../../../utils/space"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; @@ -166,6 +167,14 @@ const SpaceContextMenu = ({ space, hideHeader, onFinished, ...props }: IProps) = ; } + const onPreferencesClick = (ev: ButtonEvent) => { + ev.preventDefault(); + ev.stopPropagation(); + + showSpacePreferences(space); + onFinished(); + }; + const onExploreRoomsClick = (ev: ButtonEvent) => { ev.preventDefault(); ev.stopPropagation(); @@ -193,6 +202,11 @@ const SpaceContextMenu = ({ space, hideHeader, onFinished, ...props }: IProps) = label={canAddRooms ? _t("Manage & explore rooms") : _t("Explore rooms")} onClick={onExploreRoomsClick} /> + { settingsOption } { leaveOption } { devtoolsOption } diff --git a/src/components/views/dialogs/SpacePreferencesDialog.tsx b/src/components/views/dialogs/SpacePreferencesDialog.tsx new file mode 100644 index 00000000000..5560a4890fa --- /dev/null +++ b/src/components/views/dialogs/SpacePreferencesDialog.tsx @@ -0,0 +1,97 @@ +/* +Copyright 2021 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 React, { ChangeEvent } from "react"; +import { Room } from "matrix-js-sdk/src/models/room"; + +import { _t, _td } from '../../../languageHandler'; +import BaseDialog from "../dialogs/BaseDialog"; +import { IDialogProps } from "./IDialogProps"; +import TabbedView, { Tab } from "../../structures/TabbedView"; +import StyledCheckbox from "../elements/StyledCheckbox"; +import { useSettingValue } from "../../../hooks/useSettings"; +import SettingsStore from "../../../settings/SettingsStore"; +import { SettingLevel } from "../../../settings/SettingLevel"; +import RoomName from "../elements/RoomName"; + +export enum SpacePreferenceTab { + Appearance = "SPACE_PREFERENCE_APPEARANCE_TAB", +} + +interface IProps extends IDialogProps { + space: Room; + initialTabId?: SpacePreferenceTab; +} + +const SpacePreferencesAppearanceTab = ({ space }: Pick) => { + const showPeople = useSettingValue("Spaces.showPeopleInSpace", space.roomId); + + return ( +
+
{ _t("Sections to show") }
+ +
+ ) => { + SettingsStore.setValue( + "Spaces.showPeopleInSpace", + space.roomId, + SettingLevel.ROOM_ACCOUNT, + !showPeople, + ); + }} + > + { _t("People") } + +

+ { _t("This groups your ongoing conversations with members of this space. " + + "Turning off will hide it from your view.") } +

+
+
+ ); +}; + +const SpacePreferencesDialog: React.FC = ({ space, initialTabId, onFinished }) => { + const tabs = [ + new Tab( + SpacePreferenceTab.Appearance, + _td("Appearance"), + "mx_RoomSettingsDialog_notificationsIcon", + , + ), + ]; + + return ( + +

+ +

+
+ +
+
+ ); +}; + +export default SpacePreferencesDialog; diff --git a/src/components/views/dialogs/UserSettingsDialog.tsx b/src/components/views/dialogs/UserSettingsDialog.tsx index a2699e99821..4261baaf96e 100644 --- a/src/components/views/dialogs/UserSettingsDialog.tsx +++ b/src/components/views/dialogs/UserSettingsDialog.tsx @@ -51,7 +51,7 @@ export enum UserTab { } interface IProps extends IDialogProps { - initialTabId?: string; + initialTabId?: UserTab; } interface IState { diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 7f74d65e938..c514ba8402e 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -611,7 +611,8 @@ export default class RoomList extends React.PureComponent { if ( (this.props.activeSpace === MetaSpace.Favourites && orderedTagId !== DefaultTagID.Favourite) || (this.props.activeSpace === MetaSpace.People && orderedTagId !== DefaultTagID.DM) || - (this.props.activeSpace === MetaSpace.Orphans && orderedTagId === DefaultTagID.DM) + (this.props.activeSpace === MetaSpace.Orphans && orderedTagId === DefaultTagID.DM) || + (this.props.activeSpace[0] === "!" && orderedTagId === DefaultTagID.DM) // TODO ) { alwaysVisible = false; } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a905bba6356..8dbc2629bb4 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2678,6 +2678,8 @@ "Share Room Message": "Share Room Message", "Link to selected message": "Link to selected message", "Command Help": "Command Help", + "Sections to show": "Sections to show", + "This groups your ongoing conversations with members of this space. Turning off will hide it from your view.": "This groups your ongoing conversations with members of this space. Turning off will hide it from your view.", "Space settings": "Space settings", "Settings - %(spaceName)s": "Settings - %(spaceName)s", "To help us prevent this in future, please send us logs.": "To help us prevent this in future, please send us logs.", diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 2e008eaf3d6..a5d1341299e 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -846,6 +846,11 @@ export const SETTINGS: {[setting: string]: ISetting} = { [MetaSpace.Home]: true, }, false), }, + "Spaces.showPeopleInSpace": { + supportedLevels: [SettingLevel.ROOM_ACCOUNT], + default: true, + controller: new IncompatibleController("showCommunitiesInsteadOfSpaces", null), + }, "showCommunitiesInsteadOfSpaces": { displayName: _td("Display Communities instead of Spaces"), description: _td("Temporarily show communities instead of Spaces for this session. " + diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 9ec98cdeb11..2c132da7b04 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -543,12 +543,14 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const space = this.matrixClient?.getRoom(spaceId); // Add relevant DMs - space?.getMembers().forEach(member => { - if (member.membership !== "join" && member.membership !== "invite") return; - DMRoomMap.shared().getDMRoomsForUserId(member.userId).forEach(roomId => { - roomIds.add(roomId); + if (SettingsStore.getValue("Spaces.showPeopleInSpace", spaceId)) { + space?.getMembers().forEach(member => { + if (member.membership !== "join" && member.membership !== "invite") return; + DMRoomMap.shared().getDMRoomsForUserId(member.userId).forEach(roomId => { + roomIds.add(roomId); + }); }); - }); + } const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { @@ -917,6 +919,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } break; } + + case "Spaces.showPeopleInSpace": + // TODO extract space DMs from space filtered rooms to simplify the data flow + this.rebuild(); // rebuild everything + break; } } } diff --git a/src/utils/space.tsx b/src/utils/space.tsx index 8642d423209..a88d621bc60 100644 --- a/src/utils/space.tsx +++ b/src/utils/space.tsx @@ -40,6 +40,7 @@ import Spinner from "../components/views/elements/Spinner"; import dis from "../dispatcher/dispatcher"; import LeaveSpaceDialog from "../components/views/dialogs/LeaveSpaceDialog"; import CreateSpaceFromCommunityDialog from "../components/views/dialogs/CreateSpaceFromCommunityDialog"; +import SpacePreferencesDialog, { SpacePreferenceTab } from "../components/views/dialogs/SpacePreferencesDialog"; export const shouldShowSpaceSettings = (space: Room) => { const userId = space.client.getUserId(); @@ -197,3 +198,10 @@ export const createSpaceFromCommunity = (cli: MatrixClient, groupId: string): Pr groupId, }, "mx_CreateSpaceFromCommunityDialog_wrapper").finished as Promise<[string?]>; }; + +export const showSpacePreferences = (space: Room, initialTabId?: SpacePreferenceTab): Promise => { + return Modal.createTrackedDialog("Space preferences", "", SpacePreferencesDialog, { + initialTabId, + space, + }, null, false, true).finished; +}; From fe15f7ecc76acdbcdb49d11573ff688430db4ada Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 13:45:02 +0000 Subject: [PATCH 02/26] SpaceStore refactor space hierarchy building --- src/stores/spaces/SpaceStore.ts | 124 +++++++++++++++++++++++++++----- 1 file changed, 105 insertions(+), 19 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 67b97855ef4..c8a8ad9c873 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -356,6 +356,107 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return this.spaceFilteredRooms.get(space) || new Set(); }; + private markTreeChildren = (rootSpace: Room, unseen: Set): void => { + const stack = [rootSpace]; + while (stack.length) { + const space = stack.pop(); + unseen.delete(space); + this.getChildSpaces(space.roomId).forEach(space => { + if (unseen.has(space)) { + stack.push(space); + } + }); + } + }; + + private findRootSpaces = (joinedSpaces: Room[]): Room[] => { + // exclude invited spaces from unseenChildren as they will be forcibly shown at the top level of the treeview + const unseenSpaces = new Set(joinedSpaces); + + joinedSpaces.forEach(space => { + this.getChildSpaces(space.roomId).forEach(subspace => { + unseenSpaces.delete(subspace); + }); + }); + + // Consider any spaces remaining in unseenSpaces as root, + // given they are not children of any known spaces. + // The hierarchy from these roots may not yet be exhaustive due to the possibility of full-cycles. + const rootSpaces = Array.from(unseenSpaces); + + // Next we need to determine the roots of any remaining full-cycles. + // We sort spaces by room ID to force the cycle breaking to be deterministic. + const detachedNodes = new Set(sortBy(joinedSpaces, space => space.roomId)); + + // Mark any nodes which are children of our existing root spaces as attached. + rootSpaces.forEach(rootSpace => { + this.markTreeChildren(rootSpace, detachedNodes); + }); + + // Handle spaces forming fully cyclical relationships. + // In order, assume each remaining detachedNode is a root unless it has already + // been claimed as the child of prior detached node. + // Work from a copy of the detachedNodes set as it will be mutated as part of this operation. + // TODO consider sorting by number of in-refs to favour nodes with fewer parents. + Array.from(detachedNodes).forEach(detachedNode => { + if (!detachedNodes.has(detachedNode)) return; // already claimed, skip + // declare this detached node a new root, find its children, without ever looping back to it + rootSpaces.push(detachedNode); // consider this node a new root space + this.markTreeChildren(detachedNode, detachedNodes); // declare this node and its children attached + }); + + return rootSpaces; + }; + + // TODO cull this before this PR can merge + private temporaryWorkaround = (joinedSpaces: Room[]) => { + // if the currently selected space no longer exists, remove its selection + // TODO this probably belongs elsewhere + if (!["join", "invite"].includes(this.activeSpaceRoom?.getMyMembership())) { + this.goToFirstSpace(); + } + + this.parentMap = new EnhancedMap>(); + joinedSpaces.forEach(space => { + const children = this.getChildren(space.roomId); + children.forEach(child => { + this.parentMap.getOrCreate(child.roomId, new Set()).add(space.roomId); + }); + }); + }; + + private rebuildSpaceHierarchy = throttle(() => { + const visibleSpaces = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); + const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce((arr, s) => { + switch (s.getMyMembership()) { + case "join": + arr[0].push(s); + break; + case "invite": + arr[1].push(s); + break; + } + return arr; + }, [[], []] as [Room[], Room[]]); + + const rootSpaces = this.findRootSpaces(joinedSpaces); + const oldRootSpaces = this.rootSpaces; + this.rootSpaces = this.sortRootSpaces(rootSpaces); + + this.temporaryWorkaround(joinedSpaces); + this.onRoomsUpdate(); + + if (arrayHasOrderChange(oldRootSpaces, this.rootSpaces)) { + this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); + } + + const oldInvitedSpaces = this._invitedSpaces; + this._invitedSpaces = new Set(this.sortRootSpaces(invitedSpaces)); + if (setHasDiff(oldInvitedSpaces, this._invitedSpaces)) { + this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + } + }, 100, { trailing: true, leading: true }); + private rebuild = throttle(() => { if (!this.matrixClient) return; @@ -391,21 +492,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // somewhat algorithm to handle full-cycles const detachedNodes = new Set(spaces); - const markTreeChildren = (rootSpace: Room, unseen: Set) => { - const stack = [rootSpace]; - while (stack.length) { - const op = stack.pop(); - unseen.delete(op); - this.getChildSpaces(op.roomId).forEach(space => { - if (unseen.has(space)) { - stack.push(space); - } - }); - } - }; - rootSpaces.forEach(rootSpace => { - markTreeChildren(rootSpace, detachedNodes); + this.markTreeChildren(rootSpace, detachedNodes); }); // Handle spaces forming fully cyclical relationships. @@ -417,7 +505,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // declare this detached node a new root, find its children, without ever looping back to it detachedNodes.delete(detachedNode); rootSpaces.push(detachedNode); - markTreeChildren(detachedNode, detachedNodes); + this.markTreeChildren(detachedNode, detachedNodes); // TODO only consider a detached node a root space if it has no *parents other than the ones forming cycles }); @@ -811,13 +899,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const enabledMetaSpaces = SettingsStore.getValue("Spaces.enabledMetaSpaces"); this._enabledMetaSpaces = metaSpaceOrder.filter(k => enabledMetaSpaces[k]) as MetaSpace[]; - await this.onSpaceUpdate(); // trigger an initial update + this.rebuildSpaceHierarchy(); // trigger an initial update // restore selected state from last session if any and still valid const lastSpaceId = window.localStorage.getItem(ACTIVE_SPACE_LS_KEY); - if (lastSpaceId && ( - lastSpaceId[0] === "!" ? this.matrixClient.getRoom(lastSpaceId) : enabledMetaSpaces[lastSpaceId] - )) { + if (lastSpaceId?.[0] === "!" ? this.matrixClient.getRoom(lastSpaceId) : enabledMetaSpaces[lastSpaceId]) { // don't context switch here as it may break permalinks this.setActiveSpace(lastSpaceId, false); } else { From 56d4790d049759c06fb42dd2272e2c4ef244cb62 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 13:57:55 +0000 Subject: [PATCH 03/26] stop throttling and inline method --- src/stores/spaces/SpaceStore.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index c8a8ad9c873..19fba769412 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -425,7 +425,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); }; - private rebuildSpaceHierarchy = throttle(() => { + private rebuildSpaceHierarchy = () => { const visibleSpaces = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce((arr, s) => { switch (s.getMyMembership()) { @@ -455,7 +455,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (setHasDiff(oldInvitedSpaces, this._invitedSpaces)) { this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); } - }, 100, { trailing: true, leading: true }); + }; private rebuild = throttle(() => { if (!this.matrixClient) return; @@ -532,10 +532,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); }, 100, { trailing: true, leading: true }); - private onSpaceUpdate = () => { - this.rebuild(); - }; - private showInHomeSpace = (room: Room) => { if (this.allRoomsInHome) return true; if (room.isSpaceRoom()) return false; @@ -751,7 +747,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this._invitedSpaces.delete(room); this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); } else { - this.onSpaceUpdate(); + this.rebuild(); this.emit(room.roomId); } @@ -779,7 +775,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { switch (ev.getType()) { case EventType.SpaceChild: if (room.isSpaceRoom()) { - this.onSpaceUpdate(); + this.rebuild(); this.emit(room.roomId); } @@ -796,7 +792,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // TODO rebuild the space parent and not the room - check permissions? // TODO confirm this after implementing parenting behaviour if (room.isSpaceRoom()) { - this.onSpaceUpdate(); + this.rebuild(); } else if (!this.allRoomsInHome) { this.onRoomUpdate(room); } From a5a2f3b94637b3bb886c799b17181cc9849eb713 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 14:18:07 +0000 Subject: [PATCH 04/26] SpaceStore refactor metaspace building --- src/stores/spaces/SpaceStore.ts | 111 ++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 40 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 19fba769412..536262a1110 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -457,6 +457,62 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; + private rebuildHomeSpace = () => { + if (this.allRoomsInHome) { + // this is a special-case to not have to maintain a set of all rooms + this.spaceFilteredRooms.delete(MetaSpace.Home); + } else { + const visibleRooms = this.matrixClient.getVisibleRooms(); + + // include all room invites in the Home Space + const invites = visibleRooms.filter(r => !r.isSpaceRoom() && r.getMyMembership() === "invite"); + const rooms = new Set(invites.map(r => r.roomId)); + + visibleRooms.forEach(room => { + if (this.showInHomeSpace(room)) { + rooms.add(room.roomId); + } + }); + + this.spaceFilteredRooms.set(MetaSpace.Home, rooms); + } + }; + + private rebuildMetaSpaces = () => { + const enabledMetaSpaces = new Set(this.enabledMetaSpaces); + const visibleRooms = this.matrixClient.getVisibleRooms(); + + if (enabledMetaSpaces.has(MetaSpace.Home)) { + this.rebuildHomeSpace(); + } else { + this.spaceFilteredRooms.delete(MetaSpace.Home); + } + + if (enabledMetaSpaces.has(MetaSpace.Favourites)) { + const favourites = visibleRooms.filter(r => r.tags[DefaultTagID.Favourite]); + this.spaceFilteredRooms.set(MetaSpace.Favourites, new Set(favourites.map(r => r.roomId))); + } else { + this.spaceFilteredRooms.delete(MetaSpace.Favourites); + } + + if (enabledMetaSpaces.has(MetaSpace.People)) { + const people = visibleRooms.filter(r => DMRoomMap.shared().getUserIdForRoomId(r.roomId)); + this.spaceFilteredRooms.set(MetaSpace.People, new Set(people.map(r => r.roomId))); + } else { + this.spaceFilteredRooms.delete(MetaSpace.People); + } + + if (enabledMetaSpaces.has(MetaSpace.Orphans)) { + const orphans = visibleRooms.filter(r => { + // filter out DMs and rooms with >0 parents + return !this.parentMap.get(r.roomId)?.size && !DMRoomMap.shared().getUserIdForRoomId(r.roomId); + }); + this.spaceFilteredRooms.set(MetaSpace.Orphans, new Set(orphans.map(r => r.roomId))); + } else { + this.spaceFilteredRooms.delete(MetaSpace.Orphans); + } + }; + private rebuild = throttle(() => { if (!this.matrixClient) return; @@ -532,11 +588,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); }, 100, { trailing: true, leading: true }); - private showInHomeSpace = (room: Room) => { + private showInHomeSpace = (room: Room): boolean => { 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 + || !!DMRoomMap.shared().getUserIdForRoomId(room.roomId); // put all DMs in the Home Space }; // Update a given room due to its tag changing (e.g DM-ness or Fav-ness) @@ -568,40 +624,15 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldFilteredRooms = this.spaceFilteredRooms; this.spaceFilteredRooms = new Map(); - const enabledMetaSpaces = new Set(this.enabledMetaSpaces); - // populate the Home metaspace if it is enabled and is not set to all rooms - if (enabledMetaSpaces.has(MetaSpace.Home) && !this.allRoomsInHome) { - // put all room invites in the Home Space - const invites = visibleRooms.filter(r => !r.isSpaceRoom() && r.getMyMembership() === "invite"); - this.spaceFilteredRooms.set(MetaSpace.Home, new Set(invites.map(r => r.roomId))); - - visibleRooms.forEach(room => { - if (this.showInHomeSpace(room)) { - this.spaceFilteredRooms.get(MetaSpace.Home).add(room.roomId); - } - }); - } - - // populate the Favourites metaspace if it is enabled - if (enabledMetaSpaces.has(MetaSpace.Favourites)) { - const favourites = visibleRooms.filter(r => r.tags[DefaultTagID.Favourite]); - this.spaceFilteredRooms.set(MetaSpace.Favourites, new Set(favourites.map(r => r.roomId))); - } + // TODO do we need this here? + // copy metaspaces into new filter as they are not owned by this method + // metaSpaceOrder.forEach(spaceKey => { + // this.spaceFilteredRooms.set(spaceKey, oldFilteredRooms.get(spaceKey)); + // }); - // populate the People metaspace if it is enabled - if (enabledMetaSpaces.has(MetaSpace.People)) { - const people = visibleRooms.filter(r => DMRoomMap.shared().getUserIdForRoomId(r.roomId)); - this.spaceFilteredRooms.set(MetaSpace.People, new Set(people.map(r => r.roomId))); - } + this.rebuildMetaSpaces(); - // populate the Orphans metaspace if it is enabled - if (enabledMetaSpaces.has(MetaSpace.Orphans)) { - const orphans = visibleRooms.filter(r => { - // filter out DMs and rooms with >0 parents - return !this.parentMap.get(r.roomId)?.size && !DMRoomMap.shared().getUserIdForRoomId(r.roomId); - }); - this.spaceFilteredRooms.set(MetaSpace.Orphans, new Set(orphans.map(r => r.roomId))); - } + const enabledMetaSpaces = new Set(this.enabledMetaSpaces); const hiddenChildren = new EnhancedMap>(); visibleRooms.forEach(room => { @@ -976,7 +1007,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (this.allRoomsInHome !== newValue) { this._allRoomsInHome = newValue; this.emit(UPDATE_HOME_BEHAVIOUR, this.allRoomsInHome); - this.rebuild(); // rebuild everything + if (this.enabledMetaSpaces.includes(MetaSpace.Home)) { + this.rebuildHomeSpace(); + } } break; } @@ -986,14 +1019,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const enabledMetaSpaces = metaSpaceOrder.filter(k => newValue[k]) as MetaSpace[]; if (arrayHasDiff(this._enabledMetaSpaces, enabledMetaSpaces)) { this._enabledMetaSpaces = enabledMetaSpaces; - // if a metaspace currently being viewed was remove, go to another one - if (this.activeSpace[0] !== "!" && - !enabledMetaSpaces.includes(this.activeSpace as MetaSpace) - ) { + // if a metaspace currently being viewed was removed, go to another one + if (this.activeSpace[0] !== "!" && !newValue[this.activeSpace]) { this.goToFirstSpace(); } + this.rebuildMetaSpaces(); this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); - this.rebuild(); // rebuild everything } break; } From e4ce60203a3c66915c881a539764cd5da45e393e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 15:00:27 +0000 Subject: [PATCH 05/26] SpaceStore refactor hide internals of room filtering behind isRoomInSpace --- src/components/views/rooms/NewRoomIntro.tsx | 2 +- .../room-list/filters/SpaceFilterCondition.ts | 10 ++- src/stores/spaces/SpaceStore.ts | 42 ++++++++-- test/stores/SpaceStore-test.ts | 84 +++++++++---------- 4 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/components/views/rooms/NewRoomIntro.tsx b/src/components/views/rooms/NewRoomIntro.tsx index 100b1ca4350..6031fcca7e5 100644 --- a/src/components/views/rooms/NewRoomIntro.tsx +++ b/src/components/views/rooms/NewRoomIntro.tsx @@ -129,7 +129,7 @@ const NewRoomIntro = () => { let parentSpace: Room; if ( SpaceStore.instance.activeSpaceRoom?.canInvite(cli.getUserId()) && - SpaceStore.instance.getSpaceFilteredRoomIds(SpaceStore.instance.activeSpace).has(room.roomId) + SpaceStore.instance.isRoomInSpace(SpaceStore.instance.activeSpace, room.roomId) ) { parentSpace = SpaceStore.instance.activeSpaceRoom; } diff --git a/src/stores/room-list/filters/SpaceFilterCondition.ts b/src/stores/room-list/filters/SpaceFilterCondition.ts index fd815bf86fc..806ca427a6e 100644 --- a/src/stores/room-list/filters/SpaceFilterCondition.ts +++ b/src/stores/room-list/filters/SpaceFilterCondition.ts @@ -22,6 +22,7 @@ import { IDestroyable } from "../../../utils/IDestroyable"; import SpaceStore from "../../spaces/SpaceStore"; import { MetaSpace, SpaceKey } from "../../spaces"; import { setHasDiff } from "../../../utils/sets"; +import DMRoomMap from "../../../utils/DMRoomMap"; /** * A filter condition for the room list which reveals rooms which @@ -31,6 +32,7 @@ import { setHasDiff } from "../../../utils/sets"; */ export class SpaceFilterCondition extends EventEmitter implements IFilterCondition, IDestroyable { private roomIds = new Set(); + private userIds = new Set(); private space: SpaceKey = MetaSpace.Home; public get kind(): FilterKind { @@ -38,7 +40,7 @@ export class SpaceFilterCondition extends EventEmitter implements IFilterConditi } public isVisible(room: Room): boolean { - return this.roomIds.has(room.roomId); + return SpaceStore.instance.isRoomInSpace(this.space, room.roomId); } private onStoreUpdate = async (): Promise => { @@ -46,7 +48,11 @@ export class SpaceFilterCondition extends EventEmitter implements IFilterConditi // clone the set as it may be mutated by the space store internally this.roomIds = new Set(SpaceStore.instance.getSpaceFilteredRoomIds(this.space)); - if (setHasDiff(beforeRoomIds, this.roomIds)) { + const beforeUserIds = this.userIds; + // clone the set as it may be mutated by the space store internally + this.userIds = new Set(SpaceStore.instance.getSpaceFilteredUserIds(this.space)); + + if (setHasDiff(beforeRoomIds, this.roomIds) || setHasDiff(beforeUserIds, this.userIds)) { this.emit(FILTER_CHANGED); // XXX: Room List Store has a bug where updates to the pre-filter during a local echo of a // tags transition seem to be ignored, so refire in the next tick to work around it diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 536262a1110..6bbdd56d8f9 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -101,6 +101,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { private notificationStateMap = new Map(); // Map from space key to Set of room IDs that should be shown as part of that space's filter private spaceFilteredRooms = new Map>(); + // Map from space key to Set of user IDs that should be shown as part of that space's filter + private spaceFilteredUsers = new Map>(); // TODO actually implement // The space currently selected in the Space Panel private _activeSpace?: SpaceKey = MetaSpace.Home; // set properly by onReady private _suggestedRooms: ISuggestedRoom[] = []; @@ -215,7 +217,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // else view space home or home depending on what is being clicked on if (cliSpace?.getMyMembership() !== "invite" && this.matrixClient.getRoom(roomId)?.getMyMembership() === "join" && - this.getSpaceFilteredRoomIds(space).has(roomId) + this.isRoomInSpace(space, roomId) ) { defaultDispatcher.dispatch({ action: "view_room", @@ -349,13 +351,42 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return this.parentMap.get(roomId) || new Set(); } + public isRoomInSpace(space: SpaceKey, roomId: string): boolean { + if (space === MetaSpace.Home && this.allRoomsInHome && this.enabledMetaSpaces.includes(MetaSpace.Home)) { + return true; + } + + if (this.spaceFilteredRooms.get(space)?.has(roomId)) { + return true; + } + + const dmPartner = DMRoomMap.shared().getUserIdForRoomId(roomId); + if (dmPartner && this.spaceFilteredUsers.get(space)?.has(dmPartner)) { + return true; + } + + return false; + } + + // TODO deprecate public getSpaceFilteredRoomIds = (space: SpaceKey): Set => { - if (space === MetaSpace.Home && this.allRoomsInHome) { + if (space === MetaSpace.Home && this.allRoomsInHome && this.enabledMetaSpaces.includes(MetaSpace.Home)) { return new Set(this.matrixClient.getVisibleRooms().map(r => r.roomId)); } return this.spaceFilteredRooms.get(space) || new Set(); }; + // TODO deprecate + public getSpaceFilteredUserIds = (space: SpaceKey): Set => { + if (space === MetaSpace.Home && this.allRoomsInHome) { + return undefined; + } + if (space === MetaSpace.People || space === MetaSpace.Orphans) { + return undefined; + } + return this.spaceFilteredUsers.get(space) || new Set(); + }; + private markTreeChildren = (rootSpace: Room, unseen: Set): void => { const stack = [rootSpace]; while (stack.length) { @@ -733,7 +764,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // otherwise, try to find a metaspace which contains this room if (!parent) { // search meta spaces in reverse as Home is the first and least specific one - parent = [...this.enabledMetaSpaces].reverse().find(s => this.getSpaceFilteredRoomIds(s).has(roomId)); + parent = [...this.enabledMetaSpaces].reverse().find(s => this.isRoomInSpace(s, roomId)); } // don't trigger a context switch when we are switching a space to match the chosen room @@ -962,7 +993,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // Don't context switch when navigating to the space room // as it will cause you to end up in the wrong room this.setActiveSpace(room.roomId, false); - } else if (!this.getSpaceFilteredRoomIds(this.activeSpace).has(roomId)) { + } else if (!this.isRoomInSpace(this.activeSpace, roomId)) { this.switchToRelatedSpace(roomId); } @@ -1031,7 +1062,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { case "Spaces.showPeopleInSpace": // TODO extract space DMs from space filtered rooms to simplify the data flow - this.rebuild(); // rebuild everything + // getSpaceFilteredUserIds will return the appropriate value + this.emit(settingUpdatedPayload.roomId); break; } } diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 228d7da0597..3ca542f9b35 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -388,84 +388,84 @@ describe("SpaceStore", () => { }); it("home space contains orphaned rooms", () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(orphan1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(orphan2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, orphan1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, orphan2)).toBeTruthy(); }); it("home space does not contain all favourites", () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(fav1)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(fav2)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(fav3)).toBeFalsy(); + expect(store.isRoomInSpace(MetaSpace.Home, fav1)).toBeFalsy(); + expect(store.isRoomInSpace(MetaSpace.Home, fav2)).toBeFalsy(); + expect(store.isRoomInSpace(MetaSpace.Home, fav3)).toBeFalsy(); }); it("home space contains dm rooms", () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(dm1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(dm2)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(dm3)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, dm1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, dm2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, dm3)).toBeTruthy(); }); it("home space contains invites", () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(invite1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, invite1)).toBeTruthy(); }); it("home space contains invites even if they are also shown in a space", () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(invite2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, invite2)).toBeTruthy(); }); 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(MetaSpace.Home).has(room1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, room1)).toBeTruthy(); }); it("favourites space does contain favourites even if they are also shown in a space", async () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.Favourites).has(fav1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Favourites).has(fav2)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Favourites).has(fav3)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Favourites, fav1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Favourites, fav2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Favourites, fav3)).toBeTruthy(); }); it("people space does contain people even if they are also shown in a space", async () => { - expect(store.getSpaceFilteredRoomIds(MetaSpace.People).has(dm1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.People).has(dm2)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.People).has(dm3)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.People, dm1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.People, dm2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.People, dm3)).toBeTruthy(); }); it("orphans space does contain orphans even if they are also shown in all rooms", async () => { await setShowAllRooms(true); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Orphans).has(orphan1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Orphans).has(orphan2)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Orphans, orphan1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Orphans, orphan2)).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(MetaSpace.Home).has(room1)).toBeFalsy(); + expect(store.isRoomInSpace(MetaSpace.Home, room1)).toBeFalsy(); }); it("space contains child rooms", () => { - expect(store.getSpaceFilteredRoomIds(space1).has(fav1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space1).has(room1)).toBeTruthy(); + expect(store.isRoomInSpace(space1, fav1)).toBeTruthy(); + expect(store.isRoomInSpace(space1, room1)).toBeTruthy(); }); it("space contains child favourites", () => { - expect(store.getSpaceFilteredRoomIds(space2).has(fav1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space2).has(fav2)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space2).has(fav3)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space2).has(room1)).toBeTruthy(); + expect(store.isRoomInSpace(space2, fav1)).toBeTruthy(); + expect(store.isRoomInSpace(space2, fav2)).toBeTruthy(); + expect(store.isRoomInSpace(space2, fav3)).toBeTruthy(); + expect(store.isRoomInSpace(space2, room1)).toBeTruthy(); }); it("space contains child invites", () => { - expect(store.getSpaceFilteredRoomIds(space3).has(invite2)).toBeTruthy(); + expect(store.isRoomInSpace(space3, invite2)).toBeTruthy(); }); it("spaces contain dms which you have with members of that space", () => { - expect(store.getSpaceFilteredRoomIds(space1).has(dm1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space2).has(dm1)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space3).has(dm1)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space1).has(dm2)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space2).has(dm2)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space3).has(dm2)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space1).has(dm3)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space2).has(dm3)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(space3).has(dm3)).toBeFalsy(); + expect(store.isRoomInSpace(space1, dm1)).toBeTruthy(); + expect(store.isRoomInSpace(space2, dm1)).toBeFalsy(); + expect(store.isRoomInSpace(space3, dm1)).toBeFalsy(); + expect(store.isRoomInSpace(space1, dm2)).toBeFalsy(); + expect(store.isRoomInSpace(space2, dm2)).toBeTruthy(); + expect(store.isRoomInSpace(space3, dm2)).toBeFalsy(); + expect(store.isRoomInSpace(space1, dm3)).toBeFalsy(); + expect(store.isRoomInSpace(space2, dm3)).toBeFalsy(); + expect(store.isRoomInSpace(space3, dm3)).toBeFalsy(); }); it("dms are only added to Notification States for only the Home Space", () => { @@ -517,11 +517,11 @@ describe("SpaceStore", () => { }); it("honours m.space.parent if sender has permission in parent space", () => { - expect(store.getSpaceFilteredRoomIds(space2).has(room2)).toBeTruthy(); + expect(store.isRoomInSpace(space2, room2)).toBeTruthy(); }); it("does not honour m.space.parent if sender does not have permission in parent space", () => { - expect(store.getSpaceFilteredRoomIds(space3).has(room3)).toBeFalsy(); + expect(store.isRoomInSpace(space3, room3)).toBeFalsy(); }); }); }); @@ -612,8 +612,8 @@ describe("SpaceStore", () => { expect(store.invitedSpaces).toStrictEqual([]); expect(store.getChildSpaces(space1)).toStrictEqual([]); expect(store.getChildRooms(space1)).toStrictEqual([]); - expect(store.getSpaceFilteredRoomIds(space1).has(invite1)).toBeFalsy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(invite1)).toBeFalsy(); + expect(store.isRoomInSpace(space1, invite1)).toBeFalsy(); + expect(store.isRoomInSpace(MetaSpace.Home, invite1)).toBeFalsy(); const invite = mkRoom(invite1); invite.getMyMembership.mockReturnValue("invite"); @@ -625,8 +625,8 @@ describe("SpaceStore", () => { expect(store.invitedSpaces).toStrictEqual([]); expect(store.getChildSpaces(space1)).toStrictEqual([]); expect(store.getChildRooms(space1)).toStrictEqual([invite]); - expect(store.getSpaceFilteredRoomIds(space1).has(invite1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(MetaSpace.Home).has(invite1)).toBeTruthy(); + expect(store.isRoomInSpace(space1, invite1)).toBeTruthy(); + expect(store.isRoomInSpace(MetaSpace.Home, invite1)).toBeTruthy(); }); }); From e3208e5b5c64ceeb4df44777917eca414603b5ef Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 15:22:25 +0000 Subject: [PATCH 06/26] SpaceStore refactor building notification states --- src/stores/spaces/SpaceStore.ts | 79 ++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 6bbdd56d8f9..f55b9f62d80 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -352,7 +352,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public isRoomInSpace(space: SpaceKey, roomId: string): boolean { - if (space === MetaSpace.Home && this.allRoomsInHome && this.enabledMetaSpaces.includes(MetaSpace.Home)) { + if (space === MetaSpace.Home && this.allRoomsInHome) { return true; } @@ -361,6 +361,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } const dmPartner = DMRoomMap.shared().getUserIdForRoomId(roomId); + if (dmPartner && space === MetaSpace.People) { + return true; + } if (dmPartner && this.spaceFilteredUsers.get(space)?.has(dmPartner)) { return true; } @@ -370,7 +373,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // TODO deprecate public getSpaceFilteredRoomIds = (space: SpaceKey): Set => { - if (space === MetaSpace.Home && this.allRoomsInHome && this.enabledMetaSpaces.includes(MetaSpace.Home)) { + if (space === MetaSpace.Home && this.allRoomsInHome) { return new Set(this.matrixClient.getVisibleRooms().map(r => r.roomId)); } return this.spaceFilteredRooms.get(space) || new Set(); @@ -526,12 +529,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.spaceFilteredRooms.delete(MetaSpace.Favourites); } - if (enabledMetaSpaces.has(MetaSpace.People)) { - const people = visibleRooms.filter(r => DMRoomMap.shared().getUserIdForRoomId(r.roomId)); - this.spaceFilteredRooms.set(MetaSpace.People, new Set(people.map(r => r.roomId))); - } else { - this.spaceFilteredRooms.delete(MetaSpace.People); - } + // The People metaspace doesn't need maintaining if (enabledMetaSpaces.has(MetaSpace.Orphans)) { const orphans = visibleRooms.filter(r => { @@ -544,6 +542,43 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; + // TODO consider when we need to update notificationStates, support rebuilding just the ones which changes + private updateNotificationStates = () => { + const enabledMetaSpaces = new Set(this.enabledMetaSpaces); + const visibleRooms = this.matrixClient.getVisibleRooms(); + + let dmBadgeSpace: MetaSpace; + // only show badges on dms on the most relevant space if such exists + if (enabledMetaSpaces.has(MetaSpace.People)) { + dmBadgeSpace = MetaSpace.People; + } else if (enabledMetaSpaces.has(MetaSpace.Home)) { + dmBadgeSpace = MetaSpace.Home; + } + + this.spaceFilteredRooms.forEach((roomIds, s) => { + if (this.allRoomsInHome && s === MetaSpace.Home) return; // we'll be using the global notification state, skip + + // Update NotificationStates + this.getNotificationState(s).setRooms(visibleRooms.filter(room => { + if (!roomIds.has(room.roomId) || room.isSpaceRoom()) return false; + + if (dmBadgeSpace && DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + return s === dmBadgeSpace; + } + + return true; + })); + }); + + if (dmBadgeSpace === MetaSpace.People) { + this.getNotificationState(MetaSpace.People).setRooms(visibleRooms.filter(room => { + return this.isRoomInSpace(MetaSpace.People, room.roomId); + })); + } else { + this.notificationStateMap.delete(MetaSpace.People); + } + }; + private rebuild = throttle(() => { if (!this.matrixClient) return; @@ -663,8 +698,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.rebuildMetaSpaces(); - const enabledMetaSpaces = new Set(this.enabledMetaSpaces); - const hiddenChildren = new EnhancedMap>(); visibleRooms.forEach(room => { if (room.getMyMembership() !== "join") return; @@ -689,6 +722,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const space = this.matrixClient?.getRoom(spaceId); // Add relevant DMs + // TODO if (SettingsStore.getValue("Spaces.showPeopleInSpace", spaceId)) { space?.getMembers().forEach(member => { if (member.membership !== "join" && member.membership !== "invite") return; @@ -719,6 +753,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { fn(s.roomId, new Set()); }); + // TODO diff members const diff = mapDiff(oldFilteredRooms, this.spaceFilteredRooms); // filter out keys which changed by reference only by checking whether the sets differ const changed = diff.changed.filter(k => setHasDiff(oldFilteredRooms.get(k), this.spaceFilteredRooms.get(k))); @@ -726,28 +761,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(k); }); - let dmBadgeSpace: MetaSpace; - // only show badges on dms on the most relevant space if such exists - if (enabledMetaSpaces.has(MetaSpace.People)) { - dmBadgeSpace = MetaSpace.People; - } else if (enabledMetaSpaces.has(MetaSpace.Home)) { - dmBadgeSpace = MetaSpace.Home; - } - - this.spaceFilteredRooms.forEach((roomIds, s) => { - if (this.allRoomsInHome && s === MetaSpace.Home) return; // we'll be using the global notification state, skip - - // Update NotificationStates - this.getNotificationState(s).setRooms(visibleRooms.filter(room => { - if (!roomIds.has(room.roomId) || room.isSpaceRoom()) return false; - - if (dmBadgeSpace && DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { - return s === dmBadgeSpace; - } - - return true; - })); - }); + this.updateNotificationStates(); }, 100, { trailing: true, leading: true }); private switchToRelatedSpace = (roomId: string) => { @@ -1055,6 +1069,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.goToFirstSpace(); } this.rebuildMetaSpaces(); + this.updateNotificationStates(); // TODO we could skip some work here this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); } break; From 58fca9f7af53e0b6afa4a83b40ad25a4b8fa3f4c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:01:06 +0000 Subject: [PATCH 07/26] Fix WatchManager IRRELEVANT_ROOM --- src/settings/WatchManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/WatchManager.ts b/src/settings/WatchManager.ts index 744d75b136f..9eef8a5dbd9 100644 --- a/src/settings/WatchManager.ts +++ b/src/settings/WatchManager.ts @@ -18,7 +18,7 @@ import { SettingLevel } from "./SettingLevel"; export type CallbackFn = (changedInRoomId: string, atLevel: SettingLevel, newValAtLevel: any) => void; -const IRRELEVANT_ROOM = Symbol("irrelevant-room"); +const IRRELEVANT_ROOM: string = null; /** * Generalized management class for dealing with watchers on a per-handler (per-level) From 899c0caeeb3dfb1ea48739bd7d5e891180f73e9f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:01:26 +0000 Subject: [PATCH 08/26] Wire up `Spaces.showPeopleInSpace` --- .../room-list/filters/SpaceFilterCondition.ts | 12 ++- src/stores/spaces/SpaceStore.ts | 84 ++++++++++++------- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/src/stores/room-list/filters/SpaceFilterCondition.ts b/src/stores/room-list/filters/SpaceFilterCondition.ts index 806ca427a6e..b2d86b459ac 100644 --- a/src/stores/room-list/filters/SpaceFilterCondition.ts +++ b/src/stores/room-list/filters/SpaceFilterCondition.ts @@ -22,7 +22,7 @@ import { IDestroyable } from "../../../utils/IDestroyable"; import SpaceStore from "../../spaces/SpaceStore"; import { MetaSpace, SpaceKey } from "../../spaces"; import { setHasDiff } from "../../../utils/sets"; -import DMRoomMap from "../../../utils/DMRoomMap"; +import SettingsStore from "../../../settings/SettingsStore"; /** * A filter condition for the room list which reveals rooms which @@ -33,6 +33,7 @@ import DMRoomMap from "../../../utils/DMRoomMap"; export class SpaceFilterCondition extends EventEmitter implements IFilterCondition, IDestroyable { private roomIds = new Set(); private userIds = new Set(); + private showPeopleInSpace = true; private space: SpaceKey = MetaSpace.Home; public get kind(): FilterKind { @@ -52,7 +53,14 @@ export class SpaceFilterCondition extends EventEmitter implements IFilterConditi // clone the set as it may be mutated by the space store internally this.userIds = new Set(SpaceStore.instance.getSpaceFilteredUserIds(this.space)); - if (setHasDiff(beforeRoomIds, this.roomIds) || setHasDiff(beforeUserIds, this.userIds)) { + const beforeShowPeopleInSpace = this.showPeopleInSpace; + this.showPeopleInSpace = this.space[0] !== "!" || + SettingsStore.getValue("Spaces.showPeopleInSpace", this.space); + + if (beforeShowPeopleInSpace !== this.showPeopleInSpace || + setHasDiff(beforeRoomIds, this.roomIds) || + setHasDiff(beforeUserIds, this.userIds) + ) { this.emit(FILTER_CHANGED); // XXX: Room List Store has a bug where updates to the pre-filter during a local echo of a // tags transition seem to be ignored, so refire in the next tick to work around it diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index f55b9f62d80..811f36e6c4f 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -101,8 +101,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { private notificationStateMap = new Map(); // Map from space key to Set of room IDs that should be shown as part of that space's filter private spaceFilteredRooms = new Map>(); - // Map from space key to Set of user IDs that should be shown as part of that space's filter - private spaceFilteredUsers = new Map>(); // TODO actually implement + // Map from space ID to Set of user IDs that should be shown as part of that space's filter + private spaceFilteredUsers = new Map>(); // The space currently selected in the Space Panel private _activeSpace?: SpaceKey = MetaSpace.Home; // set properly by onReady private _suggestedRooms: ISuggestedRoom[] = []; @@ -117,6 +117,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { SettingsStore.monitorSetting("Spaces.allRoomsInHome", null); SettingsStore.monitorSetting("Spaces.enabledMetaSpaces", null); + SettingsStore.monitorSetting("Spaces.showPeopleInSpace", null); } public get invitedSpaces(): Room[] { @@ -361,10 +362,20 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } const dmPartner = DMRoomMap.shared().getUserIdForRoomId(roomId); - if (dmPartner && space === MetaSpace.People) { + if (!dmPartner) { + return false; + } + // beyond this point we know this is a DM + + if (space === MetaSpace.Home || space === MetaSpace.People) { + // these spaces contain all DMs return true; } - if (dmPartner && this.spaceFilteredUsers.get(space)?.has(dmPartner)) { + + if (space[0] === "!" && + this.spaceFilteredUsers.get(space)?.has(dmPartner) && + SettingsStore.getValue("Spaces.showPeopleInSpace", space) + ) { return true; } @@ -384,9 +395,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (space === MetaSpace.Home && this.allRoomsInHome) { return undefined; } - if (space === MetaSpace.People || space === MetaSpace.Orphans) { - return undefined; - } + if (space[0] !== "!") return undefined; return this.spaceFilteredUsers.get(space) || new Set(); }; @@ -688,7 +697,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const visibleRooms = this.matrixClient.getVisibleRooms(); const oldFilteredRooms = this.spaceFilteredRooms; + const oldFilteredUsers = this.spaceFilteredUsers; this.spaceFilteredRooms = new Map(); + this.spaceFilteredUsers = new Map(); // TODO do we need this here? // copy metaspaces into new filter as they are not owned by this method @@ -709,34 +720,26 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.rootSpaces.forEach(s => { // traverse each space tree in DFS to build up the supersets as you go up, // reusing results from like subtrees. - const fn = (spaceId: string, parentPath: Set): Set => { + const fn = (spaceId: string, parentPath: Set): [Set, Set] => { if (parentPath.has(spaceId)) return; // prevent cycles // reuse existing results if multiple similar branches exist - if (this.spaceFilteredRooms.has(spaceId)) { - return this.spaceFilteredRooms.get(spaceId); + if (this.spaceFilteredRooms.has(spaceId) && this.spaceFilteredUsers.has(spaceId)) { + return [this.spaceFilteredRooms.get(spaceId), this.spaceFilteredUsers.get(spaceId)]; } const [childSpaces, childRooms] = partitionSpacesAndRooms(this.getChildren(spaceId)); const roomIds = new Set(childRooms.map(r => r.roomId)); const space = this.matrixClient?.getRoom(spaceId); - - // Add relevant DMs - // TODO - if (SettingsStore.getValue("Spaces.showPeopleInSpace", spaceId)) { - space?.getMembers().forEach(member => { - if (member.membership !== "join" && member.membership !== "invite") return; - DMRoomMap.shared().getDMRoomsForUserId(member.userId).forEach(roomId => { - roomIds.add(roomId); - }); - }); - } + const userIds = new Set(space?.getMembers().filter(m => { + return m.membership === "join" || m.membership === "invite"; + }).map(m => m.userId)); const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { - fn(childSpace.roomId, newPath)?.forEach(roomId => { - roomIds.add(roomId); - }); + const [rooms, users] = fn(childSpace.roomId, newPath); + rooms.forEach(roomId => roomIds.add(roomId)); + users.forEach(userId => userIds.add(userId)); }); hiddenChildren.get(spaceId)?.forEach(roomId => { roomIds.add(roomId); @@ -747,17 +750,31 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return this.matrixClient.getRoomUpgradeHistory(roomId, true).map(r => r.roomId); })); this.spaceFilteredRooms.set(spaceId, expandedRoomIds); - return expandedRoomIds; + this.spaceFilteredUsers.set(spaceId, userIds); + return [expandedRoomIds, userIds]; }; fn(s.roomId, new Set()); }); - // TODO diff members - const diff = mapDiff(oldFilteredRooms, this.spaceFilteredRooms); + const roomDiff = mapDiff(oldFilteredRooms, this.spaceFilteredRooms); + const userDiff = mapDiff(oldFilteredUsers, this.spaceFilteredUsers); // filter out keys which changed by reference only by checking whether the sets differ - const changed = diff.changed.filter(k => setHasDiff(oldFilteredRooms.get(k), this.spaceFilteredRooms.get(k))); - [...diff.added, ...diff.removed, ...changed].forEach(k => { + const roomsChanged = roomDiff.changed.filter(k => { + return setHasDiff(oldFilteredRooms.get(k), this.spaceFilteredRooms.get(k)); + }); + const usersChanged = userDiff.changed.filter(k => { + return setHasDiff(oldFilteredUsers.get(k), this.spaceFilteredUsers.get(k)); + }); + + new Set([ + ...roomDiff.added, + ...userDiff.added, + ...roomDiff.removed, + ...userDiff.removed, + ...roomsChanged, + ...usersChanged, + ]).forEach(k => { this.emit(k); }); @@ -988,7 +1005,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } protected async onAction(payload: ActionPayload) { - if (!spacesEnabled) return; + if (!spacesEnabled || !this.matrixClient) return; + switch (payload.action) { case "view_room": { // Don't auto-switch rooms when reacting to a context-switch @@ -1002,7 +1020,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (!roomId) return; // we'll get re-fired with the room ID shortly - const room = this.matrixClient?.getRoom(roomId); + const room = this.matrixClient.getRoom(roomId); if (room?.isSpaceRoom()) { // Don't context switch when navigating to the space room // as it will cause you to end up in the wrong room @@ -1076,9 +1094,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } case "Spaces.showPeopleInSpace": - // TODO extract space DMs from space filtered rooms to simplify the data flow // getSpaceFilteredUserIds will return the appropriate value this.emit(settingUpdatedPayload.roomId); + if (!this.enabledMetaSpaces.some(s => s === MetaSpace.Home || s === MetaSpace.People)) { + this.updateNotificationStates(); // TODO only update space `settingUpdatedPayload.roomId` + } break; } } From 4c9c7637d4fb23c8ebaee94b8d3e42151550acbd Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:15:23 +0000 Subject: [PATCH 09/26] Round 1 of optimisations --- src/stores/spaces/SpaceStore.ts | 110 +++++++++----------------------- 1 file changed, 30 insertions(+), 80 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 811f36e6c4f..d5c606c25e4 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -466,6 +466,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.parentMap.getOrCreate(child.roomId, new Set()).add(space.roomId); }); }); + + const joinedRooms = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); + this.orphanedRooms = new Set(joinedRooms.filter(r => !this.parentMap.get(r.roomId)?.size).map(r => r.roomId)); }; private rebuildSpaceHierarchy = () => { @@ -552,7 +555,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }; // TODO consider when we need to update notificationStates, support rebuilding just the ones which changes - private updateNotificationStates = () => { + private updateNotificationStates = (spaces?: SpaceKey[]) => { const enabledMetaSpaces = new Set(this.enabledMetaSpaces); const visibleRooms = this.matrixClient.getVisibleRooms(); @@ -588,79 +591,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; + // TODO cull private rebuild = throttle(() => { if (!this.matrixClient) return; - - const [visibleSpaces, visibleRooms] = partitionSpacesAndRooms(this.matrixClient.getVisibleRooms()); - const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce((arr, s) => { - if (s.getMyMembership() === "join") { - arr[0].push(s); - } else if (s.getMyMembership() === "invite") { - arr[1].push(s); - } - return arr; - }, [[], []]); - - // exclude invited spaces from unseenChildren as they will be forcibly shown at the top level of the treeview - const unseenChildren = new Set([...visibleRooms, ...joinedSpaces]); - const backrefs = new EnhancedMap>(); - - // Sort spaces by room ID to force the cycle breaking to be deterministic - const spaces = sortBy(joinedSpaces, space => space.roomId); - - // TODO handle cleaning up links when a Space is removed - spaces.forEach(space => { - const children = this.getChildren(space.roomId); - children.forEach(child => { - unseenChildren.delete(child); - - backrefs.getOrCreate(child.roomId, new Set()).add(space.roomId); - }); - }); - - const [rootSpaces, orphanedRooms] = partitionSpacesAndRooms(Array.from(unseenChildren)); - - // somewhat algorithm to handle full-cycles - const detachedNodes = new Set(spaces); - - rootSpaces.forEach(rootSpace => { - this.markTreeChildren(rootSpace, detachedNodes); - }); - - // Handle spaces forming fully cyclical relationships. - // In order, assume each detachedNode is a root unless it has already - // been claimed as the child of prior detached node. - // Work from a copy of the detachedNodes set as it will be mutated as part of this operation. - Array.from(detachedNodes).forEach(detachedNode => { - if (!detachedNodes.has(detachedNode)) return; - // declare this detached node a new root, find its children, without ever looping back to it - detachedNodes.delete(detachedNode); - rootSpaces.push(detachedNode); - this.markTreeChildren(detachedNode, detachedNodes); - - // TODO only consider a detached node a root space if it has no *parents other than the ones forming cycles - }); - - // TODO neither of these handle an A->B->C->A with an additional C->D - // detachedNodes.forEach(space => { - // rootSpaces.push(space); - // }); - - this.orphanedRooms = new Set(orphanedRooms.map(r => r.roomId)); - this.rootSpaces = this.sortRootSpaces(rootSpaces); - this.parentMap = backrefs; - - // if the currently selected space no longer exists, remove its selection - if (this._activeSpace[0] === "!" && detachedNodes.has(this.matrixClient.getRoom(this._activeSpace))) { - this.goToFirstSpace(); - } - - this.onRoomsUpdate(); // TODO only do this if a change has happened - this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); - - // build initial state of invited spaces as we would have missed the emitted events about the room at launch - this._invitedSpaces = new Set(this.sortRootSpaces(invitedSpaces)); - this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + this.rebuildSpaceHierarchy(); }, 100, { trailing: true, leading: true }); private showInHomeSpace = (room: Room): boolean => { @@ -737,9 +671,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { - const [rooms, users] = fn(childSpace.roomId, newPath); - rooms.forEach(roomId => roomIds.add(roomId)); - users.forEach(userId => userIds.add(userId)); + const [rooms, users] = fn(childSpace.roomId, newPath) ?? []; + rooms?.forEach(roomId => roomIds.add(roomId)); + users?.forEach(userId => userIds.add(userId)); }); hiddenChildren.get(spaceId)?.forEach(roomId => { roomIds.add(roomId); @@ -767,18 +701,20 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return setHasDiff(oldFilteredUsers.get(k), this.spaceFilteredUsers.get(k)); }); - new Set([ + const changeSet = new Set([ ...roomDiff.added, ...userDiff.added, ...roomDiff.removed, ...userDiff.removed, ...roomsChanged, ...usersChanged, - ]).forEach(k => { + ]); + + changeSet.forEach(k => { this.emit(k); }); - this.updateNotificationStates(); + this.updateNotificationStates(Array.from(changeSet)); }, 100, { trailing: true, leading: true }); private switchToRelatedSpace = (roomId: string) => { @@ -1081,13 +1017,27 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const newValue = SettingsStore.getValue("Spaces.enabledMetaSpaces"); const enabledMetaSpaces = metaSpaceOrder.filter(k => newValue[k]) as MetaSpace[]; if (arrayHasDiff(this._enabledMetaSpaces, enabledMetaSpaces)) { + const hadPeopleOrHomeEnabled = this.enabledMetaSpaces.some(s => { + return s === MetaSpace.Home || s === MetaSpace.People; + }); this._enabledMetaSpaces = enabledMetaSpaces; + const hasPeopleOrHomeEnabled = this.enabledMetaSpaces.some(s => { + return s === MetaSpace.Home || s === MetaSpace.People; + }); + // if a metaspace currently being viewed was removed, go to another one if (this.activeSpace[0] !== "!" && !newValue[this.activeSpace]) { this.goToFirstSpace(); } this.rebuildMetaSpaces(); - this.updateNotificationStates(); // TODO we could skip some work here + + if (hadPeopleOrHomeEnabled !== hasPeopleOrHomeEnabled) { + // in this case we have to rebuild everything as DM badges will move to/from real spaces + this.updateNotificationStates(); + } else { + this.updateNotificationStates(enabledMetaSpaces); + } + this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); } break; @@ -1097,7 +1047,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // getSpaceFilteredUserIds will return the appropriate value this.emit(settingUpdatedPayload.roomId); if (!this.enabledMetaSpaces.some(s => s === MetaSpace.Home || s === MetaSpace.People)) { - this.updateNotificationStates(); // TODO only update space `settingUpdatedPayload.roomId` + this.updateNotificationStates([settingUpdatedPayload.roomId]); } break; } From 351649471ebecdaf80c75be61b3e5f4222142662 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:27:09 +0000 Subject: [PATCH 10/26] Round 2 of optimisations --- src/stores/spaces/SpaceStore.ts | 71 ++++++++++++++++----------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index d5c606c25e4..62ead676e66 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -93,8 +93,6 @@ const getRoomFn: FetchRoomFn = (room: Room) => { export class SpaceStoreClass extends AsyncStoreWithClient { // 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 - private orphanedRooms = new Set(); // Map from room ID to set of spaces which list it as a child private parentMap = new EnhancedMap>(); // Map from SpaceKey to SpaceNotificationState instance representing that space @@ -451,26 +449,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return rootSpaces; }; - // TODO cull this before this PR can merge - private temporaryWorkaround = (joinedSpaces: Room[]) => { - // if the currently selected space no longer exists, remove its selection - // TODO this probably belongs elsewhere - if (!["join", "invite"].includes(this.activeSpaceRoom?.getMyMembership())) { - this.goToFirstSpace(); - } - - this.parentMap = new EnhancedMap>(); - joinedSpaces.forEach(space => { - const children = this.getChildren(space.roomId); - children.forEach(child => { - this.parentMap.getOrCreate(child.roomId, new Set()).add(space.roomId); - }); - }); - - const joinedRooms = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); - this.orphanedRooms = new Set(joinedRooms.filter(r => !this.parentMap.get(r.roomId)?.size).map(r => r.roomId)); - }; - private rebuildSpaceHierarchy = () => { const visibleSpaces = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce((arr, s) => { @@ -489,7 +467,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldRootSpaces = this.rootSpaces; this.rootSpaces = this.sortRootSpaces(rootSpaces); - this.temporaryWorkaround(joinedSpaces); this.onRoomsUpdate(); if (arrayHasOrderChange(oldRootSpaces, this.rootSpaces)) { @@ -503,6 +480,20 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; + private rebuildParentMap = () => { + const joinedSpaces = this.matrixClient.getVisibleRooms().filter(r => { + return r.isSpaceRoom() && r.getMyMembership() === "join"; + }); + + this.parentMap = new EnhancedMap>(); + joinedSpaces.forEach(space => { + const children = this.getChildren(space.roomId); + children.forEach(child => { + this.parentMap.getOrCreate(child.roomId, new Set()).add(space.roomId); + }); + }); + }; + private rebuildHomeSpace = () => { if (this.allRoomsInHome) { // this is a special-case to not have to maintain a set of all rooms @@ -543,14 +534,14 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // The People metaspace doesn't need maintaining - if (enabledMetaSpaces.has(MetaSpace.Orphans)) { + // Populate the orphans space if the Home space is enabled as it is a superset of it. + // Home is effectively a super set of People + Orphans with the addition of having all invites too. + if (enabledMetaSpaces.has(MetaSpace.Orphans) || enabledMetaSpaces.has(MetaSpace.Home)) { const orphans = visibleRooms.filter(r => { // filter out DMs and rooms with >0 parents return !this.parentMap.get(r.roomId)?.size && !DMRoomMap.shared().getUserIdForRoomId(r.roomId); }); this.spaceFilteredRooms.set(MetaSpace.Orphans, new Set(orphans.map(r => r.roomId))); - } else { - this.spaceFilteredRooms.delete(MetaSpace.Orphans); } }; @@ -610,11 +601,16 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const enabledMetaSpaces = new Set(this.enabledMetaSpaces); // TODO more metaspace stuffs if (enabledMetaSpaces.has(MetaSpace.Home)) { + const homeRooms = this.spaceFilteredRooms.get(MetaSpace.Home); + const len = homeRooms.size; + if (this.showInHomeSpace(room)) { - this.spaceFilteredRooms.get(MetaSpace.Home)?.add(room.roomId); - this.emit(MetaSpace.Home); - } else if (!this.orphanedRooms.has(room.roomId)) { + homeRooms?.add(room.roomId); + } else if (!this.spaceFilteredRooms.get(MetaSpace.Orphans).has(room.roomId)) { this.spaceFilteredRooms.get(MetaSpace.Home)?.delete(room.roomId); + } + + if (len !== homeRooms.size) { this.emit(MetaSpace.Home); } } @@ -641,6 +637,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // this.spaceFilteredRooms.set(spaceKey, oldFilteredRooms.get(spaceKey)); // }); + this.rebuildParentMap(); this.rebuildMetaSpaces(); const hiddenChildren = new EnhancedMap>(); @@ -748,9 +745,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (!room.isSpaceRoom()) { // this.onRoomUpdate(room); - // this.onRoomsUpdate(); - // ideally we only need onRoomsUpdate here but it doesn't rebuild parentMap so always adds new rooms to Home - this.rebuild(); + this.onRoomsUpdate(); if (membership === "join") { // the user just joined a room, remove it from the suggested list if it was there @@ -770,11 +765,15 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // Space if (membership === "invite") { + const len = this._invitedSpaces.size; this._invitedSpaces.add(room); - this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + if (len !== this._invitedSpaces.size) { + this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + } } else if (oldMembership === "invite" && membership !== "join") { - this._invitedSpaces.delete(room); - this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + if (this._invitedSpaces.delete(room)) { + this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + } } else { this.rebuild(); this.emit(room.roomId); @@ -884,10 +883,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { protected async reset() { this.rootSpaces = []; - this.orphanedRooms = new Set(); this.parentMap = new EnhancedMap(); this.notificationStateMap = new Map(); this.spaceFilteredRooms = new Map(); + this.spaceFilteredUsers = new Map(); this._activeSpace = MetaSpace.Home; // set properly by onReady this._suggestedRooms = []; this._invitedSpaces = new Set(); From 87a1414b63b3c7f0e7a42826ffe19dfe0723b484 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:34:32 +0000 Subject: [PATCH 11/26] Fix joining subspaces not showing up immediately in the Space Panel --- src/stores/spaces/SpaceStore.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 62ead676e66..a1672c249c0 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -776,6 +776,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } } else { this.rebuild(); + // fire off updates to all parent listeners + this.parentMap.get(room.roomId)?.forEach((parentId) => { + this.emit(parentId); + }); this.emit(room.roomId); } From 5fcab8ecc61c91c51850ebb833d17e415e39010e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 7 Dec 2021 16:46:27 +0000 Subject: [PATCH 12/26] Round 3 of optimisations --- .../views/spaces/SpaceTreeLevel.tsx | 2 +- src/stores/spaces/SpaceStore.ts | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/components/views/spaces/SpaceTreeLevel.tsx b/src/components/views/spaces/SpaceTreeLevel.tsx index 6d98b277190..127a1c07de5 100644 --- a/src/components/views/spaces/SpaceTreeLevel.tsx +++ b/src/components/views/spaces/SpaceTreeLevel.tsx @@ -176,7 +176,7 @@ export class SpaceItem extends React.PureComponent { ); this.state = { - collapsed: collapsed, + collapsed, childSpaces: this.childSpaces, }; diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index a1672c249c0..a208639b49f 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -582,12 +582,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; - // TODO cull - private rebuild = throttle(() => { - if (!this.matrixClient) return; - this.rebuildSpaceHierarchy(); - }, 100, { trailing: true, leading: true }); - private showInHomeSpace = (room: Room): boolean => { if (this.allRoomsInHome) return true; if (room.isSpaceRoom()) return false; @@ -775,7 +769,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); } } else { - this.rebuild(); + this.rebuildSpaceHierarchy(); // fire off updates to all parent listeners this.parentMap.get(room.roomId)?.forEach((parentId) => { this.emit(parentId); @@ -805,26 +799,34 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (!room) return; switch (ev.getType()) { - case EventType.SpaceChild: + case EventType.SpaceChild: { + const target = this.matrixClient.getRoom(ev.getStateKey()); + if (room.isSpaceRoom()) { - this.rebuild(); + if (target?.isSpaceRoom()) { + this.rebuildSpaceHierarchy(); + this.emit(target.roomId); + } else { + this.onRoomsUpdate(); + } this.emit(room.roomId); } if (room.roomId === this.activeSpace && // current space - this.matrixClient.getRoom(ev.getStateKey())?.getMyMembership() !== "join" && // target not joined + target?.getMyMembership() !== "join" && // target not joined ev.getPrevContent().suggested !== ev.getContent().suggested // suggested flag changed ) { this.loadSuggestedRooms(room); } break; + } case EventType.SpaceParent: // TODO rebuild the space parent and not the room - check permissions? // TODO confirm this after implementing parenting behaviour if (room.isSpaceRoom()) { - this.rebuild(); + this.rebuildSpaceHierarchy(); } else if (!this.allRoomsInHome) { this.onRoomUpdate(room); } From 76801b1a5014cf0a0718dba2a5e7a4b41749938d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Dec 2021 12:33:20 +0000 Subject: [PATCH 13/26] Round 4 of optimisations --- src/stores/spaces/SpaceStore.ts | 197 ++++++++++++++++++++++---------- 1 file changed, 139 insertions(+), 58 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index a208639b49f..72d452b5641 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -14,12 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { ListIteratee, Many, sortBy, throttle } from "lodash"; +import { ListIteratee, Many, sortBy } from "lodash"; import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { Room } from "matrix-js-sdk/src/models/room"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { IRoomCapability } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; +import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; import defaultDispatcher from "../../dispatcher/dispatcher"; @@ -90,6 +91,11 @@ const getRoomFn: FetchRoomFn = (room: Room) => { return RoomNotificationStateStore.instance.getRoomState(room); }; +enum RoomsUpdateCause { + SpaceHierarchy, + RoomMembership, +} + export class SpaceStoreClass extends AsyncStoreWithClient { // The spaces representing the roots of the various tree-like hierarchies private rootSpaces: Room[] = []; @@ -98,7 +104,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // Map from SpaceKey to SpaceNotificationState instance representing that space private notificationStateMap = new Map(); // Map from space key to Set of room IDs that should be shown as part of that space's filter - private spaceFilteredRooms = new Map>(); + private spaceFilteredRooms = new Map>(); // won't contain MetaSpace.People // Map from space ID to Set of user IDs that should be shown as part of that space's filter private spaceFilteredUsers = new Map>(); // The space currently selected in the Space Panel @@ -380,7 +386,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return false; } - // TODO deprecate public getSpaceFilteredRoomIds = (space: SpaceKey): Set => { if (space === MetaSpace.Home && this.allRoomsInHome) { return new Set(this.matrixClient.getVisibleRooms().map(r => r.roomId)); @@ -388,7 +393,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return this.spaceFilteredRooms.get(space) || new Set(); }; - // TODO deprecate public getSpaceFilteredUserIds = (space: SpaceKey): Set => { if (space === MetaSpace.Home && this.allRoomsInHome) { return undefined; @@ -467,7 +471,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldRootSpaces = this.rootSpaces; this.rootSpaces = this.sortRootSpaces(rootSpaces); - this.onRoomsUpdate(); + this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); if (arrayHasOrderChange(oldRootSpaces, this.rootSpaces)) { this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); @@ -499,18 +503,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // this is a special-case to not have to maintain a set of all rooms this.spaceFilteredRooms.delete(MetaSpace.Home); } else { - const visibleRooms = this.matrixClient.getVisibleRooms(); - - // include all room invites in the Home Space - const invites = visibleRooms.filter(r => !r.isSpaceRoom() && r.getMyMembership() === "invite"); - const rooms = new Set(invites.map(r => r.roomId)); - - visibleRooms.forEach(room => { - if (this.showInHomeSpace(room)) { - rooms.add(room.roomId); - } - }); - + const rooms = new Set(this.matrixClient.getVisibleRooms().filter(this.showInHomeSpace).map(r => r.roomId)); this.spaceFilteredRooms.set(MetaSpace.Home, rooms); } }; @@ -545,7 +538,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; - // TODO consider when we need to update notificationStates, support rebuilding just the ones which changes private updateNotificationStates = (spaces?: SpaceKey[]) => { const enabledMetaSpaces = new Set(this.enabledMetaSpaces); const visibleRooms = this.matrixClient.getVisibleRooms(); @@ -558,12 +550,26 @@ export class SpaceStoreClass extends AsyncStoreWithClient { dmBadgeSpace = MetaSpace.Home; } - this.spaceFilteredRooms.forEach((roomIds, s) => { + if (!spaces) { + spaces = [...this.spaceFilteredRooms.keys()]; + if (dmBadgeSpace === MetaSpace.People) { + spaces.push(MetaSpace.People); + } + if (enabledMetaSpaces.has(MetaSpace.Home) && !this.allRoomsInHome) { + spaces.push(MetaSpace.Home); + } + } + + spaces.forEach((s) => { if (this.allRoomsInHome && s === MetaSpace.Home) return; // we'll be using the global notification state, skip // Update NotificationStates this.getNotificationState(s).setRooms(visibleRooms.filter(room => { - if (!roomIds.has(room.roomId) || room.isSpaceRoom()) return false; + if (s === MetaSpace.People) { + return this.isRoomInSpace(MetaSpace.People, room.roomId); + } + + if (room.isSpaceRoom() || !this.spaceFilteredRooms.get(s).has(room.roomId)) return false; if (dmBadgeSpace && DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { return s === dmBadgeSpace; @@ -573,11 +579,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { })); }); - if (dmBadgeSpace === MetaSpace.People) { - this.getNotificationState(MetaSpace.People).setRooms(visibleRooms.filter(room => { - return this.isRoomInSpace(MetaSpace.People, room.roomId); - })); - } else { + if (dmBadgeSpace !== MetaSpace.People) { this.notificationStateMap.delete(MetaSpace.People); } }; @@ -586,38 +588,84 @@ export class SpaceStoreClass extends AsyncStoreWithClient { 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 + || !!DMRoomMap.shared().getUserIdForRoomId(room.roomId) || // put all DMs in the Home Space + room.getMyMembership() === "invite"; // put all invites in the Home Space }; - // Update a given room due to its tag changing (e.g DM-ness or Fav-ness) - // This can only change whether it shows up in the HOME_SPACE or not - private onRoomUpdate = (room: Room) => { - const enabledMetaSpaces = new Set(this.enabledMetaSpaces); - // TODO more metaspace stuffs - if (enabledMetaSpaces.has(MetaSpace.Home)) { - const homeRooms = this.spaceFilteredRooms.get(MetaSpace.Home); - const len = homeRooms.size; + private static isInSpace(member: RoomMember): boolean { + return member.membership === "join" || member.membership === "invite"; + } - if (this.showInHomeSpace(room)) { - homeRooms?.add(room.roomId); - } else if (!this.spaceFilteredRooms.get(MetaSpace.Orphans).has(room.roomId)) { - this.spaceFilteredRooms.get(MetaSpace.Home)?.delete(room.roomId); + private static getSpaceMembers(space: Room): string[] { + return space.getMembers().filter(SpaceStoreClass.isInSpace).map(m => m.userId); + } + + // Method for resolving the impact of a single user's membership change in the given Space and its hierarchy + private onMemberUpdate = (space: Room, userId: string) => { + const inSpace = SpaceStoreClass.isInSpace(space.getMember(userId)); + + if (this.spaceFilteredUsers.get(space.roomId).has(userId)) { + if (inSpace) return; // nothing to do, user was already joined to subspace + if (this.getChildSpaces(space.roomId).some(s => this.spaceFilteredUsers.get(s.roomId).has(userId))) { + return; // nothing to do, this user leaving will have no effect as they are in a subspace } + } else if (!inSpace) { + return; // nothing to do, user already not in the list + } - if (len !== homeRooms.size) { - this.emit(MetaSpace.Home); + const seen = new Set(); + const stack = [space.roomId]; + while (stack.length) { + const spaceId = stack.pop(); + seen.add(spaceId); + + if (inSpace) { + // add to our list and to that of all of our parents + this.spaceFilteredUsers.get(spaceId).add(userId); + } else { + // remove from our list and that of all of our parents until we hit a parent with this user + this.spaceFilteredUsers.get(spaceId).delete(userId); } + + this.getKnownParents(spaceId).forEach(parentId => { + if (seen.has(parentId)) return; + const parent = this.matrixClient.getRoom(parentId); + // because spaceFilteredUsers is cumulative, if we are removing from lower in the hierarchy, + // but the member is present higher in the hierarchy we must take care not to wrongly over-remove them. + if (inSpace || !SpaceStoreClass.isInSpace(parent.getMember(userId))) { + stack.push(parentId); + } + }); } }; - private onSpaceMembersChange = (ev: MatrixEvent) => { - // skip this update if we do not have a DM with this user - if (DMRoomMap.shared().getDMRoomsForUserId(ev.getStateKey()).length < 1) return; - this.onRoomsUpdate(); + private onMembersUpdate = (space: Room, seen = new Set()) => { + // Update this space's membership list + const userIds = new Set(SpaceStoreClass.getSpaceMembers(space)); + // We only need to look one level with children + // as any further descendants will already be in their parent's superset + this.getChildSpaces(space.roomId).forEach(subspace => { + SpaceStoreClass.getSpaceMembers(subspace).forEach(userId => { + userIds.add(userId); + }); + }); + this.spaceFilteredUsers.set(space.roomId, userIds); + this.emit(space.roomId); + + // Traverse all parents and update them too + this.getKnownParents(space.roomId).forEach(parentId => { + if (seen.has(parentId)) return; + const parent = this.matrixClient.getRoom(parentId); + if (parent) { + const newSeen = new Set(seen); + newSeen.add(parentId); + this.onMembersUpdate(parent, newSeen); + } + }); }; - private onRoomsUpdate = throttle(() => { - // TODO resolve some updates as deltas + // TODO use delta updates for some callers of RoomsUpdateCause.SpaceHierarchy + private onRoomsUpdate = (cause: RoomsUpdateCause) => { const visibleRooms = this.matrixClient.getVisibleRooms(); const oldFilteredRooms = this.spaceFilteredRooms; @@ -706,7 +754,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); this.updateNotificationStates(Array.from(changeSet)); - }, 100, { trailing: true, leading: true }); + }; private switchToRelatedSpace = (roomId: string) => { if (this.suggestedRooms.find(r => r.room_id === roomId)) return; @@ -738,8 +786,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const membership = newMembership || roomMembership; if (!room.isSpaceRoom()) { - // this.onRoomUpdate(room); - this.onRoomsUpdate(); + this.onRoomsUpdate(RoomsUpdateCause.RoomMembership); if (membership === "join") { // the user just joined a room, remove it from the suggested list if it was there @@ -807,7 +854,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.rebuildSpaceHierarchy(); this.emit(target.roomId); } else { - this.onRoomsUpdate(); + this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); } this.emit(room.roomId); } @@ -827,15 +874,15 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // TODO confirm this after implementing parenting behaviour if (room.isSpaceRoom()) { this.rebuildSpaceHierarchy(); - } else if (!this.allRoomsInHome) { - this.onRoomUpdate(room); + } else { + this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); } this.emit(room.roomId); break; case EventType.RoomPowerLevels: if (room.isSpaceRoom()) { - this.onRoomsUpdate(); + this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); } break; } @@ -844,8 +891,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // listening for m.room.member events in onRoomState above doesn't work as the Member object isn't updated by then private onRoomStateMembers = (ev: MatrixEvent) => { const room = this.matrixClient.getRoom(ev.getRoomId()); - if (room?.isSpaceRoom()) { - this.onSpaceMembersChange(ev); + const userId = ev.getStateKey(); + if (room?.isSpaceRoom() && // only consider space rooms + DMRoomMap.shared().getDMRoomsForUserId(userId).length > 0 && // only consider members we have a DM with + ev.getPrevContent().membership !== ev.getContent().membership // only consider when membership changes + ) { + this.onMemberUpdate(room, userId); } }; @@ -864,13 +915,43 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldTags = lastEv?.getContent()?.tags || {}; const newTags = ev.getContent()?.tags || {}; if (!!oldTags[DefaultTagID.Favourite] !== !!newTags[DefaultTagID.Favourite]) { - this.onRoomUpdate(room); + this.onRoomFavouriteChange(room); } } }; + private onRoomFavouriteChange(room: Room) { + if (this.enabledMetaSpaces.includes(MetaSpace.Favourites)) { + if (room.tags[DefaultTagID.Favourite]) { + this.spaceFilteredRooms.get(MetaSpace.Favourites).add(room.roomId); + } else { + this.spaceFilteredRooms.get(MetaSpace.Favourites).delete(room.roomId); + } + this.emit(MetaSpace.Favourites); + } + } + + private onRoomDmChange(room: Room) { + const enabledMetaSpaces = new Set(this.enabledMetaSpaces); + + if (!this.allRoomsInHome && enabledMetaSpaces.has(MetaSpace.Home)) { + const homeRooms = this.spaceFilteredRooms.get(MetaSpace.Home); + if (this.showInHomeSpace(room)) { + homeRooms?.add(room.roomId); + } else if (!this.spaceFilteredRooms.get(MetaSpace.Orphans).has(room.roomId)) { + this.spaceFilteredRooms.get(MetaSpace.Home)?.delete(room.roomId); + } + + this.emit(MetaSpace.Home); + } + + if (enabledMetaSpaces.has(MetaSpace.People)) { + this.emit(MetaSpace.People); + } + } + private onAccountData = (ev: MatrixEvent, prevEvent?: MatrixEvent) => { - if (!this.allRoomsInHome && ev.getType() === EventType.Direct) { + if (ev.getType() === EventType.Direct) { const lastContent = prevEvent?.getContent() ?? {}; const content = ev.getContent(); @@ -881,7 +962,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { new Set([...diff.added, ...diff.removed, ...changed]).forEach(roomId => { const room = this.matrixClient?.getRoom(roomId); if (room) { - this.onRoomUpdate(room); + this.onRoomDmChange(room); } }); } From 6d8f744bbf99f341a4a0b0011798a47ef3b4fc14 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Dec 2021 13:23:05 +0000 Subject: [PATCH 14/26] remove unused bits --- src/stores/spaces/SpaceStore.ts | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 72d452b5641..afbafd8cd55 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -91,11 +91,6 @@ const getRoomFn: FetchRoomFn = (room: Room) => { return RoomNotificationStateStore.instance.getRoomState(room); }; -enum RoomsUpdateCause { - SpaceHierarchy, - RoomMembership, -} - export class SpaceStoreClass extends AsyncStoreWithClient { // The spaces representing the roots of the various tree-like hierarchies private rootSpaces: Room[] = []; @@ -471,7 +466,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const oldRootSpaces = this.rootSpaces; this.rootSpaces = this.sortRootSpaces(rootSpaces); - this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); + this.onRoomsUpdate(); if (arrayHasOrderChange(oldRootSpaces, this.rootSpaces)) { this.emit(UPDATE_TOP_LEVEL_SPACES, this.spacePanelSpaces, this.enabledMetaSpaces); @@ -664,8 +659,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); }; - // TODO use delta updates for some callers of RoomsUpdateCause.SpaceHierarchy - private onRoomsUpdate = (cause: RoomsUpdateCause) => { + private onRoomsUpdate = () => { const visibleRooms = this.matrixClient.getVisibleRooms(); const oldFilteredRooms = this.spaceFilteredRooms; @@ -673,12 +667,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.spaceFilteredRooms = new Map(); this.spaceFilteredUsers = new Map(); - // TODO do we need this here? - // copy metaspaces into new filter as they are not owned by this method - // metaSpaceOrder.forEach(spaceKey => { - // this.spaceFilteredRooms.set(spaceKey, oldFilteredRooms.get(spaceKey)); - // }); - this.rebuildParentMap(); this.rebuildMetaSpaces(); @@ -786,7 +774,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const membership = newMembership || roomMembership; if (!room.isSpaceRoom()) { - this.onRoomsUpdate(RoomsUpdateCause.RoomMembership); + this.onRoomsUpdate(); if (membership === "join") { // the user just joined a room, remove it from the suggested list if it was there @@ -854,7 +842,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.rebuildSpaceHierarchy(); this.emit(target.roomId); } else { - this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); + this.onRoomsUpdate(); } this.emit(room.roomId); } @@ -875,14 +863,14 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (room.isSpaceRoom()) { this.rebuildSpaceHierarchy(); } else { - this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); + this.onRoomsUpdate(); } this.emit(room.roomId); break; case EventType.RoomPowerLevels: if (room.isSpaceRoom()) { - this.onRoomsUpdate(RoomsUpdateCause.SpaceHierarchy); + this.onRoomsUpdate(); } break; } From cf4093e13f7c6ec120bdc67d38796ea8ab9b52f4 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Dec 2021 13:33:03 +0000 Subject: [PATCH 15/26] Fix edge case --- src/components/views/rooms/RoomList.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index ad8425e60c3..220b29527ab 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -62,6 +62,7 @@ import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { useEventEmitterState } from "../../../hooks/useEventEmitter"; import { ChevronFace, ContextMenuTooltipButton, useContextMenu } from "../../structures/ContextMenu"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; +import SettingsStore from "../../../settings/SettingsStore"; interface IProps { onKeyDown: (ev: React.KeyboardEvent, state: IRovingTabIndexState) => void; @@ -612,7 +613,11 @@ export default class RoomList extends React.PureComponent { (this.props.activeSpace === MetaSpace.Favourites && orderedTagId !== DefaultTagID.Favourite) || (this.props.activeSpace === MetaSpace.People && orderedTagId !== DefaultTagID.DM) || (this.props.activeSpace === MetaSpace.Orphans && orderedTagId === DefaultTagID.DM) || - (this.props.activeSpace[0] === "!" && orderedTagId === DefaultTagID.DM) // TODO + ( + this.props.activeSpace[0] === "!" && + orderedTagId === DefaultTagID.DM && + !SettingsStore.getValue("Spaces.showPeopleInSpace", this.props.activeSpace) + ) ) { alwaysVisible = false; } From ec3e55951bbf9bf950a060d713ca2c0b166dd41b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Dec 2021 14:05:27 +0000 Subject: [PATCH 16/26] Fix edge case behaviour around orphans and dms --- src/stores/spaces/SpaceStore.ts | 56 ++++++++++++++++++++++++--------- src/utils/sets.ts | 15 +++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index afbafd8cd55..92a3da1c667 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { ListIteratee, Many, sortBy } from "lodash"; +import { ListIteratee, Many, sortBy, throttle } from "lodash"; import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { Room } from "matrix-js-sdk/src/models/room"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; @@ -33,11 +33,10 @@ import { SpaceNotificationState } from "../notifications/SpaceNotificationState" import { RoomNotificationStateStore } from "../notifications/RoomNotificationStateStore"; import { DefaultTagID } from "../room-list/models"; import { EnhancedMap, mapDiff } from "../../utils/maps"; -import { setHasDiff } from "../../utils/sets"; +import { setDiff, setHasDiff } from "../../utils/sets"; import RoomViewStore from "../RoomViewStore"; import { Action } from "../../dispatcher/actions"; import { arrayHasDiff, arrayHasOrderChange } from "../../utils/arrays"; -import { objectDiff } from "../../utils/objects"; import { reorderLexicographically } from "../../utils/stringOrderField"; import { TAG_ORDER } from "../../components/views/rooms/RoomList"; import { SettingUpdatedPayload } from "../../dispatcher/payloads/SettingUpdatedPayload"; @@ -501,6 +500,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const rooms = new Set(this.matrixClient.getVisibleRooms().filter(this.showInHomeSpace).map(r => r.roomId)); this.spaceFilteredRooms.set(MetaSpace.Home, rooms); } + + if (this.activeSpace === MetaSpace.Home) { + this.switchSpaceIfNeeded(); + } }; private rebuildMetaSpaces = () => { @@ -531,6 +534,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); this.spaceFilteredRooms.set(MetaSpace.Orphans, new Set(orphans.map(r => r.roomId))); } + + if (this.activeSpace[0] !== "!") { + this.switchSpaceIfNeeded(); + } }; private updateNotificationStates = (spaces?: SpaceKey[]) => { @@ -632,6 +639,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }); } + + this.switchSpaceIfNeeded(); }; private onMembersUpdate = (space: Room, seen = new Set()) => { @@ -741,9 +750,20 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(k); }); + if (changeSet.has(this.activeSpace)) { + this.switchSpaceIfNeeded(); + } + this.updateNotificationStates(Array.from(changeSet)); }; + private switchSpaceIfNeeded = throttle(() => { + const roomId = RoomViewStore.getRoomId(); + if (!this.matrixClient.getRoom(roomId)?.isSpaceRoom() && !this.isRoomInSpace(this.activeSpace, roomId)) { + this.switchToRelatedSpace(roomId); + } + }, 100, { leading: true, trailing: true }); + private switchToRelatedSpace = (roomId: string) => { if (this.suggestedRooms.find(r => r.room_id === roomId)) return; @@ -919,7 +939,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } } - private onRoomDmChange(room: Room) { + private onRoomDmChange(room: Room, isDm: boolean): void { const enabledMetaSpaces = new Set(this.enabledMetaSpaces); if (!this.allRoomsInHome && enabledMetaSpaces.has(MetaSpace.Home)) { @@ -936,23 +956,31 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (enabledMetaSpaces.has(MetaSpace.People)) { this.emit(MetaSpace.People); } + + if (enabledMetaSpaces.has(MetaSpace.Orphans) || enabledMetaSpaces.has(MetaSpace.Home)) { + if (isDm && this.spaceFilteredRooms.get(MetaSpace.Orphans).delete(room.roomId)) { + this.emit(MetaSpace.Orphans); + this.emit(MetaSpace.Home); + } + } } - private onAccountData = (ev: MatrixEvent, prevEvent?: MatrixEvent) => { + private onAccountData = (ev: MatrixEvent, prevEv?: MatrixEvent) => { if (ev.getType() === EventType.Direct) { - const lastContent = prevEvent?.getContent() ?? {}; - const content = ev.getContent(); - - const diff = objectDiff>(lastContent, content); - // filter out keys which changed by reference only by checking whether the sets differ - const changed = diff.changed.filter(k => arrayHasDiff(lastContent[k], content[k])); - // DM tag changes, refresh relevant rooms - new Set([...diff.added, ...diff.removed, ...changed]).forEach(roomId => { + const previousRooms = new Set(Object.values(prevEv?.getContent>() ?? {}).flat()); + const currentRooms = new Set(Object.values(ev.getContent>()).flat()); + + const diff = setDiff(previousRooms, currentRooms); + [...diff.added, ...diff.removed].forEach(roomId => { const room = this.matrixClient?.getRoom(roomId); if (room) { - this.onRoomDmChange(room); + this.onRoomDmChange(room, currentRooms.has(roomId)); } }); + + if (diff.removed.length > 0) { + this.switchSpaceIfNeeded(); + } } }; diff --git a/src/utils/sets.ts b/src/utils/sets.ts index e5427b2e94b..e5cd8787ba2 100644 --- a/src/utils/sets.ts +++ b/src/utils/sets.ts @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { arrayDiff, arrayUnion } from "./arrays"; + /** * Determines if two sets are different through a shallow comparison. * @param a The first set. Must be defined. @@ -32,3 +34,16 @@ export function setHasDiff(a: Set, b: Set): boolean { return true; // different lengths means they are naturally diverged } } + +type Diff = { added: T[], removed: T[] }; + +/** + * Determines the values added and removed between two sets. + * @param a The first set. Must be defined. + * @param b The second set. Must be defined. + * @returns The difference between the values in each set. + */ +export function setDiff(a: Set, b: Set): Diff { + const keyDiff = arrayDiff(Array.from(a), Array.from(b)); + return { added: keyDiff.added, removed: keyDiff.removed }; +} From 11cb10b1306375faa3e95dc5221100ce8df26ff0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Dec 2021 14:16:16 +0000 Subject: [PATCH 17/26] Fix edge case behaviour around orphans and dms --- src/stores/spaces/SpaceStore.ts | 18 +++++++++++++----- test/stores/SpaceStore-test.ts | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 92a3da1c667..8dcb3abc6a4 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -759,7 +759,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { private switchSpaceIfNeeded = throttle(() => { const roomId = RoomViewStore.getRoomId(); - if (!this.matrixClient.getRoom(roomId)?.isSpaceRoom() && !this.isRoomInSpace(this.activeSpace, roomId)) { + if (this.isRoomInSpace(this.activeSpace, roomId)) return; + + if (this.matrixClient.getRoom(roomId)?.isSpaceRoom()) { + this.goToFirstSpace(true); + } else { this.switchToRelatedSpace(roomId); } }, 100, { leading: true, trailing: true }); @@ -782,7 +786,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } // don't trigger a context switch when we are switching a space to match the chosen room - this.setActiveSpace(parent ?? MetaSpace.Home, false); // TODO + if (parent) { + this.setActiveSpace(parent, false); + } else { + this.goToFirstSpace(); + } }; private onRoom = (room: Room, newMembership?: string, oldMembership?: string) => { @@ -1034,7 +1042,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // don't context switch here as it may break permalinks this.setActiveSpace(lastSpaceId, false); } else { - this.goToFirstSpace(); + this.switchSpaceIfNeeded(); } } @@ -1084,7 +1092,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { case "after_leave_room": if (this._activeSpace[0] === "!" && payload.room_id === this._activeSpace) { // User has left the current space, go to first space - this.goToFirstSpace(); + this.goToFirstSpace(true); } break; @@ -1129,7 +1137,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // if a metaspace currently being viewed was removed, go to another one if (this.activeSpace[0] !== "!" && !newValue[this.activeSpace]) { - this.goToFirstSpace(); + this.switchSpaceIfNeeded(); } this.rebuildMetaSpaces(); diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 3ca542f9b35..7adcad30995 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -817,7 +817,7 @@ describe("SpaceStore", () => { expect(store.activeSpace).toBe(MetaSpace.Orphans); }); - it("switch to first space when selected metaspace is disabled", async () => { + it("switch to first valid space when selected metaspace is disabled", async () => { store.setActiveSpace(MetaSpace.People, false); expect(store.activeSpace).toBe(MetaSpace.People); await SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.DEVICE, { @@ -827,7 +827,7 @@ describe("SpaceStore", () => { [MetaSpace.Orphans]: true, }); jest.runAllTimers(); - expect(store.activeSpace).toBe(MetaSpace.Favourites); + expect(store.activeSpace).toBe(MetaSpace.Orphans); }); it("when switching rooms in the all rooms home space don't switch to related space", async () => { From 905bcc4caa5fb602acce4aab3ba36192db3fa310 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Dec 2021 14:23:38 +0000 Subject: [PATCH 18/26] delint --- src/utils/sets.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/sets.ts b/src/utils/sets.ts index e5cd8787ba2..7dd4dc271a9 100644 --- a/src/utils/sets.ts +++ b/src/utils/sets.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { arrayDiff, arrayUnion } from "./arrays"; +import { arrayDiff } from "./arrays"; /** * Determines if two sets are different through a shallow comparison. From 8252a09cfe525acd74ed10fae3150d6913e2357f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Dec 2021 14:35:49 +0000 Subject: [PATCH 19/26] stash --- test/stores/SpaceStore-test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 7adcad30995..9d6af80e53d 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -889,4 +889,24 @@ describe("SpaceStore", () => { expect(fn).toBeCalledWith("!c:server"); }); }); + + describe("dynamic text fixture", () => { + // receive invite to space + // expect it to appear in SpaceStore.instance.invitedSpaces + + // accept invite to space + // expect it to disappear from SpaceStore.instance.invitedSpaces and move to SpaceStore.instance.spacePanelSpaces + + // join room in space + // expect it to show up in the Room List only for that Space + + // receive room invite + // expect it to show up in the Space and Home + + // start DM in space + // expect it to show up in the Space and Home and People + + // join subspace + // expect an emit on the parent space + }); }); From ba34f8476c92c0e813e9a8b963217f613713864c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Dec 2021 13:26:33 +0000 Subject: [PATCH 20/26] TestyMcTestFace --- test/stores/SpaceStore-test.ts | 141 +++++++++++++++++++++++++-------- test/test-utils.js | 8 +- 2 files changed, 115 insertions(+), 34 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 9d6af80e53d..172da42f7b8 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -35,16 +35,12 @@ import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import defaultDispatcher from "../../src/dispatcher/dispatcher"; import SettingsStore from "../../src/settings/SettingsStore"; import { SettingLevel } from "../../src/settings/SettingLevel"; +import { emitPromise } from "../utils/test-utils"; jest.useFakeTimers(); const testUserId = "@test:user"; -const getUserIdForRoomId = jest.fn(); -const getDMRoomsForUserId = jest.fn(); -// @ts-ignore -DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; - const fav1 = "!fav1:server"; const fav2 = "!fav2:server"; const fav3 = "!fav3:server"; @@ -68,6 +64,28 @@ const space1 = "!space1:server"; const space2 = "!space2:server"; const space3 = "!space3:server"; +const getUserIdForRoomId = jest.fn(roomId => { + return { + [dm1]: dm1Partner.userId, + [dm2]: dm2Partner.userId, + [dm3]: dm3Partner.userId, + }[roomId]; +}); +const getDMRoomsForUserId = jest.fn(userId => { + switch (userId) { + case dm1Partner.userId: + return [dm1]; + case dm2Partner.userId: + return [dm2]; + case dm3Partner.userId: + return [dm3]; + default: + return []; + } +}); +// @ts-ignore +DMRoomMap.sharedInstance = { getUserIdForRoomId, getDMRoomsForUserId }; + describe("SpaceStore", () => { stubClient(); const store = SpaceStore.instance; @@ -306,26 +324,6 @@ describe("SpaceStore", () => { client.getRoom(roomId).getMyMembership.mockReturnValue("invite"); }); - getUserIdForRoomId.mockImplementation(roomId => { - return { - [dm1]: dm1Partner.userId, - [dm2]: dm2Partner.userId, - [dm3]: dm3Partner.userId, - }[roomId]; - }); - getDMRoomsForUserId.mockImplementation(userId => { - switch (userId) { - case dm1Partner.userId: - return [dm1]; - case dm2Partner.userId: - return [dm2]; - case dm3Partner.userId: - return [dm3]; - default: - return []; - } - }); - // have dmPartner1 be in space1 with you const mySpace1Member = new RoomMember(space1, testUserId); mySpace1Member.membership = "join"; @@ -890,23 +888,102 @@ describe("SpaceStore", () => { }); }); - describe("dynamic text fixture", () => { + it("integrated test", async () => { + // init the store + await run(); + // receive invite to space - // expect it to appear in SpaceStore.instance.invitedSpaces + const rootSpace = mkSpace(space1, [room1, room2, space2]); + rootSpace.getMyMembership.mockReturnValue("invite"); + client.emit("Room", rootSpace); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([rootSpace]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([]); // accept invite to space - // expect it to disappear from SpaceStore.instance.invitedSpaces and move to SpaceStore.instance.spacePanelSpaces + rootSpace.getMyMembership.mockReturnValue("join"); + client.emit("Room.myMembership", rootSpace, "join", "invite"); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); // join room in space - // expect it to show up in the Room List only for that Space + expect(SpaceStore.instance.isRoomInSpace(space1, room1)).toBeFalsy(); + const rootSpaceRoom1 = mkRoom(room1); + rootSpaceRoom1.getMyMembership.mockReturnValue("join"); + client.emit("Room", rootSpaceRoom1); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); + expect(SpaceStore.instance.isRoomInSpace(space1, room1)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Home, room1)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Favourites, room1)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.People, room1)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Orphans, room1)).toBeFalsy(); // receive room invite - // expect it to show up in the Space and Home + expect(SpaceStore.instance.isRoomInSpace(space1, room2)).toBeFalsy(); + const rootSpaceRoom2 = mkRoom(room2); + rootSpaceRoom2.getMyMembership.mockReturnValue("invite"); + client.emit("Room", rootSpaceRoom2); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); + expect(SpaceStore.instance.isRoomInSpace(space1, room2)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Home, room2)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Favourites, room2)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.People, room2)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Orphans, room2)).toBeFalsy(); // start DM in space - // expect it to show up in the Space and Home and People + const myRootSpaceMember = new RoomMember(space1, testUserId); + myRootSpaceMember.membership = "join"; + const rootSpaceFriend = new RoomMember(space1, dm1Partner.userId); + rootSpace.getMembers.mockReturnValue([ + myRootSpaceMember, + rootSpaceFriend, + ]); + rootSpace.getMember.mockImplementation(userId => { + switch (userId) { + case testUserId: + return myRootSpaceMember; + case dm1Partner.userId: + return rootSpaceFriend; + } + }); + expect(SpaceStore.instance.getSpaceFilteredUserIds(space1).has(dm1Partner.userId)).toBeFalsy(); + client.emit("RoomState.members", mkEvent({ + event: true, + type: EventType.RoomMember, + content: { + membership: "join", + }, + skey: dm1Partner.userId, + user: dm1Partner.userId, + room: space1, + })); + jest.runAllTimers(); + expect(SpaceStore.instance.getSpaceFilteredUserIds(space1).has(dm1Partner.userId)).toBeTruthy(); + const dm1Room = mkRoom(dm1); + dm1Room.getMyMembership.mockReturnValue("join"); + client.emit("Room", dm1Room); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); + expect(SpaceStore.instance.isRoomInSpace(space1, dm1)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Home, dm1)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Favourites, dm1)).toBeFalsy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.People, dm1)).toBeTruthy(); + expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Orphans, dm1)).toBeFalsy(); // join subspace - // expect an emit on the parent space + const subspace = mkSpace(space1); + subspace.getMyMembership.mockReturnValue("join"); + const prom = emitPromise(SpaceStore.instance, space1); + client.emit("Room", subspace); + jest.runAllTimers(); + expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); + expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); + await prom; }); }); diff --git a/test/test-utils.js b/test/test-utils.js index 9efa8b6098d..cab783c342e 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -6,6 +6,7 @@ import { ValidatedServerConfig } from "../src/utils/AutoDiscoveryUtils"; import ShallowRenderer from 'react-test-renderer/shallow'; import MatrixClientContext from "../src/contexts/MatrixClientContext"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; +import EventEmitter from "events"; export function getRenderer() { // Old: ReactTestUtils.createRenderer(); @@ -42,6 +43,8 @@ export function stubClient() { * @returns {object} MatrixClient stub */ export function createTestClient() { + const eventEmitter = new EventEmitter(); + return { getHomeserverUrl: jest.fn(), getIdentityServerUrl: jest.fn(), @@ -56,8 +59,9 @@ export function createTestClient() { getVisibleRooms: jest.fn().mockReturnValue([]), getGroups: jest.fn().mockReturnValue([]), loginFlows: jest.fn(), - on: jest.fn(), - removeListener: jest.fn(), + on: eventEmitter.on.bind(eventEmitter), + emit: eventEmitter.emit.bind(eventEmitter), + removeListener: eventEmitter.removeListener.bind(eventEmitter), isRoomEncrypted: jest.fn().mockReturnValue(false), peekInRoom: jest.fn().mockResolvedValue(mkStubRoom()), From 3a0805e85d41f1955a82262543f3e89653d16690 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Dec 2021 14:19:39 +0000 Subject: [PATCH 21/26] Fix tests --- src/stores/spaces/SpaceStore.ts | 8 ++++- test/stores/SpaceStore-test.ts | 58 ++++++++++++++------------------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 8dcb3abc6a4..d746976c412 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -754,7 +754,13 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.switchSpaceIfNeeded(); } - this.updateNotificationStates(Array.from(changeSet)); + const notificationStatesToUpdate = [...changeSet]; + if (this.enabledMetaSpaces.includes(MetaSpace.People) && + userDiff.added.length + userDiff.removed.length + usersChanged.length > 0 + ) { + notificationStatesToUpdate.push(MetaSpace.People); + } + this.updateNotificationStates(notificationStatesToUpdate); }; private switchSpaceIfNeeded = throttle(() => { diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 172da42f7b8..52aff5b3815 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; @@ -35,7 +34,6 @@ import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import defaultDispatcher from "../../src/dispatcher/dispatcher"; import SettingsStore from "../../src/settings/SettingsStore"; import { SettingLevel } from "../../src/settings/SettingLevel"; -import { emitPromise } from "../utils/test-utils"; jest.useFakeTimers(); @@ -466,23 +464,24 @@ describe("SpaceStore", () => { expect(store.isRoomInSpace(space3, dm3)).toBeFalsy(); }); - it("dms are only added to Notification States for only the Home Space", () => { - // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better - // [dm1, dm2, dm3].forEach(d => { - // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(d)).toBeTruthy(); - // }); - [space1, space2, space3].forEach(s => { + it("dms are only added to Notification States for only the People Space", async () => { + [dm1, dm2, dm3].forEach(d => { + expect(store.getNotificationState(MetaSpace.People) + .rooms.map(r => r.roomId).includes(d)).toBeTruthy(); + }); + [space1, space2, space3, MetaSpace.Home, MetaSpace.Orphans, MetaSpace.Favourites].forEach(s => { [dm1, dm2, dm3].forEach(d => { expect(store.getNotificationState(s).rooms.map(r => r.roomId).includes(d)).toBeFalsy(); }); }); }); - it("orphan rooms are added to Notification States for only the Home Space", () => { - // XXX: All rooms space is forcibly enabled, as part of a future PR test Home space better - // [orphan1, orphan2].forEach(d => { - // expect(store.getNotificationState(HOME_SPACE).rooms.map(r => r.roomId).includes(d)).toBeTruthy(); - // }); + it("orphan rooms are added to Notification States for only the Home Space", async () => { + await setShowAllRooms(false); + [orphan1, orphan2].forEach(d => { + expect(store.getNotificationState(MetaSpace.Home) + .rooms.map(r => r.roomId).includes(d)).toBeTruthy(); + }); [space1, space2, space3].forEach(s => { [orphan1, orphan2].forEach(d => { expect(store.getNotificationState(s).rooms.map(r => r.roomId).includes(d)).toBeFalsy(); @@ -525,23 +524,12 @@ describe("SpaceStore", () => { }); describe("hierarchy resolution update tests", () => { - let emitter: EventEmitter; - beforeEach(async () => { - emitter = new EventEmitter(); - client.on.mockImplementation(emitter.on.bind(emitter)); - client.removeListener.mockImplementation(emitter.removeListener.bind(emitter)); - }); - afterEach(() => { - client.on.mockReset(); - client.removeListener.mockReset(); - }); - it("updates state when spaces are joined", async () => { await run(); expect(store.spacePanelSpaces).toStrictEqual([]); const space = mkSpace(space1); const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES); - emitter.emit("Room", space); + client.emit("Room", space); await prom; expect(store.spacePanelSpaces).toStrictEqual([space]); expect(store.invitedSpaces).toStrictEqual([]); @@ -554,7 +542,7 @@ describe("SpaceStore", () => { expect(store.spacePanelSpaces).toStrictEqual([space]); space.getMyMembership.mockReturnValue("leave"); const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES); - emitter.emit("Room.myMembership", space, "leave", "join"); + client.emit("Room.myMembership", space, "leave", "join"); await prom; expect(store.spacePanelSpaces).toStrictEqual([]); }); @@ -566,7 +554,7 @@ describe("SpaceStore", () => { const space = mkSpace(space1); space.getMyMembership.mockReturnValue("invite"); const prom = testUtils.emitPromise(store, UPDATE_INVITED_SPACES); - emitter.emit("Room", space); + client.emit("Room", space); await prom; expect(store.spacePanelSpaces).toStrictEqual([]); expect(store.invitedSpaces).toStrictEqual([space]); @@ -581,7 +569,7 @@ describe("SpaceStore", () => { expect(store.invitedSpaces).toStrictEqual([space]); space.getMyMembership.mockReturnValue("join"); const prom = testUtils.emitPromise(store, UPDATE_TOP_LEVEL_SPACES); - emitter.emit("Room.myMembership", space, "join", "invite"); + client.emit("Room.myMembership", space, "join", "invite"); await prom; expect(store.spacePanelSpaces).toStrictEqual([space]); expect(store.invitedSpaces).toStrictEqual([]); @@ -596,7 +584,7 @@ describe("SpaceStore", () => { expect(store.invitedSpaces).toStrictEqual([space]); space.getMyMembership.mockReturnValue("leave"); const prom = testUtils.emitPromise(store, UPDATE_INVITED_SPACES); - emitter.emit("Room.myMembership", space, "leave", "invite"); + client.emit("Room.myMembership", space, "leave", "invite"); await prom; expect(store.spacePanelSpaces).toStrictEqual([]); expect(store.invitedSpaces).toStrictEqual([]); @@ -616,7 +604,7 @@ describe("SpaceStore", () => { const invite = mkRoom(invite1); invite.getMyMembership.mockReturnValue("invite"); const prom = testUtils.emitPromise(store, space1); - emitter.emit("Room", space); + client.emit("Room", space); await prom; expect(store.spacePanelSpaces).toStrictEqual([space]); @@ -888,9 +876,10 @@ describe("SpaceStore", () => { }); }); - it("integrated test", async () => { + it("test user flow", async () => { // init the store await run(); + await setShowAllRooms(false); // receive invite to space const rootSpace = mkSpace(space1, [room1, room2, space2]); @@ -939,6 +928,7 @@ describe("SpaceStore", () => { const myRootSpaceMember = new RoomMember(space1, testUserId); myRootSpaceMember.membership = "join"; const rootSpaceFriend = new RoomMember(space1, dm1Partner.userId); + rootSpaceFriend.membership = "join"; rootSpace.getMembers.mockReturnValue([ myRootSpaceMember, rootSpaceFriend, @@ -977,13 +967,13 @@ describe("SpaceStore", () => { expect(SpaceStore.instance.isRoomInSpace(MetaSpace.Orphans, dm1)).toBeFalsy(); // join subspace - const subspace = mkSpace(space1); + const subspace = mkSpace(space2); subspace.getMyMembership.mockReturnValue("join"); - const prom = emitPromise(SpaceStore.instance, space1); + const prom = testUtils.emitPromise(SpaceStore.instance, space1); client.emit("Room", subspace); jest.runAllTimers(); expect(SpaceStore.instance.invitedSpaces).toStrictEqual([]); - expect(SpaceStore.instance.spacePanelSpaces).toStrictEqual([rootSpace]); + expect(SpaceStore.instance.spacePanelSpaces.map(r => r.roomId)).toStrictEqual([rootSpace.roomId]); await prom; }); }); From 44c7a8438c73769211219d7802e4d5e17f88067b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Dec 2021 12:47:48 +0000 Subject: [PATCH 22/26] Tweak copy --- src/components/views/dialogs/SpacePreferencesDialog.tsx | 6 ++++-- src/i18n/strings/en_EN.json | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/views/dialogs/SpacePreferencesDialog.tsx b/src/components/views/dialogs/SpacePreferencesDialog.tsx index 5560a4890fa..dc047245ac4 100644 --- a/src/components/views/dialogs/SpacePreferencesDialog.tsx +++ b/src/components/views/dialogs/SpacePreferencesDialog.tsx @@ -58,8 +58,10 @@ const SpacePreferencesAppearanceTab = ({ space }: Pick) => { { _t("People") }

- { _t("This groups your ongoing conversations with members of this space. " + - "Turning off will hide it from your view.") } + { _t("This groups your chats with members of this space. " + + "Turning this off will hide those chats from your view of %(spaceName)s.", { + spaceName: space.name, + }) }

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 143f04c0c7c..af6b3e9d60e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2706,7 +2706,7 @@ "Link to room": "Link to room", "Command Help": "Command Help", "Sections to show": "Sections to show", - "This groups your ongoing conversations with members of this space. Turning off will hide it from your view.": "This groups your ongoing conversations with members of this space. Turning off will hide it from your view.", + "This groups your chats with members of this space. Turning this off will hide those chats from your view of %(spaceName)s.": "This groups your chats with members of this space. Turning this off will hide those chats from your view of %(spaceName)s.", "Space settings": "Space settings", "Settings - %(spaceName)s": "Settings - %(spaceName)s", "Spaces you're in": "Spaces you're in", From 04eceec2775156e8f01994c5cdae5b145645edab Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 14 Dec 2021 09:56:42 +0000 Subject: [PATCH 23/26] consolidate types --- src/utils/arrays.ts | 2 ++ src/utils/sets.ts | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index 3f9dcbc34bc..a528b5fc1ae 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -184,6 +184,8 @@ export function arrayHasDiff(a: any[], b: any[]): boolean { } } +export type Diff = { added: T[], removed: T[] }; + /** * Performs a diff on two arrays. The result is what is different with the * first array (`added` in the returned object means objects in B that aren't diff --git a/src/utils/sets.ts b/src/utils/sets.ts index 7dd4dc271a9..85dda0dfec0 100644 --- a/src/utils/sets.ts +++ b/src/utils/sets.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { arrayDiff } from "./arrays"; +import { arrayDiff, Diff } from "./arrays"; /** * Determines if two sets are different through a shallow comparison. @@ -35,8 +35,6 @@ export function setHasDiff(a: Set, b: Set): boolean { } } -type Diff = { added: T[], removed: T[] }; - /** * Determines the values added and removed between two sets. * @param a The first set. Must be defined. From 12910bced0eefe902ad0c4f59c0c720b8f9220a8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 14 Dec 2021 10:03:43 +0000 Subject: [PATCH 24/26] Extract isMetaSpace to utility function --- src/components/views/rooms/RoomList.tsx | 9 +++++---- src/stores/room-list/SpaceWatcher.ts | 4 ++-- src/stores/spaces/SpaceStore.ts | 22 +++++++++++++--------- src/stores/spaces/index.ts | 7 +++++++ 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/components/views/rooms/RoomList.tsx b/src/components/views/rooms/RoomList.tsx index 220b29527ab..c36df0546d2 100644 --- a/src/components/views/rooms/RoomList.tsx +++ b/src/components/views/rooms/RoomList.tsx @@ -52,6 +52,7 @@ import { SpaceKey, UPDATE_SUGGESTED_ROOMS, UPDATE_SELECTED_SPACE, + isMetaSpace, } from "../../../stores/spaces"; import { shouldShowSpaceInvite, showAddExistingRooms, showCreateNewRoom, showSpaceInvite } from "../../../utils/space"; import { replaceableComponent } from "../../../utils/replaceableComponent"; @@ -494,10 +495,10 @@ export default class RoomList extends React.PureComponent { }; private onExplore = () => { - if (this.props.activeSpace[0] === "!") { + if (!isMetaSpace(this.props.activeSpace)) { defaultDispatcher.dispatch({ action: "view_room", - room_id: SpaceStore.instance.activeSpace, + room_id: this.props.activeSpace, }); } else { const initialText = RoomListStore.instance.getFirstNameFilterCondition()?.search; @@ -614,7 +615,7 @@ export default class RoomList extends React.PureComponent { (this.props.activeSpace === MetaSpace.People && orderedTagId !== DefaultTagID.DM) || (this.props.activeSpace === MetaSpace.Orphans && orderedTagId === DefaultTagID.DM) || ( - this.props.activeSpace[0] === "!" && + !isMetaSpace(this.props.activeSpace) && orderedTagId === DefaultTagID.DM && !SettingsStore.getValue("Spaces.showPeopleInSpace", this.props.activeSpace) ) @@ -674,7 +675,7 @@ export default class RoomList extends React.PureComponent { kind="link" onClick={this.onExplore} > - { this.props.activeSpace[0] === "!" ? _t("Explore rooms") : _t("Explore all public rooms") } + { !isMetaSpace(this.props.activeSpace) ? _t("Explore rooms") : _t("Explore all public rooms") } ; } diff --git a/src/stores/room-list/SpaceWatcher.ts b/src/stores/room-list/SpaceWatcher.ts index e7d6e78206a..d9aa032c4b8 100644 --- a/src/stores/room-list/SpaceWatcher.ts +++ b/src/stores/room-list/SpaceWatcher.ts @@ -17,7 +17,7 @@ limitations under the License. import { RoomListStoreClass } from "./RoomListStore"; import { SpaceFilterCondition } from "./filters/SpaceFilterCondition"; import SpaceStore from "../spaces/SpaceStore"; -import { MetaSpace, SpaceKey, UPDATE_HOME_BEHAVIOUR, UPDATE_SELECTED_SPACE } from "../spaces"; +import { isMetaSpace, MetaSpace, SpaceKey, UPDATE_HOME_BEHAVIOUR, UPDATE_SELECTED_SPACE } from "../spaces"; /** * Watches for changes in spaces to manage the filter on the provided RoomListStore @@ -66,7 +66,7 @@ export class SpaceWatcher { }; private updateFilter = () => { - if (this.activeSpace[0] === "!") { + if (!isMetaSpace(this.activeSpace)) { SpaceStore.instance.traverseSpace(this.activeSpace, roomId => { this.store.matrixClient?.getRoom(roomId)?.loadMembersIfNeeded(); }); diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index d746976c412..97c56c02972 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -41,6 +41,7 @@ import { reorderLexicographically } from "../../utils/stringOrderField"; import { TAG_ORDER } from "../../components/views/rooms/RoomList"; import { SettingUpdatedPayload } from "../../dispatcher/payloads/SettingUpdatedPayload"; import { + isMetaSpace, ISuggestedRoom, MetaSpace, SpaceKey, @@ -135,7 +136,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public get activeSpaceRoom(): Room | null { - if (this._activeSpace[0] !== "!") return null; + if (isMetaSpace(this._activeSpace)) return null; return this.matrixClient?.getRoom(this._activeSpace); } @@ -148,7 +149,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public setActiveRoomInSpace(space: SpaceKey): void { - if (space[0] === "!" && !this.matrixClient?.getRoom(space)?.isSpaceRoom()) return; + if (!isMetaSpace(space) && !this.matrixClient?.getRoom(space)?.isSpaceRoom()) return; if (space !== this.activeSpace) this.setActiveSpace(space); if (space) { @@ -196,7 +197,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (!space || !this.matrixClient || space === this.activeSpace) return; let cliSpace: Room; - if (space[0] === "!") { + if (!isMetaSpace(space)) { cliSpace = this.matrixClient.getRoom(space); if (!cliSpace?.isSpaceRoom()) return; } else if (!this.enabledMetaSpaces.includes(space as MetaSpace)) { @@ -370,7 +371,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return true; } - if (space[0] === "!" && + if (!isMetaSpace(space) && this.spaceFilteredUsers.get(space)?.has(dmPartner) && SettingsStore.getValue("Spaces.showPeopleInSpace", space) ) { @@ -391,7 +392,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if (space === MetaSpace.Home && this.allRoomsInHome) { return undefined; } - if (space[0] !== "!") return undefined; + if (isMetaSpace(space)) return undefined; return this.spaceFilteredUsers.get(space) || new Set(); }; @@ -535,7 +536,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.spaceFilteredRooms.set(MetaSpace.Orphans, new Set(orphans.map(r => r.roomId))); } - if (this.activeSpace[0] !== "!") { + if (isMetaSpace(this.activeSpace)) { this.switchSpaceIfNeeded(); } }; @@ -1044,7 +1045,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // restore selected state from last session if any and still valid const lastSpaceId = window.localStorage.getItem(ACTIVE_SPACE_LS_KEY); - if (lastSpaceId?.[0] === "!" ? this.matrixClient.getRoom(lastSpaceId) : enabledMetaSpaces[lastSpaceId]) { + const valid = (lastSpaceId && !isMetaSpace(lastSpaceId)) + ? this.matrixClient.getRoom(lastSpaceId) + : enabledMetaSpaces[lastSpaceId]; + if (valid) { // don't context switch here as it may break permalinks this.setActiveSpace(lastSpaceId, false); } else { @@ -1096,7 +1100,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { break; case "after_leave_room": - if (this._activeSpace[0] === "!" && payload.room_id === this._activeSpace) { + if (!isMetaSpace(this._activeSpace) && payload.room_id === this._activeSpace) { // User has left the current space, go to first space this.goToFirstSpace(true); } @@ -1142,7 +1146,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); // if a metaspace currently being viewed was removed, go to another one - if (this.activeSpace[0] !== "!" && !newValue[this.activeSpace]) { + if (isMetaSpace(this.activeSpace) && !newValue[this.activeSpace]) { this.switchSpaceIfNeeded(); } this.rebuildMetaSpaces(); diff --git a/src/stores/spaces/index.ts b/src/stores/spaces/index.ts index 7272cd6095b..f4bba0621b6 100644 --- a/src/stores/spaces/index.ts +++ b/src/stores/spaces/index.ts @@ -53,3 +53,10 @@ export type SpaceKey = MetaSpace | Room["roomId"]; export interface ISuggestedRoom extends IHierarchyRoom { viaServers: string[]; } + +export function isMetaSpace(spaceKey: SpaceKey): boolean { + return spaceKey === MetaSpace.Home || + spaceKey === MetaSpace.Favourites || + spaceKey === MetaSpace.People || + spaceKey === MetaSpace.Orphans; +} From 46654a09ce4f9cd98321e401a3791dfeff55a4d5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 14 Dec 2021 10:04:53 +0000 Subject: [PATCH 25/26] tidy --- src/stores/spaces/SpaceStore.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 97c56c02972..068d51bacef 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -52,6 +52,7 @@ import { UPDATE_TOP_LEVEL_SPACES, } from "."; import { getCachedRoomIDForAlias } from "../../RoomAliasCache"; +import { EffectiveMembership, getEffectiveMembership } from "../../utils/membership"; interface IState {} @@ -450,16 +451,16 @@ export class SpaceStoreClass extends AsyncStoreWithClient { private rebuildSpaceHierarchy = () => { const visibleSpaces = this.matrixClient.getVisibleRooms().filter(r => r.isSpaceRoom()); - const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce((arr, s) => { - switch (s.getMyMembership()) { - case "join": - arr[0].push(s); + const [joinedSpaces, invitedSpaces] = visibleSpaces.reduce(([joined, invited], s) => { + switch (getEffectiveMembership(s.getMyMembership())) { + case EffectiveMembership.Join: + joined.push(s); break; - case "invite": - arr[1].push(s); + case EffectiveMembership.Invite: + invited.push(s); break; } - return arr; + return [joined, invited]; }, [[], []] as [Room[], Room[]]); const rootSpaces = this.findRootSpaces(joinedSpaces); From 2b7cdeafab164cdaf1b07a67c170fa5c0357a3a9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 17 Dec 2021 09:19:38 +0000 Subject: [PATCH 26/26] tidy up --- src/utils/arrays.ts | 2 +- src/utils/sets.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index d8976df995b..a0fddde45cd 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -194,7 +194,7 @@ export type Diff = { added: T[], removed: T[] }; * @param b The second array. Must be defined. * @returns The diff between the arrays. */ -export function arrayDiff(a: T[], b: T[]): { added: T[], removed: T[] } { +export function arrayDiff(a: T[], b: T[]): Diff { return { added: b.filter(i => !a.includes(i)), removed: a.filter(i => !b.includes(i)), diff --git a/src/utils/sets.ts b/src/utils/sets.ts index 85dda0dfec0..da856af2b5f 100644 --- a/src/utils/sets.ts +++ b/src/utils/sets.ts @@ -42,6 +42,5 @@ export function setHasDiff(a: Set, b: Set): boolean { * @returns The difference between the values in each set. */ export function setDiff(a: Set, b: Set): Diff { - const keyDiff = arrayDiff(Array.from(a), Array.from(b)); - return { added: keyDiff.added, removed: keyDiff.removed }; + return arrayDiff(Array.from(a), Array.from(b)); }