refactor(subpages): address PR #155 review

- Extract buildSubtree/mapSharedNodes/countNodes/SubpageNode into
  subpages-view.utils.ts with a unit test (subpages-view.utils.test.ts)
  covering nesting, position order, missing/unreachable parent, self-parent
  guard, empty input, countNodes and mapSharedNodes remap.
- Replace the manual useState + editor.on("transaction") subscription in
  subpages-menu.tsx with useEditorState (the idiom the sibling bubble menus
  use), so the mode icon/tooltip track the live recursive attribute without
  re-rendering on every keystroke.
- i18n: add the 6 menu/tree strings and a pluralized
  "Showing {{count}} subpages" key to en-US and ru-RU.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/client/src/features/editor/components/subpages/subpages-menu.tsx

@@ -1,7 +1,7 @@
 import { BubbleMenu as BaseBubbleMenu } from "@tiptap/react/menus";
-import { posToDOMRect, findParentNode } from "@tiptap/react";
+import { posToDOMRect, findParentNode, useEditorState } from "@tiptap/react";
 import { Node as PMNode } from "@tiptap/pm/model";
-import React, { useCallback, useEffect, useState } from "react";
+import React, { useCallback } from "react";
 import { ActionIcon, Group, Tooltip } from "@mantine/core";
 import { IconTrash, IconList, IconSitemap } from "@tabler/icons-react";
 import { useTranslation } from "react-i18next";
@@ -64,24 +64,14 @@ export const SubpagesMenu = React.memo(
         .run();
     }, [editor]);
 
