Allow pinned hosts with name sorting (#1043)

src/ui/sidebar/HostsPanel.tsx

@@ -44,16 +44,11 @@ import {
 } from "@/main-axios";
 import type { SSHHostWithStatus } from "@/main-axios";
 import type { Host, HostFolder, TabType } from "@/types/ui-types";
-
+import {
-type SortKey =
+  resolveHostSortPreferences,
-  | "default"
+  sortHostTree,
-  | "name-asc"
+  type SortKey,
-  | "name-desc"
+} from "@/sidebar/host-sort";
-  | "ip-asc"
-  | "ip-desc"
-  | "status-online"
-  | "status-offline"
-  | "pinned";
 
 type FilterState = {
   status: ("online" | "offline" | "pinned")[];
@@ -127,43 +122,6 @@ function groupHosts(
   return { name: "root", children };
 }
 
-function sortHostTree(folder: HostFolder, key: SortKey): HostFolder {
-  if (key === "default") return folder;
-
-  const comparator = (a: Host | HostFolder, b: Host | HostFolder): number => {
-    const aIsFolder = isFolder(a);
-    const bIsFolder = isFolder(b);
-    if (aIsFolder && !bIsFolder) return -1;
-    if (!aIsFolder && bIsFolder) return 1;
-    if (aIsFolder && bIsFolder)
-      return (a as HostFolder).name.localeCompare((b as HostFolder).name);
-    const ha = a as Host,
-      hb = b as Host;
-    switch (key) {
-      case "name-asc":
-        return ha.name.localeCompare(hb.name);
-      case "name-desc":
-        return hb.name.localeCompare(ha.name);
-      case "ip-asc":
-        return ha.ip.localeCompare(hb.ip);
-      case "ip-desc":
-        return hb.ip.localeCompare(ha.ip);
-      case "status-online":
-        return (hb.online ? 1 : 0) - (ha.online ? 1 : 0);
-      case "status-offline":
-        return (ha.online ? 1 : 0) - (hb.online ? 1 : 0);
-      case "pinned":
-        return (hb.pin ? 1 : 0) - (ha.pin ? 1 : 0);
-    }
-    return 0;
-  };
-
-  const sortedChildren = [...folder.children]
-    .sort(comparator)
-    .map((child) => (isFolder(child) ? sortHostTree(child, key) : child));
-  return { ...folder, children: sortedChildren };
-}
-
 function hostPassesFilters(host: Host, filters: FilterState): boolean {
   if (filters.status.length > 0) {
     const ok =
@@ -255,9 +213,18 @@ export function HostsPanel({
   const [proxmoxDefaultUsername, setProxmoxDefaultUsername] = useState<
     string | undefined
   >(undefined);
-  const [sortKey, setSortKey] = useState<SortKey>(
+  const [sortKey, setSortKey] = useState<SortKey>(() => {
-    () => (localStorage.getItem("hostSortKey") as SortKey) ?? "default",
+    return resolveHostSortPreferences(
-  );
+      localStorage.getItem("hostSortKey"),
+      localStorage.getItem("hostPinnedFirst"),
+    ).sortKey;
+  });
+  const [pinnedFirst, setPinnedFirst] = useState(() => {
+    return resolveHostSortPreferences(
+      localStorage.getItem("hostSortKey"),
+      localStorage.getItem("hostPinnedFirst"),
+    ).pinnedFirst;
+  });
   const [groupKey, setGroupKey] = useState<GroupKey>(
     () => (localStorage.getItem("hostGroupKey") as GroupKey) ?? "folder",
   );
@@ -280,6 +247,11 @@ export function HostsPanel({
     localStorage.setItem("hostSortKey", key);
   }
 
+  function handlePinnedFirstChange(enabled: boolean) {
+    setPinnedFirst(enabled);
+    localStorage.setItem("hostPinnedFirst", String(enabled));
+  }
+
   function handleGroupChange(key: GroupKey) {
     setGroupKey(key);
     localStorage.setItem("hostGroupKey", key);
@@ -654,7 +626,7 @@ export function HostsPanel({
                 <Button
                   variant="ghost"
                   size="icon"
-                  className={`size-7 ${sortKey !== "default" ? "text-accent-brand" : "text-muted-foreground hover:text-foreground"}`}
+                  className={`size-7 ${sortKey !== "default" || pinnedFirst ? "text-accent-brand" : "text-muted-foreground hover:text-foreground"}`}
                   title={t("hosts.sortHosts")}
                 >
                   <ArrowUpDown className="size-3.5" />
@@ -708,28 +680,32 @@ export function HostsPanel({
                   </DropdownMenuItem>
                 ))}
                 <DropdownMenuSeparator />
-                {(["status-online", "status-offline", "pinned"] as const).map(
+                {(["status-online", "status-offline"] as const).map((key) => (
-                  (key) => (
+                  <DropdownMenuItem
-                    <DropdownMenuItem
+                    key={key}
-                      key={key}
+                    onClick={() => handleSortChange(key)}
-                      onClick={() => handleSortChange(key)}
+                    className="flex items-center gap-1.5"
-                      className="flex items-center gap-1.5"
+                  >
-                    >
+                    {sortKey === key ? (
-                      {sortKey === key ? (
+                      <Check className="size-3 shrink-0 text-accent-brand" />
-                        <Check className="size-3 shrink-0 text-accent-brand" />
+                    ) : (
-                      ) : (
+                      <span className="size-3 shrink-0 inline-block" />
-                        <span className="size-3 shrink-0 inline-block" />
+                    )}
-                      )}
+                    {t(
-                      {t(
+                      key === "status-online"
-                        key === "status-online"
+                        ? "hosts.sortOnlineFirst"
-                          ? "hosts.sortOnlineFirst"
+                        : "hosts.sortOfflineFirst",
-                          : key === "status-offline"
+                    )}
-                            ? "hosts.sortOfflineFirst"
+                  </DropdownMenuItem>
-                            : "hosts.sortPinnedFirst",
+                ))}
-                      )}
+                <DropdownMenuSeparator />
-                    </DropdownMenuItem>
+                <DropdownMenuCheckboxItem
-                  ),
+                  checked={pinnedFirst}
-                )}
+                  onCheckedChange={handlePinnedFirstChange}
+                  onSelect={(event) => event.preventDefault()}
+                >
+                  {t("hosts.sortPinnedFirst")}
+                </DropdownMenuCheckboxItem>
               </DropdownMenuContent>
             </DropdownMenu>
             <DropdownMenu>
@@ -1017,7 +993,10 @@ export function HostsPanel({
           children={
             hostTree
               ? groupHosts(
-                  applyFilters(sortHostTree(hostTree, sortKey), filterState),
+                  applyFilters(
+                    sortHostTree(hostTree, sortKey, pinnedFirst),
+                    filterState,
+                  ),
                   groupKey,
                   groupLabel,
                 ).children

src/ui/sidebar/host-sort.test.ts

@@ -0,0 +1,88 @@
+import { describe, expect, it } from "vitest";
+import type { Host, HostFolder } from "@/types/ui-types";
+import { resolveHostSortPreferences, sortHostTree } from "./host-sort";
+
+function host(name: string, pin = false): Host {
+  return {
+    id: name,
+    name,
+    ip: `10.0.0.${name.length}`,
+    online: true,
+    pin,
+  } as Host;
+}
+
+function names(folder: HostFolder): string[] {
+  return folder.children.map((child) => child.name);
+}
+
+describe("sortHostTree", () => {
+  it("sorts pinned and unpinned hosts by name within their groups", () => {
+    const tree: HostFolder = {
+      name: "root",
+      children: [
+        host("gamma"),
+        host("zeta", true),
+        host("beta"),
+        host("alpha", true),
+      ],
+    };
+
+    expect(names(sortHostTree(tree, "name-asc", true))).toEqual([
+      "alpha",
+      "zeta",
+      "beta",
+      "gamma",
+    ]);
+    expect(names(tree)).toEqual(["gamma", "zeta", "beta", "alpha"]);
+  });
+
+  it("keeps pinned-first independent from the selected base sort", () => {
+    const tree: HostFolder = {
+      name: "root",
+      children: [host("alpha"), host("zeta", true), host("beta")],
+    };
+
+    expect(names(sortHostTree(tree, "name-asc"))).toEqual([
+      "alpha",
+      "beta",
+      "zeta",
+    ]);
+    expect(names(sortHostTree(tree, "default", true))).toEqual([
+      "zeta",
+      "alpha",
+      "beta",
+    ]);
+  });
+
+  it("applies combined sorting inside folders", () => {
+    const tree: HostFolder = {
+      name: "root",
+      children: [
+        {
+          name: "production",
+          children: [host("beta"), host("alpha", true)],
+        },
+      ],
+    };
+
+    const sorted = sortHostTree(tree, "name-asc", true);
+    expect(names(sorted.children[0] as HostFolder)).toEqual(["alpha", "beta"]);
+  });
+});
+
+describe("resolveHostSortPreferences", () => {
+  it("migrates the legacy pinned sort without losing the preference", () => {
+    expect(resolveHostSortPreferences("pinned", null)).toEqual({
+      sortKey: "default",
+      pinnedFirst: true,
+    });
+  });
+
+  it("keeps the saved pinned modifier independent from base sorting", () => {
+    expect(resolveHostSortPreferences("name-desc", "true")).toEqual({
+      sortKey: "name-desc",
+      pinnedFirst: true,
+    });
+  });
+});

src/ui/sidebar/host-sort.ts

@@ -0,0 +1,82 @@
+import type { Host, HostFolder } from "@/types/ui-types";
+
+export type SortKey =
+  | "default"
+  | "name-asc"
+  | "name-desc"
+  | "ip-asc"
+  | "ip-desc"
+  | "status-online"
+  | "status-offline";
+
+const SORT_KEYS = new Set<SortKey>([
+  "default",
+  "name-asc",
+  "name-desc",
+  "ip-asc",
+  "ip-desc",
+  "status-online",
+  "status-offline",
+]);
+
+export function resolveHostSortPreferences(
+  savedSortKey: string | null,
+  savedPinnedFirst: string | null,
+): { sortKey: SortKey; pinnedFirst: boolean } {
+  const legacyPinned = savedSortKey === "pinned";
+  return {
+    sortKey: SORT_KEYS.has(savedSortKey as SortKey)
+      ? (savedSortKey as SortKey)
+      : "default",
+    pinnedFirst:
+      savedPinnedFirst === null ? legacyPinned : savedPinnedFirst === "true",
+  };
+}
+
+function isFolder(item: Host | HostFolder): item is HostFolder {
+  return "children" in item;
+}
+
+export function sortHostTree(
+  folder: HostFolder,
+  key: SortKey,
+  pinnedFirst = false,
+): HostFolder {
+  if (key === "default" && !pinnedFirst) return folder;
+
+  const comparator = (a: Host | HostFolder, b: Host | HostFolder): number => {
+    const aIsFolder = isFolder(a);
+    const bIsFolder = isFolder(b);
+    if (aIsFolder && !bIsFolder) return -1;
+    if (!aIsFolder && bIsFolder) return 1;
+    if (aIsFolder && bIsFolder) return a.name.localeCompare(b.name);
+
+    if (pinnedFirst && !!a.pin !== !!b.pin) return b.pin ? 1 : -1;
+
+    switch (key) {
+      case "name-asc":
+        return a.name.localeCompare(b.name);
+      case "name-desc":
+        return b.name.localeCompare(a.name);
+      case "ip-asc":
+        return a.ip.localeCompare(b.ip);
+      case "ip-desc":
+        return b.ip.localeCompare(a.ip);
+      case "status-online":
+        return Number(b.online) - Number(a.online);
+      case "status-offline":
+        return Number(a.online) - Number(b.online);
+      case "default":
+        return 0;
+    }
+  };
+
+  return {
+    ...folder,
+    children: [...folder.children]
+      .sort(comparator)
+      .map((child) =>
+        isFolder(child) ? sortHostTree(child, key, pinnedFirst) : child,
+      ),
+  };
+}