improvement: preserve query parameters in nav menu (#7207)

Fixes #7131

Preserve query parameters when passing in urls to the NavMenu's. This
only preserves `file` for now to change user code as little as possible,
but we can add more if needed (or preserve all)

---------

Co-authored-by: Shahmir Varqha <sham9871@gmail.com>

Author: mscolnick

frontend/src/css/md.css

@@ -78,6 +78,11 @@ a .markdown iconify-icon:first-child {
   margin-inline-end: 0.4em;
 }
 
+/* make links and their contents inline-flex to avoid extra gaps */
+a > .markdown > .paragraph {
+  display: inline-flex;
+}
+
 iconify-icon {
   display: inline-flex;
   align-items: center;

frontend/src/plugins/layout/NavigationMenuPlugin.tsx

@@ -14,11 +14,13 @@ import {
 import { Tooltip, TooltipProvider } from "@/components/ui/tooltip";
 import { renderHTML } from "@/plugins/core/RenderHTML";
 import { cn } from "@/utils/cn";
+import { appendQueryParams } from "@/utils/urls";
 import type {
   IStatelessPlugin,
   IStatelessPluginProps,
 } from "../stateless-plugin";
 import "./navigation-menu.css";
+import { KnownQueryParams } from "@/core/constants";
 
 interface MenuItem {
   label: string;
@@ -99,6 +101,15 @@ const NavMenuComponent = ({
     return "_self";
   };
 
+  const preserveQueryParams = (href: string) => {
+    const currentUrl = new URL(globalThis.location.href);
+    return appendQueryParams({
+      href,
+      queryParams: currentUrl.search,
+      keys: [KnownQueryParams.filePath],
+    });
+  };
+
   const renderMenuItem = (item: MenuItem | MenuItemGroup) => {
     if ("items" in item) {
       return orientation === "horizontal" ? (
@@ -113,7 +124,7 @@ const NavMenuComponent = ({
                   <ListItem
                     key={subItem.label}
                     label={subItem.label}
-                    href={subItem.href}
+                    href={preserveQueryParams(subItem.href)}
                     target={target(subItem.href)}
                   >
                     {subItem.description &&
@@ -142,7 +153,7 @@ const NavMenuComponent = ({
                 {maybeWithTooltip(
                   <NavigationMenuLink
                     key={subItem.label}
-                    href={subItem.href}
+                    href={preserveQueryParams(subItem.href)}
                     target={target(subItem.href)}
                     className={navigationMenuTriggerStyle({
                       orientation: orientation,
@@ -162,7 +173,7 @@ const NavMenuComponent = ({
     return (
       <NavigationMenuItem key={item.label}>
         <NavigationMenuLink
-          href={item.href}
+          href={preserveQueryParams(item.href)}
           target={target(item.href)}
           className={navigationMenuTriggerStyle({
             orientation: orientation,

frontend/src/utils/__tests__/urls.test.ts

@@ -4,7 +4,7 @@ import {
   EDGE_CASE_FILENAMES,
   URL_SPECIAL_CHAR_FILENAMES,
 } from "../../__tests__/mocks";
-import { isUrl, updateQueryParams } from "../urls";
+import { appendQueryParams, isUrl, updateQueryParams } from "../urls";
 
 describe("isUrl", () => {
   it("should return true for a valid URL", () => {
@@ -63,3 +63,167 @@ describe("URL parameter handling with edge case filenames", () => {
     });
   });
 });
+
+describe("appendQueryParams", () => {
+  it("should append params to a simple path", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: new URLSearchParams("file=test.py"),
+    });
+    expect(result).toBe("/about?file=test.py");
+  });
+
+  it("should append params to a hash-based path", () => {
+    const result = appendQueryParams({
+      href: "#/about",
+      queryParams: new URLSearchParams("file=test.py"),
+    });
+    expect(result).toBe("/?file=test.py#/about");
+  });
+
+  it("should accept query string instead of URLSearchParams", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: "file=test.py&mode=edit",
+    });
+    expect(result).toBe("/about?file=test.py&mode=edit");
+  });
+
+  it("should filter params by keys when provided", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: "file=test.py&mode=edit&extra=data",
+      keys: ["file", "mode"],
+    });
+    expect(result).toBe("/about?file=test.py&mode=edit");
+  });
+
+  it("should filter params by keys in the middle of the query string", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: "file=test.py&mode=edit&extra=data",
+      keys: ["file", "extra"],
+    });
+    expect(result).toBe("/about?file=test.py&extra=data");
+  });
+
+  it("should preserve existing query params", () => {
+    const result = appendQueryParams({
+      href: "/about?existing=1",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("/about?existing=1&file=test.py");
+  });
+
+  it("should overwrite existing params with same key", () => {
+    const result = appendQueryParams({
+      href: "/about?file=old.py",
+      queryParams: "file=new.py",
+    });
+    expect(result).toBe("/about?file=new.py");
+  });
+
+  it("should preserve hash fragment and put params before it", () => {
+    const result = appendQueryParams({
+      href: "/about#section",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("/about?file=test.py#section");
+  });
+
+  it("should handle hash-based path with existing params and hash", () => {
+    const result = appendQueryParams({
+      href: "#/about?existing=1",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("#/about?existing=1&file=test.py");
+  });
+
+  it("should handle hash-based path", () => {
+    const result = appendQueryParams({
+      href: "#/about",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("/?file=test.py#/about");
+  });
+
+  it("should not modify external links", () => {
+    const httpUrl = "http://example.com/page";
+    const httpsUrl = "https://example.com/page";
+
+    expect(
+      appendQueryParams({
+        href: httpUrl,
+        queryParams: "file=test.py",
+      }),
+    ).toBe(httpUrl);
+
+    expect(
+      appendQueryParams({
+        href: httpsUrl,
+        queryParams: "file=test.py",
+      }),
+    ).toBe(httpsUrl);
+  });
+
+  it("should return original href when no params to append", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: new URLSearchParams(),
+    });
+    expect(result).toBe("/about");
+  });
+
+  it("should handle complex scenarios with all features", () => {
+    const result = appendQueryParams({
+      href: "#/dashboard?view=grid#top",
+      queryParams: "file=notebook.py&view=list&mode=edit",
+      keys: ["file", "mode"],
+    });
+    // view=grid should remain, file and mode should be added (view is not in keys)
+    expect(result).toBe("#/dashboard?view=grid&file=notebook.py&mode=edit#top");
+  });
+
+  it("should handle empty path", () => {
+    const result = appendQueryParams({
+      href: "",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("?file=test.py");
+  });
+
+  it("should handle just a hash", () => {
+    const result = appendQueryParams({
+      href: "#",
+      queryParams: "file=test.py",
+    });
+    expect(result).toBe("/?file=test.py#");
+  });
+
+  it("should handle unicode filenames in params", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: new URLSearchParams([
+        ["file", "文件.py"],
+        ["name", "テスト"],
+      ]),
+    });
+    expect(result).toContain("/about?");
+    // Verify the params are properly encoded
+    const url = new URL(result, "http://example.com");
+    expect(url.searchParams.get("file")).toBe("文件.py");
+    expect(url.searchParams.get("name")).toBe("テスト");
+  });
+
+  it("should handle special characters in param values", () => {
+    const result = appendQueryParams({
+      href: "/about",
+      queryParams: new URLSearchParams([
+        ["path", "folder/file with spaces.py"],
+      ]),
+    });
+    expect(result).toContain("/about?");
+    const url = new URL(result, "http://example.com");
+    expect(url.searchParams.get("path")).toBe("folder/file with spaces.py");
+  });
+});

frontend/src/utils/urls.ts

@@ -31,3 +31,123 @@ const urlRegex = /^(https?:\/\/\S+)$/;
 export function isUrl(value: unknown): boolean {
   return typeof value === "string" && urlRegex.test(value);
 }
+
+/**
+ * Appends query parameters to an href, handling various edge cases.
+ *
+ * @param href - The href to append params to (e.g., "/path", "#/hash", "/path?existing=1", "/path#hash")
+ * @param queryParams - URLSearchParams or query string to append
+ * @param keys - Optional array of keys to filter which params to append
+ * @returns The href with appended query parameters
+ *
+ * @example
+ * appendQueryParams({ href: "/about", queryParams: new URLSearchParams("file=test.py") })
+ * // Returns: "/about?file=test.py"
+ *
+ * @example
+ * appendQueryParams({ href: "#/about", queryParams: "file=test.py&mode=edit", keys: ["file"] })
+ * // Returns: "/?file=test.py#/about"
+ *
+ * @example
+ * appendQueryParams({ href: "#/about?existing=1", queryParams: "file=test.py" })
+ * // Returns: "#/about?existing=1&file=test.py"
+ *
+ * @example
+ * appendQueryParams({ href: "/about?existing=1", queryParams: "file=test.py" })
+ * // Returns: "/about?existing=1&file=test.py"
+ *
+ * @example
+ * appendQueryParams({ href: "/about#section", queryParams: "file=test.py" })
+ * // Returns: "/about?file=test.py#section"
+ */
+export function appendQueryParams({
+  href,
+  queryParams,
+  keys,
+}: {
+  href: string;
+  queryParams: URLSearchParams | string;
+  keys?: string[];
+}): string {
+  // Convert queryParams to URLSearchParams if it's a string
+  const params =
+    typeof queryParams === "string"
+      ? new URLSearchParams(queryParams)
+      : queryParams;
+
+  // If no params to append, return as is
+  if (params.size === 0) {
+    return href;
+  }
+
+  // Don't modify external links (full URLs)
+  if (href.startsWith("http://") || href.startsWith("https://")) {
+    return href;
+  }
+
+  // Special handling for hash-based routing
+  const isHashBased = href.startsWith("#");
+  const hasQueryInHash = isHashBased && href.includes("?");
+
+  if (isHashBased && !hasQueryInHash) {
+    // For hash-based hrefs without query params (e.g., #/about),
+    // put query params on the main path before the hash: /?params#/route
+    // This is common in SPAs where query params on the main URL need to be preserved
+    const paramsToAdd = keys
+      ? [...params.entries()].filter(([key]) => keys.includes(key))
+      : [...params.entries()];
+
+    const queryParams = new URLSearchParams();
+    for (const [key, value] of paramsToAdd) {
+      queryParams.set(key, value);
+    }
+
+    const queryString = queryParams.toString();
+    if (!queryString) {
+      return href;
+    }
+
+    return `/?${queryString}${href}`;
+  }
+
+  // Parse the href to extract parts for all other cases
+  let basePath = href;
+  let hash = "";
+  let existingParams = new URLSearchParams();
+
+  // For hash-based routing with query params, look for a second # to find actual hash fragment
+  const hashSearchStart = isHashBased ? 1 : 0;
+  const hashIndex = href.indexOf("#", hashSearchStart);
+
+  if (hashIndex !== -1) {
+    hash = href.slice(hashIndex);
+    basePath = href.slice(0, hashIndex);
+  }
+
+  // Extract existing query params
+  const queryIndex = basePath.indexOf("?");
+  if (queryIndex !== -1) {
+    existingParams = new URLSearchParams(basePath.slice(queryIndex + 1));
+    basePath = basePath.slice(0, queryIndex);
+  }
+
+  // Merge params (new params overwrite existing ones with same key)
+  const mergedParams = new URLSearchParams(existingParams);
+
+  // Filter params by keys if provided
+  const paramsToAdd = keys
+    ? [...params.entries()].filter(([key]) => keys.includes(key))
+    : [...params.entries()];
+
+  for (const [key, value] of paramsToAdd) {
+    mergedParams.set(key, value);
+  }
+
+  // Build the final URL
+  const queryString = mergedParams.toString();
+  if (!queryString) {
+    return href;
+  }
+
+  return `${basePath}?${queryString}${hash}`;
+}