From 8fbd3bd460ce8f15b70e6dd166192b001b67fc2e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Oct 2025 12:42:28 +0100 Subject: [PATCH 1/6] Fix sort order in space hierarchy To match spec and not add unexpected sorting by space vs room --- src/components/structures/SpaceHierarchy.tsx | 91 +++++++++++--------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index 0d41e56c925..0b22028bc1c 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -522,50 +522,55 @@ export const HierarchyLevel: React.FC = ({ const newParents = new Set(parents).add(root.room_id); return ( - {uniqBy(childRooms, "room_id").map((room) => ( - onViewRoomClick(room.room_id, room.room_type as RoomType)} - onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} - hasPermissions={hasPermissions} - onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} - /> - ))} - - {subspaces - .filter((room) => !newParents.has(room.room_id)) - .map((space) => ( - { - const room = hierarchy.roomMap.get(ev.state_key); - return room && roomSet.has(room) && !room.room_type; - }).length - } - suggested={hierarchy.isSuggested(root.room_id, space.room_id)} - selected={selectedMap?.get(root.room_id)?.has(space.room_id)} - onViewRoomClick={() => onViewRoomClick(space.room_id, RoomType.Space)} - onJoinRoomClick={() => onJoinRoomClick(space.room_id, newParents)} - hasPermissions={hasPermissions} - onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, space.room_id) : undefined} - > - { + const room = toLocalRoom(cli, hierarchy.roomMap.get(ev.state_key), hierarchy); + + if (room.room_type === RoomType.Space) { + if (newParents.has(room.room_id)) return null; // prevent cycles + return ( + { + const child = hierarchy.roomMap.get(ev.state_key); + return child && roomSet.has(child) && !child.room_type; + }).length + } + suggested={hierarchy.isSuggested(root.room_id, room.room_id)} + selected={selectedMap?.get(root.room_id)?.has(room.room_id)} + onViewRoomClick={() => onViewRoomClick(room.room_id, RoomType.Space)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} + hasPermissions={hasPermissions} + onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} + > + + + ); + } else { + return ( + onViewRoomClick(room.room_id, room.room_type as RoomType)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} + hasPermissions={hasPermissions} + onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} /> - - ))} + ); + } + }); ); }; From 6fe8dc4da6dfa98158beef43a07ad8f22df14e80 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Oct 2025 12:45:23 +0100 Subject: [PATCH 2/6] Update SpaceHierarchy.tsx --- src/components/structures/SpaceHierarchy.tsx | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index 0b22028bc1c..ffc84a7d099 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -508,17 +508,6 @@ export const HierarchyLevel: React.FC = ({ return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); }); - const [subspaces, childRooms] = sortedChildren.reduce( - (result, ev: HierarchyRelation) => { - const room = hierarchy.roomMap.get(ev.state_key); - if (room && roomSet.has(room)) { - result[room.room_type === RoomType.Space ? 0 : 1].push(toLocalRoom(cli, room, hierarchy)); - } - return result; - }, - [[] as HierarchyRoom[], [] as HierarchyRoom[]], - ); - const newParents = new Set(parents).add(root.room_id); return ( @@ -570,7 +559,7 @@ export const HierarchyLevel: React.FC = ({ /> ); } - }); + })} ); }; From 41dc60449bc0d8da28e29608b700125988f257b1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Oct 2025 15:07:27 +0100 Subject: [PATCH 3/6] Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/SpaceHierarchy.tsx | 47 +++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index ffc84a7d099..f460784dc67 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -32,7 +32,6 @@ import { RoomType, GuestAccess, HistoryVisibility, - type HierarchyRelation, type HierarchyRoom, JoinRule, } from "matrix-js-sdk/src/matrix"; @@ -71,6 +70,7 @@ import { getTopic } from "../../hooks/room/useTopic"; import { SdkContextClass } from "../../contexts/SDKContext"; import { getDisplayAliasForAliasSet } from "../../Rooms"; import SettingsStore from "../../settings/SettingsStore"; +import { filterBoolean } from "../../utils/arrays.ts"; interface IProps { space: Room; @@ -504,17 +504,35 @@ export const HierarchyLevel: React.FC = ({ const space = cli.getRoom(root.room_id); const hasPermissions = space?.currentState.maySendStateEvent(EventType.SpaceChild, cli.getSafeUserId()); - const sortedChildren = sortBy(root.children_state, (ev) => { - return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); - }); + const sortedChildren = filterBoolean( + sortBy(root.children_state, (ev) => { + return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); + }).map((ev) => { + const hierarchyRoom = hierarchy.roomMap.get(ev.state_key); + if (!hierarchyRoom || !roomSet.has(hierarchyRoom)) return null; + // Find the most up-to-date info for this room, if it has been upgraded and we know about it. + return toLocalRoom(cli, hierarchyRoom, hierarchy); + }), + ); const newParents = new Set(parents).add(root.room_id); return ( - {uniqBy(sortedChildren, "room_id").map((ev) => { - const room = toLocalRoom(cli, hierarchy.roomMap.get(ev.state_key), hierarchy); - - if (room.room_type === RoomType.Space) { + {uniqBy(sortedChildren, "room_id").map((room) => { + if (room.room_type !== RoomType.Space) { + return ( + onViewRoomClick(room.room_id, room.room_type as RoomType)} + onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} + hasPermissions={hasPermissions} + onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} + /> + ); + } else { if (newParents.has(room.room_id)) return null; // prevent cycles return ( = ({ /> ); - } else { - return ( - onViewRoomClick(room.room_id, room.room_type as RoomType)} - onJoinRoomClick={() => onJoinRoomClick(room.room_id, newParents)} - hasPermissions={hasPermissions} - onToggleClick={onToggleClick ? () => onToggleClick(root.room_id, room.room_id) : undefined} - /> - ); } })} From d96c5e625b4c8657a94917ee27f8d9ddfda81632 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Oct 2025 15:20:30 +0100 Subject: [PATCH 4/6] Update snapshot Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../SpaceHierarchy-test.tsx.snap | 188 +++++++++--------- 1 file changed, 94 insertions(+), 94 deletions(-) diff --git a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap index c75b79aee41..7594ee5902b 100644 --- a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap +++ b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +// Jest Snapshot v1, https://goo.gl/fbAQLP exports[`SpaceHierarchy renders 1`] = ` @@ -254,12 +254,13 @@ exports[`SpaceHierarchy renders 1`] = `
  • @@ -271,13 +272,13 @@ exports[`SpaceHierarchy renders 1`] = ` > - K + N
    renders 1`] = ` - Knock room + Nested space
    - 3 members + 1 member · 1 room
    - View + Join
    renders 1`] = `
    +
    +
  • @@ -385,13 +392,13 @@ exports[`SpaceHierarchy renders 1`] = ` - Nested space + Nested room
    - 1 member · 1 room + 3 members
    renders 1`] = ` > Join
    -
    -
    -
    - +
    - - + +
    +
    -
    -
    - + +
    -
    -
  • @@ -475,22 +481,22 @@ exports[`SpaceHierarchy renders 1`] = ` > - N + K
    - Nested room + Knock room
    renders 1`] = ` class="mx_SpaceHierarchy_actions" >
    - Join + View
    - -
    +
    - - + +
    -
    - - +
    +
    +
    • From aa4908d2d824b55e4d40260373a4e56e4a141e8c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Oct 2025 09:13:58 +0100 Subject: [PATCH 5/6] Add test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../structures/SpaceHierarchy-test.tsx | 36 +- .../SpaceHierarchy-test.tsx.snap | 363 ++++++++++++++++++ 2 files changed, 398 insertions(+), 1 deletion(-) diff --git a/test/unit-tests/components/structures/SpaceHierarchy-test.tsx b/test/unit-tests/components/structures/SpaceHierarchy-test.tsx index cea704b90b7..f0d6429789d 100644 --- a/test/unit-tests/components/structures/SpaceHierarchy-test.tsx +++ b/test/unit-tests/components/structures/SpaceHierarchy-test.tsx @@ -169,11 +169,14 @@ describe("SpaceHierarchy", () => { const room2 = mkStubRoom("room-id-3", "Room 2", client); const space1 = mkStubRoom("space-id-4", "Space 2", client); const room3 = mkStubRoom("room-id-5", "Room 3", client); + const space2 = mkStubRoom("space-id-6", "Space 3", client); mocked(client.getRooms).mockReturnValue([root]); mocked(client.getRoom).mockImplementation( (roomId) => client.getRooms().find((room) => room.roomId === roomId) ?? null, ); - [room1, room2, space1, room3].forEach((r) => mocked(r.getMyMembership).mockReturnValue(KnownMembership.Leave)); + [room1, room2, space1, room3, space2].forEach((r) => + mocked(r.getMyMembership).mockReturnValue(KnownMembership.Leave), + ); const hierarchyRoot: HierarchyRoom = { room_id: root.roomId, @@ -324,5 +327,36 @@ describe("SpaceHierarchy", () => { undefined, ); }); + + it("should not render cycles", async () => { + const hierarchySpace2: HierarchyRoom = { + room_id: space2.roomId, + name: "Space with cycle", + num_joined_members: 1, + room_type: "m.space", + children_state: [ + { + state_key: root.roomId, + content: { order: "1" }, + origin_server_ts: 111, + type: "m.space.child", + sender: "@other:server", + }, + ], + world_readable: true, + guest_can_join: true, + }; + + mocked(client.getRoomHierarchy).mockResolvedValue({ + rooms: [hierarchyRoot, hierarchyRoom1, hierarchyRoom2, hierarchySpace1, hierarchySpace2], + }); + + const { getAllByText, queryByText, asFragment } = render(getComponent()); + // Wait for spinners to go away + await waitForElementToBeRemoved(screen.getAllByRole("progressbar")); + expect(getAllByText("Nested space")).toHaveLength(1); + expect(queryByText("Space 1")).not.toBeInTheDocument(); + expect(asFragment()).toMatchSnapshot(); + }); }); }); diff --git a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap index 7594ee5902b..8b9d9212b42 100644 --- a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap +++ b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap @@ -564,3 +564,366 @@ exports[`SpaceHierarchy renders 1`] = `     `; + +exports[`SpaceHierarchy should not render cycles 1`] = ` + +
    + +
    +
    +
    +

    + Rooms and spaces +

    +
    +
    + Remove +
    +
    + Mark as not suggested +
    +
    +
    +
      +
    • +
      +
      +
      + + U + +
      +
      + + Unnamed Room + +
      +
      + 2 members +
      +
      +
      +
      + Join +
      +
      +
      +
      +
      + +
      + +
      +
      +
      +
      +
      + +
      +
      +
      • +
    • +
      +
      +
      + + U + +
      +
      + + Unnamed Room + +
      +
      + 3 members +
      +
      +
      +
      + Join +
      +
      +
      +
      +
      + +
      + +
      +
      +
      +
      +
      + +
      +
      +
      • +
    • +
      +
      +
      + + N + +
      +
      + + Nested space + +
      +
      + 1 member · 0 rooms +
      +
      +
      +
      + Join +
      +
      +
      +
      +
      + +
      + +
      +
      +
      +
      +
      + +
      +
      +
      +
      +
      • +
    + +`; From 817c4a5402045374618a1ecc8aaba0ecd5cb9af9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Oct 2025 09:22:10 +0100 Subject: [PATCH 6/6] Update snapshot Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../__snapshots__/SpaceHierarchy-test.tsx.snap | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap index 8b9d9212b42..26784377c36 100644 --- a/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap +++ b/test/unit-tests/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap @@ -623,7 +623,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` role="tree" >
  • @@ -653,7 +653,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="mx_SpaceHierarchy_roomTile_name" > Unnamed Room @@ -687,7 +687,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="_container_1hel1_10" > should not render cycles 1`] = `
  • @@ -751,7 +751,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="mx_SpaceHierarchy_roomTile_name" > Unnamed Room @@ -785,7 +785,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="_container_1hel1_10" > should not render cycles 1`] = `
  • @@ -850,7 +850,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="mx_SpaceHierarchy_roomTile_name" > Nested space @@ -884,7 +884,7 @@ exports[`SpaceHierarchy should not render cycles 1`] = ` class="_container_1hel1_10" >