Merge: R2 S2a — e2e scroll-restore fix + columnSort/linkify unit tests

2026-07-21 03:17:16 +02:00
parent f95d889a92 bdb0185db8
commit eea1a22572
4 changed files with 186 additions and 2 deletions
@@ -1,5 +1,5 @@
 import { test, expect } from "@playwright/test";
-import { openApp } from "./helpers";
+import { clickVisibleChannelLink, openApp } from "./helpers";

 // Locks channel-view entry + exit: opening a channel from a feed card replaces the view, and
 // both the in-page Back and the browser Back return to the feed. (This orthogonal channel branch
@@ -44,7 +44,11 @@ test.describe("channel view", () => {
    const saved = await main.evaluate((el) => el.scrollTop);
    expect(saved).toBeGreaterThan(1500);

-    await page.getByTestId("video-card-channel").first().click();
+    // Click a channel card that is actually ON SCREEN — a real user clicks what they see. Clicking
+    // `.first()` here would grab an overscan card ABOVE the viewport, and Playwright's scroll-into-
+    // view would move the feed before navigating, so Back would restore that moved position, not
+    // the one under test. (This is what made the test fail while the app was in fact correct.)
+    expect(await clickVisibleChannelLink(page)).toBe(true);
    await expect(page.getByTestId("channel-title")).toBeVisible();
    await page.getByTestId("channel-back").click();
    await expect(page.getByTestId("video-card").first()).toBeVisible();
@@ -12,6 +12,27 @@ export async function gotoPage(page: Page, id: string): Promise<void> {
  await expect(page.getByTestId(`nav-${id}`)).toHaveAttribute("aria-current", "page");
 }

+/**
+ * Click a channel link that is currently WITHIN the viewport — what a real user does. Never use
+ * `.first()` for this after scrolling: the first rendered card lives in the virtualizer's overscan
+ * ABOVE the viewport, so Playwright would scroll it into view to click it, moving the scroll
+ * position — fatal to any test that then asserts on where the feed was. Returns false if none found.
+ */
+export async function clickVisibleChannelLink(page: Page): Promise<boolean> {
+  const links = page.getByTestId("video-card-channel").filter({ visible: true });
+  const viewport = page.viewportSize();
+  const height = viewport?.height ?? 800;
+  const count = await links.count();
+  for (let i = 0; i < count; i++) {
+    const box = await links.nth(i).boundingBox();
+    if (box && box.y >= 0 && box.y + box.height <= height) {
+      await links.nth(i).click();
+      return true;
+    }
+  }
+  return false;
+}
+
 /** Open the in-app player from the first feed card (clicks its thumbnail link). */
 export async function openFirstVideo(page: Page): Promise<void> {
  await expect(page.getByTestId("video-card").first()).toBeVisible();
@@ -0,0 +1,86 @@
+import { describe, expect, it } from "vitest";
+import { sortRows, parseSortState, cycleSort, type SortState } from "./columnSort";
+
+interface Row {
+  name: string;
+  views: number;
+}
+const rows: Row[] = [
+  { name: "beta", views: 30 },
+  { name: "alpha", views: 100 },
+  { name: "gamma", views: 5 },
+];
+const columns = [
+  { key: "name", sortValue: (r: Row) => r.name },
+  { key: "views", sortValue: (r: Row) => r.views },
+  { key: "nosort" }, // a column with no sortValue
+];
+
+describe("sortRows", () => {
+  it("returns the same array reference when there is no sort", () => {
+    expect(sortRows(rows, columns, null)).toBe(rows);
+  });
+
+  it("returns the input unchanged for a column that has no sortValue", () => {
+    expect(sortRows(rows, columns, { key: "nosort", dir: "asc" })).toBe(rows);
+  });
+
+  it("sorts numbers by numeric difference, not string order", () => {
+    // string order would put "100" before "30"; numeric must not.
+    const asc = sortRows(rows, columns, { key: "views", dir: "asc" }).map((r) => r.views);
+    expect(asc).toEqual([5, 30, 100]);
+    const desc = sortRows(rows, columns, { key: "views", dir: "desc" }).map((r) => r.views);
+    expect(desc).toEqual([100, 30, 5]);
+  });
+
+  it("sorts strings with locale compare, ascending and descending", () => {
+    expect(sortRows(rows, columns, { key: "name", dir: "asc" }).map((r) => r.name)).toEqual([
+      "alpha",
+      "beta",
+      "gamma",
+    ]);
+    expect(sortRows(rows, columns, { key: "name", dir: "desc" }).map((r) => r.name)).toEqual([
+      "gamma",
+      "beta",
+      "alpha",
+    ]);
+  });
+
+  it("does not mutate the input array", () => {
+    const before = rows.map((r) => r.name);
+    sortRows(rows, columns, { key: "name", dir: "asc" });
+    expect(rows.map((r) => r.name)).toEqual(before);
+  });
+});
+
+describe("parseSortState", () => {
+  it("accepts a well-formed state", () => {
+    expect(parseSortState({ key: "views", dir: "desc" })).toEqual({ key: "views", dir: "desc" });
+  });
+
+  it.each([
+    ["null", null],
+    ["a string", "views"],
+    ["missing dir", { key: "views" }],
+    ["a bad dir", { key: "views", dir: "sideways" }],
+    ["a non-string key", { key: 3, dir: "asc" }],
+  ])("coerces %s to null rather than passing junk to the comparator", (_label, raw) => {
+    expect(parseSortState(raw)).toBeNull();
+  });
+});
+
+describe("cycleSort", () => {
+  it("walks unsorted → asc → desc → unsorted for the same key", () => {
+    let s: SortState = null;
+    s = cycleSort(s, "views");
+    expect(s).toEqual({ key: "views", dir: "asc" });
+    s = cycleSort(s, "views");
+    expect(s).toEqual({ key: "views", dir: "desc" });
+    s = cycleSort(s, "views");
+    expect(s).toBeNull();
+  });
+
+  it("jumps straight to ascending when a different column is clicked", () => {
+    expect(cycleSort({ key: "name", dir: "desc" }, "views")).toEqual({ key: "views", dir: "asc" });
+  });
+});
@@ -0,0 +1,73 @@
+import { describe, expect, it } from "vitest";
+import { isValidElement, type ReactNode } from "react";
+import { linkify } from "./linkify";
+
+// linkify returns React nodes without rendering, so we can inspect them structurally in node env:
+// an <a> element carries { href, children:label }, everything else is text held in a Fragment.
+function links(nodes: ReactNode[]): { href: string; label: string }[] {
+  return nodes
+    .filter(
+      (n): n is React.ReactElement<{ href: string; children: string }> =>
+        isValidElement(n) && (n as React.ReactElement).type === "a",
+    )
+    .map((n) => ({ href: n.props.href, label: n.props.children }));
+}
+
+describe("linkify", () => {
+  it("leaves plain prose untouched (no anchors)", () => {
+    expect(links(linkify("just some text, nothing to see"))).toEqual([]);
+  });
+
+  it("links an explicit http(s) URL to itself", () => {
+    expect(links(linkify("see https://example.com/watch?v=1 now"))).toEqual([
+      { href: "https://example.com/watch?v=1", label: "https://example.com/watch?v=1" },
+    ]);
+  });
+
+  it("gives a www.* host an https:// href but keeps the bare label", () => {
+    expect(links(linkify("at www.example.com/foo"))).toEqual([
+      { href: "https://www.example.com/foo", label: "www.example.com/foo" },
+    ]);
+  });
+
+  it("links a bare domain.tld/path", () => {
+    expect(links(linkify("go to example.com/foo please"))).toEqual([
+      { href: "https://example.com/foo", label: "example.com/foo" },
+    ]);
+  });
+
+  it("does not mistake a version number for a bare URL", () => {
+    // letters-only TLD + a required path keep 1.2.3 out (no path, numeric last segment).
+    expect(links(linkify("upgraded to 1.2.3 today"))).toEqual([]);
+  });
+
+  it("links only the handle of a '<platform>: @handle' mention, keeping the label as text", () => {
+    const out = linkify("follow x: @jane please");
+    expect(links(out)).toEqual([{ href: "https://x.com/jane", label: "@jane" }]);
+    // the "x: " prefix survives as text, not swallowed into the link.
+    const text = out
+      .filter((n) => typeof n === "string" || (isValidElement(n) && n.type !== "a"))
+      .map((n) => (isValidElement(n) ? (n.props as { children?: string }).children : n))
+      .join("");
+    expect(text).toContain("x: ");
+  });
+
+  it("maps a platform alias to its canonical base URL", () => {
+    expect(links(linkify("ig: @foo"))).toEqual([
+      { href: "https://instagram.com/foo", label: "@foo" },
+    ]);
+  });
+
+  it("leaves a bare @handle with no platform, and #hashtags, as plain text", () => {
+    expect(links(linkify("hi @nobody and #hashtag"))).toEqual([]);
+  });
+
+  it("handles several links in one string in order", () => {
+    const out = links(linkify("https://a.com/x then youtube: @chan then b.org/y"));
+    expect(out.map((l) => l.href)).toEqual([
+      "https://a.com/x",
+      "https://www.youtube.com/@chan",
+      "https://b.org/y",
+    ]);
+  });
+});