-    // The component is memoized on `editor` (a stable reference), so reading the
-    // attribute at render time would leave the mode icon/tooltip stale right
-    // after toggling. Track it in state synced on editor transactions; setState
-    // bails when the value is unchanged, so this does not re-render per keystroke.
-    const [isRecursive, setIsRecursive] = useState<boolean>(
-      () => editor.getAttributes("subpages")?.recursive ?? false,
-    );
-    useEffect(() => {
-      const sync = () => {
-        const value = editor.getAttributes("subpages")?.recursive ?? false;
-        setIsRecursive((prev) => (prev === value ? prev : value));
-      };
-      sync();
-      editor.on("transaction", sync);
-      return () => {
-        editor.off("transaction", sync);
-      };
-    }, [editor]);
+    // Subscribe to the live `recursive` attribute the standard way (as the
+    // sibling bubble menus do): useEditorState re-renders only when the selected
+    // value actually changes, so the mode icon/tooltip stay current after a
+    // toggle without re-rendering on every keystroke.
+    const isRecursive = useEditorState({
+      editor,
+      selector: (ctx) => ctx.editor?.getAttributes("subpages")?.recursive ?? false,
+    });
 
     return (
       <BaseBubbleMenu

apps/client/src/features/editor/components/subpages/subpages-view.tsx

@@ -19,81 +19,17 @@ import {
   useSharedPageSubpages,
   useSharedPageSubtree,
 } from "@/features/share/hooks/use-shared-page-subpages";
-import { IPage } from "@/features/page/types/page.types";
-import { SharedPageTreeNode } from "@/features/share/utils";
+import {
+  SubpageNode,
+  buildSubtree,
+  mapSharedNodes,
+  countNodes,
+} from "./subpages-view.utils";
 
 // Threshold above which the recursive tree shows a small count note. We never
 // cap the data — this is only an informational hint for very large trees.
 const LARGE_TREE_THRESHOLD = 300;
 
-// Normalized node shared by the flat and recursive renderers so the same
-// link/icon markup works for both API pages and shared-tree nodes.
-interface SubpageNode {
-  id: string;
-  slugId: string;
-  title: string;
-  icon?: string;
-  children: SubpageNode[];
-}
-
-// Subpage node carrying `position` so each level can be sorted in place.
-type SubpageNodeWithPos = SubpageNode & {
-  position: string;
-  children: SubpageNodeWithPos[];
-};
-
-// Build a nested subtree from the flat IPage[] returned by /pages/tree.
-function buildSubtree(pages: IPage[], rootId: string): SubpageNode[] {
-  const byId = new Map<string, SubpageNodeWithPos>(
-    pages.map((p) => [
-      p.id,
-      {
-        id: p.id,
-        slugId: p.slugId,
-        title: p.title,
-        icon: p.icon,
-        position: p.position,
-        children: [],
-      },
-    ]),
-  );
-
-  for (const p of pages) {
-    const node = byId.get(p.id);
-    const parent = p.parentPageId ? byId.get(p.parentPageId) : undefined;
-    // Guard against cycles / self-parenting: never attach a node to itself or
-    // to the root, and only attach when the parent is actually present.
-    if (node && parent && p.id !== rootId) {
-      parent.children.push(node);
-    }
-  }
-
-  const sortRecursive = (nodes: SubpageNodeWithPos[]) => {
-    const sorted = sortPositionKeys(nodes) as SubpageNodeWithPos[];
-    sorted.forEach((n) => sortRecursive(n.children));
-    return sorted;
-  };
-
-  const root = byId.get(rootId);
-  return root ? sortRecursive(root.children) : [];
-}
-
-// Map shared-tree nodes (already nested) onto the normalized SubpageNode shape.
-function mapSharedNodes(nodes: SharedPageTreeNode[]): SubpageNode[] {
-  return nodes.map((node) => ({
-    id: node.value,
-    slugId: node.slugId,
-    title: node.name,
-    icon: node.icon,
-    children: node.children ? mapSharedNodes(node.children) : [],
-  }));
-}
-
-// Count every descendant in a normalized subtree.
-function countNodes(nodes: SubpageNode[]): number {
-  return nodes.reduce((acc, n) => acc + 1 + countNodes(n.children), 0);
-}
-
 interface TreeNodeProps {
   node: SubpageNode;
   depth: number;
@@ -194,7 +130,7 @@ interface SubpagesVariantProps {
   currentPageId: string;
   shareId?: string;
   spaceSlug?: string;
-  t: (key: string) => string;
+  t: (key: string, options?: Record<string, unknown>) => string;
 }
 
 function FlatSubpages({
@@ -368,7 +304,7 @@ function RecursiveSubpages({
         </Stack>
         {total > LARGE_TREE_THRESHOLD && (
           <Text c="dimmed" size="xs" pt="xs">
-            {t("Showing")} {total} {t("subpages")}
+            {t("Showing {{count}} subpages", { count: total })}
           </Text>
         )}
       </div>

apps/client/src/features/editor/components/subpages/subpages-view.utils.test.ts

@@ -0,0 +1,114 @@
+import { describe, it, expect } from "vitest";
+import {
+  buildSubtree,
+  countNodes,
+  mapSharedNodes,
+  SubpageNode,
+} from "./subpages-view.utils";
+import { IPage } from "@/features/page/types/page.types";
+
+// Minimal IPage fixture — buildSubtree only reads id/slugId/title/icon/position/
+// parentPageId. `position` keys are fractional-indexing strings (lexicographic).
+const page = (p: Partial<IPage> & { id: string }): IPage =>
+  ({
+    slugId: `slug-${p.id}`,
+    title: `Title ${p.id}`,
+    icon: undefined,
+    position: "a0",
+    parentPageId: null,
+    ...p,
+  }) as IPage;
+
+const ids = (nodes: SubpageNode[]): string[] => nodes.map((n) => n.id);
+
+describe("buildSubtree", () => {
+  it("nests children under the root and excludes the root itself", () => {
+    const pages = [
+      page({ id: "root" }),
+      page({ id: "a", parentPageId: "root", position: "a0" }),
+      page({ id: "b", parentPageId: "root", position: "a1" }),
+      page({ id: "a1", parentPageId: "a", position: "a0" }),
+    ];
+    const tree = buildSubtree(pages, "root");
+    // Root is not rendered; only its descendants.
+    expect(ids(tree)).toEqual(["a", "b"]);
+    expect(ids(tree[0].children)).toEqual(["a1"]);
+    expect(tree[1].children).toEqual([]);
+  });
+
+  it("sorts each level by position", () => {
+    const pages = [
+      page({ id: "root" }),
+      page({ id: "z", parentPageId: "root", position: "a2" }),
+      page({ id: "x", parentPageId: "root", position: "a0" }),
+      page({ id: "y", parentPageId: "root", position: "a1" }),
+    ];
+    expect(ids(buildSubtree(pages, "root"))).toEqual(["x", "y", "z"]);
+  });
+
+  it("returns [] when the root is absent from the page set", () => {
+    const pages = [page({ id: "a", parentPageId: "missing-root" })];
+    expect(buildSubtree(pages, "missing-root")).toEqual([]);
+  });
+
+  it("silently drops a node whose parent is absent (unreachable parent)", () => {
+    const pages = [
+      page({ id: "root" }),
+      page({ id: "ok", parentPageId: "root" }),
+      page({ id: "orphan", parentPageId: "ghost" }), // parent not in the set
+    ];
+    expect(ids(buildSubtree(pages, "root"))).toEqual(["ok"]);
+  });
+
+  it("guards against self-parenting / attaching the root", () => {
+    const pages = [
+      // A (defensive) self-parented root must not attach to itself.
+      page({ id: "root", parentPageId: "root" }),
+      page({ id: "a", parentPageId: "root" }),
+    ];
+    const tree = buildSubtree(pages, "root");
+    expect(ids(tree)).toEqual(["a"]);
+  });
+
+  it("returns [] for empty input", () => {
+    expect(buildSubtree([], "root")).toEqual([]);
+  });
+});
+
+describe("countNodes", () => {
+  it("counts every descendant across all levels", () => {
+    const tree: SubpageNode[] = [
+      {
+        id: "a",
+        slugId: "s",
+        title: "A",
+        children: [
+          { id: "a1", slugId: "s", title: "A1", children: [] },
+          { id: "a2", slugId: "s", title: "A2", children: [] },
+        ],
+      },
+      { id: "b", slugId: "s", title: "B", children: [] },
+    ];
+    expect(countNodes(tree)).toBe(4);
+    expect(countNodes([])).toBe(0);
+  });
+});
+
+describe("mapSharedNodes", () => {
+  it("remaps value->id / name->title and keeps nested children", () => {
+    const shared = [
+      {
+        value: "p1",
+        slugId: "s1",
+        name: "Parent",
+        icon: "📁",
+        children: [
+          { value: "c1", slugId: "sc1", name: "Child", children: [] },
+        ],
+      },
+    ] as any;
+    const mapped = mapSharedNodes(shared);
+    expect(mapped[0]).toMatchObject({ id: "p1", slugId: "s1", title: "Parent", icon: "📁" });
+    expect(mapped[0].children[0]).toMatchObject({ id: "c1", title: "Child" });
+  });
+});

apps/client/src/features/editor/components/subpages/subpages-view.utils.ts

@@ -0,0 +1,83 @@
+import { sortPositionKeys } from "@/features/page/tree/utils/utils";
+import { IPage } from "@/features/page/types/page.types";
+import { SharedPageTreeNode } from "@/features/share/utils";
+
+// Normalized node shared by the flat and recursive subpages renderers so the
+// same link/icon markup works for both API pages and shared-tree nodes.
+export interface SubpageNode {
+  id: string;
+  slugId: string;
+  title: string;
+  icon?: string;
+  children: SubpageNode[];
+}
+
+// Subpage node carrying `position` so each level can be sorted in place.
+export type SubpageNodeWithPos = SubpageNode & {
+  position: string;
+  children: SubpageNodeWithPos[];
+};
+
+/**
+ * Build a nested subtree (the current page's descendants) from the flat `IPage[]`
+ * the `/pages/tree` endpoint returns. Attaches each node to its parent by
+ * `parentPageId`, drops the root itself, and sorts every level by `position`.
+ *
+ * Guards only against SELF-PARENTING and attaching the root (`p.id !== rootId`) —
+ * NOT against multi-node `parentPageId` cycles. Those cannot occur here: the
+ * server rejects cyclic moves, and the recursive `getPageAndDescendants` CTE that
+ * produces this list would itself loop before reaching the client, so the flat
+ * input is acyclic by construction. A node whose `parentPageId` points outside
+ * the result set (an unreachable parent) is silently dropped — it is, by
+ * definition, not a descendant of the root being rendered.
+ */
+export function buildSubtree(pages: IPage[], rootId: string): SubpageNode[] {
+  const byId = new Map<string, SubpageNodeWithPos>(
+    pages.map((p) => [
+      p.id,
+      {
+        id: p.id,
+        slugId: p.slugId,
+        title: p.title,
+        icon: p.icon,
+        position: p.position,
+        children: [],
+      },
+    ]),
+  );
+
+  for (const p of pages) {
+    const node = byId.get(p.id);
+    const parent = p.parentPageId ? byId.get(p.parentPageId) : undefined;
+    if (node && parent && p.id !== rootId) {
+      parent.children.push(node);
+    }
+  }
+
+  const sortRecursive = (
+    nodes: SubpageNodeWithPos[],
+  ): SubpageNodeWithPos[] => {
+    const sorted = sortPositionKeys(nodes) as SubpageNodeWithPos[];
+    sorted.forEach((n) => sortRecursive(n.children));
+    return sorted;
+  };
+
+  const root = byId.get(rootId);
+  return root ? sortRecursive(root.children) : [];
+}
+
+// Map shared-tree nodes (already nested) onto the normalized SubpageNode shape.
+export function mapSharedNodes(nodes: SharedPageTreeNode[]): SubpageNode[] {
+  return nodes.map((node) => ({
+    id: node.value,
+    slugId: node.slugId,
+    title: node.name,
+    icon: node.icon,
+    children: node.children ? mapSharedNodes(node.children) : [],
+  }));
+}
+
+// Count every descendant in a normalized subtree.
+export function countNodes(nodes: SubpageNode[]): number {
+  return nodes.reduce((acc, n) => acc + 1 + countNodes(n.children), 0);
+}