From 7ec3d45e9c8729a2cd647d1e28f6c1c2ef47346b Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 7 Apr 2026 11:29:18 -0400 Subject: [PATCH 1/4] fix(ui): preserves scroll position on repo lock/move/unlock --- .../components/shared/RepoLockControls.tsx | 14 +++-- .../shared/RepoLockControls.test.tsx | 52 ++++++++++++++++++- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/app/components/shared/RepoLockControls.tsx b/src/app/components/shared/RepoLockControls.tsx index 521414f9..5b9544ed 100644 --- a/src/app/components/shared/RepoLockControls.tsx +++ b/src/app/components/shared/RepoLockControls.tsx @@ -7,6 +7,12 @@ interface RepoLockControlsProps { repoFullName: string; } +const withScrollLock = (fn: () => void) => { + const y = window.scrollY; + fn(); + window.scrollTo(0, y); +}; + export default function RepoLockControls(props: RepoLockControlsProps) { const lockInfo = createMemo(() => { const list = viewState.lockedRepos[props.tab]; @@ -26,7 +32,7 @@ export default function RepoLockControls(props: RepoLockControlsProps) { - · - - - - + const orgContent = () => ( + <> + {/* Per-org bulk selection bar (accordion mode only — in non-accordion, these are in the header) */} + +
+ + · + +
+ {/* Loading state for this org */} @@ -532,8 +543,7 @@ export default function RepoSelector(props: RepoSelectorProps) { >
    - {(repo) => { - return ( + {(repo) => (
    • - ); - }} + )}
+ + ); + + return ( +
+ {/* Org header — accordion button when >= 6 orgs, plain header otherwise */} + + + {state().org} + + +
+ + · + +
+ +
+ } + > + + + + {/* Content: wrapped in grid animation for accordion, direct otherwise */} + + {orgContent()} + + +
+
+ {orgContent()} +
+
+ ); }} diff --git a/src/app/index.css b/src/app/index.css index 6b6f5d8d..52002308 100644 --- a/src/app/index.css +++ b/src/app/index.css @@ -145,4 +145,7 @@ .loading { animation: none; } + .accordion-panel { + transition: none; + } } diff --git a/tests/components/onboarding/RepoSelector.test.tsx b/tests/components/onboarding/RepoSelector.test.tsx index 1ccf2154..1427549c 100644 --- a/tests/components/onboarding/RepoSelector.test.tsx +++ b/tests/components/onboarding/RepoSelector.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, waitFor } from "@solidjs/testing-library"; +import { render, screen, waitFor, fireEvent } from "@solidjs/testing-library"; import userEvent from "@testing-library/user-event"; import type { RepoRef, RepoEntry } from "../../../src/app/services/api"; @@ -390,8 +390,9 @@ describe("RepoSelector — upstream discovery", () => { await waitFor(() => { screen.getByText("repo-a"); }); - // Give time for any async effects to settle - await new Promise((r) => setTimeout(r, 50)); + // Verify discoverUpstreamRepos was never called after repos finished loading. + // waitFor above already ensures the async effect settled (fetchRepos resolved), + // so no additional delay is needed. expect(api.discoverUpstreamRepos).not.toHaveBeenCalled(); }); @@ -799,3 +800,349 @@ describe("RepoSelector — monitor toggle", () => { expect(screen.queryByLabelText(/monitor all activity/i)).toBeNull(); }); }); + +// ── Org-grouped accordion (C2) ──────────────────────────────────────────────── + +describe("RepoSelector — org accordion", () => { + // 6 org names that trigger accordion mode (threshold is >= 6) + const sixOrgs = ["alpha-org", "beta-org", "gamma-org", "delta-org", "epsilon-org", "zeta-org"]; + + // Minimal OrgEntry list for preloading (skips fetchOrgs network call) + const sixOrgEntries = sixOrgs.map((login) => ({ + login, + avatarUrl: "", + type: "org" as const, + })); + + // One repo per org + function makeOrgRepos(org: string): RepoEntry[] { + return [{ owner: org, name: `${org}-repo`, fullName: `${org}/${org}-repo`, pushedAt: "2026-03-20T10:00:00Z" }]; + } + + beforeEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => + Promise.resolve(makeOrgRepos(org as string)) + ); + }); + + it("renders all orgs expanded when fewer than 6 orgs", async () => { + // 4 orgs → no accordion + const fourOrgs = ["alpha-org", "beta-org", "gamma-org", "delta-org"]; + const fourOrgEntries = fourOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + screen.getByText("delta-org-repo"); + }); + + // No accordion chevron buttons present + expect(screen.queryAllByRole("button", { name: /chevron|expand|collapse/i })).toHaveLength(0); + // All repo content visible (no inert panels) + expect(document.querySelectorAll("[inert]")).toHaveLength(0); + }); + + it("renders accordion with first org expanded by default when 6+ orgs", async () => { + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // First org panel is not inert (expanded) + const panels = document.querySelectorAll("[inert]"); + // All other 5 orgs are collapsed (inert) + expect(panels).toHaveLength(5); + + // First org repo content is accessible + expect(screen.queryByText("alpha-org-repo")).not.toBeNull(); + }); + + it("clicking another org header collapses the first and expands the clicked one", async () => { + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Find and click the beta-org accordion header button + const betaBtn = screen.getByRole("button", { name: /beta-org/ }); + fireEvent.click(betaBtn); + + // Now beta-org panel should be expanded (not inert), alpha-org collapsed + await waitFor(() => { + // 5 orgs still collapsed + expect(document.querySelectorAll("[inert]")).toHaveLength(5); + }); + + // The expanded button should now be beta-org (aria-expanded=true) + expect(betaBtn.getAttribute("aria-expanded")).toBe("true"); + }); + + it("clicking the currently-expanded org header is a no-op (content stays visible)", async () => { + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Click the already-expanded first org header + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + fireEvent.click(alphaBtn); + + // alpha-org is still expanded (setUserExpandedOrg sets to same value) + expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); + expect(document.querySelectorAll("[inert]")).toHaveLength(5); + }); + + it("shows repo count badge on org headers in accordion mode", async () => { + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Each org has 1 repo — badge shows "1 repo" + const badges = screen.getAllByText("1 repo"); + expect(badges.length).toBe(sixOrgs.length); + }); + + it("shows per-org Select All / Deselect All in expanded accordion panel", async () => { + const onChange = vi.fn(); + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // The expanded org (alpha-org) should show per-org Select All inside the panel content + const selectAllBtns = screen.getAllByRole("button", { name: /Select All/i }); + // Global Select All + per-org Select All for the expanded org + const perOrgSelectAll = selectAllBtns.filter( + (btn) => !btn.closest("[inert]") && btn.textContent === "Select All" + ); + expect(perOrgSelectAll.length).toBeGreaterThanOrEqual(2); // global + expanded org + + // Click per-org Select All inside the expanded panel (not the global one) + // The per-org one is inside the accordion panel content area + const panelContent = document.getElementById("accordion-panel-alpha-org"); + expect(panelContent).not.toBeNull(); + const perOrgBtn = panelContent!.querySelector("button"); + expect(perOrgBtn).not.toBeNull(); + expect(perOrgBtn!.textContent).toBe("Select All"); + fireEvent.click(perOrgBtn!); + + // Should select only alpha-org's repo + expect(onChange).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({ fullName: "alpha-org/alpha-org-repo" })]) + ); + // Should NOT include repos from other orgs + const selectedNames = onChange.mock.calls[0][0].map((r: { fullName: string }) => r.fullName); + expect(selectedNames).toHaveLength(1); + expect(selectedNames[0]).toBe("alpha-org/alpha-org-repo"); + }); + + it("global Select All includes repos from collapsed orgs", async () => { + const onChange = vi.fn(); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Click the global Select All (first button with that text) + const selectAllBtns = screen.getAllByText("Select All"); + fireEvent.click(selectAllBtns[0]); + + expect(onChange).toHaveBeenCalled(); + const result = onChange.mock.calls[0][0] as RepoRef[]; + // Should include all 6 repos, one per org + expect(result.length).toBe(6); + for (const org of sixOrgs) { + expect(result.map((r) => r.fullName)).toContain(`${org}/${org}-repo`); + } + }); + + it("global Select All with text filter only includes repos matching the filter (PA-003)", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Type a filter that only matches repos in "alpha-org" + const filterInput = screen.getByPlaceholderText(/Filter repos/i); + await user.type(filterInput, "alpha-org-repo"); + + // Wait for the debounce to fire and the filter to take effect: + // the badge for "beta-org" should drop to "0 repos" once the filter is active + await waitFor(() => { + const betaBtn = screen.getByRole("button", { name: /beta-org/ }); + expect(betaBtn.textContent).toContain("0 repos"); + }); + + // Click global Select All — should only include repos visible after filter + const selectAllBtns = screen.getAllByText("Select All"); + fireEvent.click(selectAllBtns[0]); + + expect(onChange).toHaveBeenCalled(); + const result = onChange.mock.calls[0][0] as RepoRef[]; + // Only alpha-org-repo matches the filter + expect(result.map((r) => r.fullName)).toContain("alpha-org/alpha-org-repo"); + // Repos from other orgs must NOT be included + for (const org of sixOrgs.filter((o) => o !== "alpha-org")) { + expect(result.map((r) => r.fullName)).not.toContain(`${org}/${org}-repo`); + } + }); + + it("expanded org panel does not have inert; collapsed org panels do have inert", async () => { + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // In accordion mode, expanded org is alpha-org (first alphabetically) + // The panel id is on the inner div that also carries the inert attribute + const alphaPanel = document.getElementById("accordion-panel-alpha-org") as HTMLElement; + expect(alphaPanel).not.toBeNull(); + expect(alphaPanel.hasAttribute("inert")).toBe(false); + + // All other org panels should have inert + for (const org of sixOrgs.filter((o) => o !== "alpha-org")) { + const panel = document.getElementById(`accordion-panel-${org}`) as HTMLElement; + expect(panel).not.toBeNull(); + expect(panel.hasAttribute("inert")).toBe(true); + } + }); + + it("org removal falls back to first remaining org when removed org was expanded", async () => { + const { createSignal } = await import("solid-js"); + const [selectedOrgs, setSelectedOrgs] = createSignal(sixOrgs); + const [orgEntries, setOrgEntries] = createSignal(sixOrgEntries); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Expand the 3rd org (gamma-org) + const gammaBtn = screen.getByRole("button", { name: /gamma-org/ }); + fireEvent.click(gammaBtn); + expect(gammaBtn.getAttribute("aria-expanded")).toBe("true"); + + // Re-render without gamma-org (still 5 orgs >= 6 threshold? No, 5 < 6 so accordion exits) + // Use 7 orgs to ensure >= 6 remains after removing one + // Actually: let's test with 7 orgs so we stay in accordion mode after removal + const sevenOrgs = [...sixOrgs, "eta-org"]; + const sevenOrgEntries = sevenOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => + Promise.resolve(makeOrgRepos(org as string)) + ); + + // Re-render to 7 orgs first + setSelectedOrgs(sevenOrgs); + setOrgEntries(sevenOrgEntries); + + await waitFor(() => { + screen.getByText("eta-org-repo"); + }); + + // Expand gamma-org + const gammaBtnAgain = screen.getByRole("button", { name: /gamma-org/ }); + fireEvent.click(gammaBtnAgain); + expect(gammaBtnAgain.getAttribute("aria-expanded")).toBe("true"); + + // Now remove gamma-org (still 6 orgs remain → still accordion) + const remainingOrgs = sevenOrgs.filter((o) => o !== "gamma-org"); + const remainingEntries = remainingOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); + setSelectedOrgs(remainingOrgs); + setOrgEntries(remainingEntries); + + await waitFor(() => { + expect(screen.queryByRole("button", { name: /gamma-org/ })).toBeNull(); + }); + + // expandedOrg should fall back to states[0] (alpha-org) + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); + }); +}); From 3e77d319a2e12dfcab4ada9f8d0647ac30fe705b Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 7 Apr 2026 15:55:24 -0400 Subject: [PATCH 3/4] feat(ui): FLIP animation, poll scroll fix, and accordion UX improvements - extracts withScrollLock to shared src/app/lib/scroll.ts with try/finally - adds withFlipAnimation (200ms ease-in-out, reduced-motion fallback) - replaces instant repo pin/move with FLIP animation via data-repo-group attrs - preserves scroll position on timed poll refresh in DashboardPage - refactors accordion: single bordered unit, inline Select/Deselect in header bar, loading spinner on collapsed headers, orgId sanitization for DOM IDs - refactors orgContent closure (removes isAccordion check), Show+fallback pattern - adds 12 new tests (1573 total, 71 files, typecheck clean) --- src/app/components/dashboard/ActionsTab.tsx | 2 +- .../components/dashboard/DashboardPage.tsx | 19 +- src/app/components/dashboard/IssuesTab.tsx | 2 +- .../components/dashboard/PullRequestsTab.tsx | 2 +- .../components/onboarding/RepoSelector.tsx | 114 +++++---- .../components/shared/RepoLockControls.tsx | 15 +- src/app/lib/scroll.ts | 55 +++++ tests/components/DashboardPage.test.tsx | 30 +++ .../onboarding/RepoSelector.test.tsx | 223 ++++++++++++++++-- .../shared/RepoLockControls.test.tsx | 3 + tests/lib/scroll.test.ts | 58 +++++ 11 files changed, 431 insertions(+), 92 deletions(-) create mode 100644 src/app/lib/scroll.ts create mode 100644 tests/lib/scroll.test.ts diff --git a/src/app/components/dashboard/ActionsTab.tsx b/src/app/components/dashboard/ActionsTab.tsx index 99925d42..19784de8 100644 --- a/src/app/components/dashboard/ActionsTab.tsx +++ b/src/app/components/dashboard/ActionsTab.tsx @@ -291,7 +291,7 @@ export default function ActionsTab(props: ActionsTabProps) { }); return ( -
+
{/* Repo header */}
- · - -
- - {/* Loading state for this org */}
@@ -602,7 +575,7 @@ export default function RepoSelector(props: RepoSelectorProps) { ); return ( -
+
{/* Org header — accordion button when >= 6 orgs, plain header otherwise */} } > - + {/* Per-org bulk actions — inline in the header bar when expanded */} + +
+ + · + +
- +
{/* Content: wrapped in grid animation for accordion, direct otherwise */} - - {orgContent()} - - +
@@ -684,6 +693,7 @@ export default function RepoSelector(props: RepoSelectorProps) { ); }} +
{/* Upstream Repositories section */} diff --git a/src/app/components/shared/RepoLockControls.tsx b/src/app/components/shared/RepoLockControls.tsx index 5b9544ed..aea82998 100644 --- a/src/app/components/shared/RepoLockControls.tsx +++ b/src/app/components/shared/RepoLockControls.tsx @@ -1,18 +1,13 @@ import { Show, createMemo } from "solid-js"; import { viewState, lockRepo, unlockRepo, moveLockedRepo, type LockedReposTab } from "../../stores/view"; import { Tooltip } from "./Tooltip"; +import { withFlipAnimation } from "../../lib/scroll"; interface RepoLockControlsProps { tab: LockedReposTab; repoFullName: string; } -const withScrollLock = (fn: () => void) => { - const y = window.scrollY; - fn(); - window.scrollTo(0, y); -}; - export default function RepoLockControls(props: RepoLockControlsProps) { const lockInfo = createMemo(() => { const list = viewState.lockedRepos[props.tab]; @@ -32,7 +27,7 @@ export default function RepoLockControls(props: RepoLockControlsProps) { +
+ + + + 0} + fallback={ +

+ {props.q + ? "No repos match your filter." + : "No repositories found."} +

+ } + > +
+
    + + {(repo) => ( +
  • +
    + + + + + + +
    +
    • + )} +     +
+
+ + + + ); +} + export default function RepoSelector(props: RepoSelectorProps) { const [filter, setFilter] = createSignal(""); const [orgStates, setOrgStates] = createSignal([]); @@ -410,14 +520,18 @@ export default function RepoSelector(props: RepoSelectorProps) { // ── Accordion state ─────────────────────────────────────────────────────── - const [userExpandedOrg, setUserExpandedOrg] = createSignal(null); - const isAccordion = createMemo(() => sortedOrgStates().length >= 6); - const expandedOrg = createMemo(() => { - if (!isAccordion()) return null; + const isAccordion = createMemo(() => props.selectedOrgs.length >= 6); + // Stable default: props.selectedOrgs[0] avoids the mid-load shift that + // occurs when sortedOrgStates switches from insertion-order to alphabetical + const [expandedOrg, setExpandedOrg] = createSignal( + props.selectedOrgs[0] ?? "" + ); + const safeExpandedOrg = createMemo(() => { const states = sortedOrgStates(); - const userChoice = userExpandedOrg(); - if (userChoice !== null && states.some(s => s.org === userChoice)) return userChoice; - return states.length > 0 ? states[0].org : null; + const current = expandedOrg(); + if (states.some(s => s.org === current)) return current; + const stateOrgs = new Set(states.map(s => s.org)); + return props.selectedOrgs.find(o => stateOrgs.has(o)) ?? (states.length > 0 ? states[0].org : ""); }); // ── Status ──────────────────────────────────────────────────────────────── @@ -463,123 +577,14 @@ export default function RepoSelector(props: RepoSelectorProps) { {/* Per-org repo lists — Index (not For) avoids tearing down every org's DOM subtree when a single org's state updates via setOrgStates(prev.map(...)) */} -
- - {(state) => { - const visible = createMemo(() => filteredReposForOrg(state())); - const selectedCount = createMemo(() => - visible().filter((r) => isSelected(r.fullName)).length - ); - const orgId = createMemo(() => state().org.replace(/[^a-zA-Z0-9-]/g, "-")); - - const orgContent = () => ( - <> - {/* Loading state for this org */} - -
- -
- - - {/* Error state for this org */} - -
- - {state().error} - - -
- - - {/* Repo list */} - - 0} - fallback={ -

- {q() - ? "No repos match your filter." - : "No repositories found."} -

- } - > -
-
    - - {(repo) => ( -
  • -
    - - - - - - -
    -
    • - )} -     -
-
- - - - ); - - return ( -
- {/* Org header — accordion button when >= 6 orgs, plain header otherwise */} - + {(state) => { + const visible = createMemo(() => filteredReposForOrg(state())); + return ( +
{state().org} @@ -612,88 +617,90 @@ export default function RepoSelector(props: RepoSelectorProps) {
- } - > -
- - {/* Per-org bulk actions — inline in the header bar when expanded */} - -
- - · - -
- +
- - - {/* Content: wrapped in grid animation for accordion, direct otherwise */} - -
-
- {orgContent()} + ); + }} + + } + > + { + if (vals.length > 0) setExpandedOrg(vals[0]); + }} + > + + {(state) => { + const visible = createMemo(() => filteredReposForOrg(state())); + // Count against ALL repos in the org (unfiltered) so the badge + // doesn't mislead users into thinking selections were lost when + // a text filter is active. + const selectedCount = createMemo(() => + state().repos.filter((r) => isSelected(r.fullName)).length + ); + const isExpanded = () => safeExpandedOrg() === state().org; + return ( + +
+ + + + + {state().org} + + + + + +
+ + · + +
+
-
- -
- ); - }} - -
+ + + + + ); + }} + + + {/* Upstream Repositories section */} diff --git a/src/app/index.css b/src/app/index.css index 52002308..37192f21 100644 --- a/src/app/index.css +++ b/src/app/index.css @@ -136,6 +136,25 @@ scrollbar-color: color-mix(in oklch, var(--color-base-content) 30%, transparent) var(--color-base-200); } +/* ── Kobalte accordion animation ─────────────────────────────────────────── */ + +.kb-accordion-content { + overflow: hidden; + animation: kb-accordion-down 200ms ease-in-out forwards; +} +.kb-accordion-content[data-closed] { + animation: kb-accordion-up 200ms ease-in-out forwards; +} + +@keyframes kb-accordion-down { + from { height: 0; } + to { height: var(--kb-accordion-content-height); } +} +@keyframes kb-accordion-up { + from { height: var(--kb-accordion-content-height); } + to { height: 0; } +} + @media (prefers-reduced-motion: reduce) { .animate-slow-pulse, .animate-toast-in, .animate-toast-out, @@ -145,7 +164,8 @@ .loading { animation: none; } - .accordion-panel { - transition: none; + .kb-accordion-content, + .kb-accordion-content[data-closed] { + animation: none; } } diff --git a/src/app/lib/scroll.ts b/src/app/lib/scroll.ts index e688a3ac..82e0f1e1 100644 --- a/src/app/lib/scroll.ts +++ b/src/app/lib/scroll.ts @@ -5,7 +5,11 @@ */ export function withScrollLock(fn: () => void): void { const y = window.scrollY; - try { fn(); } finally { window.scrollTo(0, y); } + try { + fn(); + } finally { + window.scrollTo(0, y); + } } /** @@ -34,18 +38,29 @@ export function withFlipAnimation(fn: () => void): void { } // Execute state change (SolidJS updates DOM synchronously) + const scrollY = window.scrollY; fn(); - // Last, Invert, Play + // Last, Invert, Play — reads before writes to avoid layout thrash. + // All getBoundingClientRect calls happen before scrollTo so the browser + // doesn't force a synchronous layout recalculation between them. requestAnimationFrame(() => { const afterItems = document.querySelectorAll("[data-repo-group]"); + const scrollDrift = window.scrollY - scrollY; + const deltas: { el: HTMLElement; dy: number }[] = []; for (const el of afterItems) { const key = el.dataset.repoGroup!; const old = before.get(key); if (!old) continue; const now = el.getBoundingClientRect(); - const dy = old.top - now.top; + // Adjust for scroll drift: old.top was measured at scrollY, + // now.top is measured at the current (possibly drifted) scroll position. + const dy = old.top - now.top - scrollDrift; if (Math.abs(dy) < 1) continue; + deltas.push({ el, dy }); + } + window.scrollTo(0, scrollY); + for (const { el, dy } of deltas) { el.animate( [{ transform: `translateY(${dy}px)` }, { transform: "translateY(0)" }], { duration: 200, easing: "ease-in-out" }, diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index 77ea4458..452ea33f 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -682,6 +682,17 @@ describe("DashboardPage — onAuthCleared integration", () => { }); describe("DashboardPage — scroll preservation on poll refresh", () => { + // MOCK INVARIANT: fetchAllData is mocked via vi.fn() and never calls its + // onLightData callback, so phaseOneFired is always false inside pollFetch(). + // This means every poll cycle takes the withScrollLock branch (not the + // fine-grained produce() path). If fetchAllData is ever changed to invoke + // onLightData in tests, phaseOneFired will become true and withScrollLock + // will NOT be called, silently breaking this test. + // + // window.scrollTo is the correct behavioral proxy for withScrollLock: + // withScrollLock captures scrollY then calls window.scrollTo(0, y) after + // the setter. Asserting scrollTo was called with the saved position is + // equivalent to asserting withScrollLock ran and completed successfully. it("preserves scroll position when setDashboardData replaces arrays", async () => { const issues = [makeIssue({ id: 1, title: "Scroll test issue" })]; vi.mocked(pollService.fetchAllData).mockResolvedValue({ @@ -700,11 +711,15 @@ describe("DashboardPage — scroll preservation on poll refresh", () => { document.documentElement.scrollTop = 500; vi.spyOn(window, "scrollTo"); - // Trigger a second poll (subsequent refresh — the path that uses withScrollLock) + // Trigger a second poll (subsequent refresh — the path that uses withScrollLock). + // phaseOneFired is false (mock never calls onLightData), so withScrollLock + // wraps the full atomic setDashboardData replacement and restores scroll. if (capturedFetchAll) { await capturedFetchAll(); } + // window.scrollTo(0, 500) is the observable side-effect of withScrollLock + // saving and restoring the pre-update scroll position. expect(window.scrollTo).toHaveBeenCalledWith(0, 500); vi.restoreAllMocks(); document.documentElement.scrollTop = 0; diff --git a/tests/components/onboarding/RepoSelector.test.tsx b/tests/components/onboarding/RepoSelector.test.tsx index 123c162e..f6d89acb 100644 --- a/tests/components/onboarding/RepoSelector.test.tsx +++ b/tests/components/onboarding/RepoSelector.test.tsx @@ -348,8 +348,9 @@ describe("RepoSelector", () => { await waitFor(() => { screen.getByText("repo-a"); }); - const scrollContainer = screen.getByRole("region", { name: "myorg repositories" }); - expect(scrollContainer.classList.contains("max-h-[300px]")).toBe(true); + const region = screen.getByRole("region", { name: "myorg repositories" }); + const scrollContainer = region.querySelector(".max-h-\\[300px\\]")!; + expect(scrollContainer).not.toBeNull(); expect(scrollContainer.classList.contains("overflow-y-auto")).toBe(true); }); @@ -863,7 +864,6 @@ describe("RepoSelector — org accordion", () => { }); it("renders all orgs expanded when fewer than 6 orgs", async () => { - // 4 orgs → no accordion const fourOrgs = ["alpha-org", "beta-org", "gamma-org", "delta-org"]; const fourOrgEntries = fourOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); @@ -881,14 +881,11 @@ describe("RepoSelector — org accordion", () => { screen.getByText("delta-org-repo"); }); - // No accordion header buttons present (no aria-expanded attributes) + // No accordion triggers present (no aria-expanded attributes) expect(document.querySelectorAll("[aria-expanded]")).toHaveLength(0); - // All repo content visible (no inert panels) - expect(document.querySelectorAll("[inert]")).toHaveLength(0); }); it("renders all orgs expanded when exactly 5 orgs (boundary below threshold)", async () => { - // 5 orgs → still no accordion (threshold is >= 6) const fiveOrgs = ["alpha-org", "beta-org", "gamma-org", "delta-org", "epsilon-org"]; const fiveOrgEntries = fiveOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); @@ -906,10 +903,8 @@ describe("RepoSelector — org accordion", () => { screen.getByText("epsilon-org-repo"); }); - // No accordion chevron buttons present — 5 orgs is still below the >= 6 threshold - expect(screen.queryAllByRole("button", { name: /chevron|expand|collapse/i })).toHaveLength(0); - // All repo content visible — no inert panels - expect(document.querySelectorAll("[inert]")).toHaveLength(0); + // No accordion triggers present — 5 orgs is still below the >= 6 threshold + expect(document.querySelectorAll("[aria-expanded]")).toHaveLength(0); }); it("renders accordion with first org expanded by default when 6+ orgs", async () => { @@ -926,13 +921,13 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // First org panel is not inert (expanded) - const panels = document.querySelectorAll("[inert]"); - // All other 5 orgs are collapsed (inert) - expect(panels).toHaveLength(5); - - // First org repo content is accessible + // First org is expanded (content visible) expect(screen.queryByText("alpha-org-repo")).not.toBeNull(); + // Other orgs' content is not rendered (Kobalte unmounts collapsed panels) + expect(screen.queryByText("beta-org-repo")).toBeNull(); + // First org trigger has aria-expanded=true + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); }); it("clicking another org header collapses the first and expands the clicked one", async () => { @@ -949,18 +944,18 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // Find and click the beta-org accordion header button const betaBtn = screen.getByRole("button", { name: /beta-org/ }); fireEvent.click(betaBtn); - // Now beta-org panel should be expanded (not inert), alpha-org collapsed await waitFor(() => { - // 5 orgs still collapsed - expect(document.querySelectorAll("[inert]")).toHaveLength(5); + expect(betaBtn.getAttribute("aria-expanded")).toBe("true"); + // beta-org content now visible + screen.getByText("beta-org-repo"); }); - // The expanded button should now be beta-org (aria-expanded=true) - expect(betaBtn.getAttribute("aria-expanded")).toBe("true"); + // alpha-org trigger is now collapsed + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.getAttribute("aria-expanded")).toBe("false"); }); it("clicking the default-expanded org header is a no-op (null → explicit path)", async () => { @@ -977,13 +972,12 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // Click the already-expanded first org header const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); fireEvent.click(alphaBtn); - // alpha-org is still expanded (setUserExpandedOrg sets to same value) + // alpha-org is still expanded (not collapsible) expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); - expect(document.querySelectorAll("[inert]")).toHaveLength(5); + expect(screen.queryByText("alpha-org-repo")).not.toBeNull(); }); it("clicking the explicitly-expanded org header a second time is a no-op (re-click path)", async () => { @@ -1000,17 +994,14 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // First: click beta-org to explicitly set userExpandedOrg = 'beta-org' const betaBtn = screen.getByRole("button", { name: /beta-org/ }); fireEvent.click(betaBtn); expect(betaBtn.getAttribute("aria-expanded")).toBe("true"); - expect(document.querySelectorAll("[inert]")).toHaveLength(5); - // Second: click beta-org again — userExpandedOrg is already 'beta-org', - // so setUserExpandedOrg('beta-org') is a no-op and beta-org must remain expanded + // Re-click beta — still expanded (not collapsible) fireEvent.click(betaBtn); expect(betaBtn.getAttribute("aria-expanded")).toBe("true"); - expect(document.querySelectorAll("[inert]")).toHaveLength(5); + screen.getByText("beta-org-repo"); }); it("shows repo count badge on org headers in accordion mode", async () => { @@ -1047,21 +1038,18 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // The expanded org (alpha-org) should show per-org Select All in the header bar - const headerBtn = document.getElementById("accordion-header-alpha-org")!; - expect(headerBtn).not.toBeNull(); - const headerRow = headerBtn.parentElement!; + // The expanded org (alpha-org) has per-org Select All next to the trigger + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + const headerRow = alphaBtn.closest("[class*='border-b']") ?? alphaBtn.parentElement!.parentElement!; const perOrgBtn = Array.from(headerRow.querySelectorAll("button")).find( - (b) => b.textContent === "Select All" && b !== headerBtn + (b) => b.textContent === "Select All" && b !== alphaBtn ); expect(perOrgBtn).not.toBeUndefined(); fireEvent.click(perOrgBtn!); - // Should select only alpha-org's repo expect(onChange).toHaveBeenCalledWith( expect.arrayContaining([expect.objectContaining({ fullName: "alpha-org/alpha-org-repo" })]) ); - // Should NOT include repos from other orgs const selectedNames = onChange.mock.calls[0][0].map((r: { fullName: string }) => r.fullName); expect(selectedNames).toHaveLength(1); expect(selectedNames[0]).toBe("alpha-org/alpha-org-repo"); @@ -1087,22 +1075,52 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // alpha-org is expanded — per-org buttons are in the header bar - const headerBtn = document.getElementById("accordion-header-alpha-org")!; - const headerRow = headerBtn.parentElement!; - const actionBtns = Array.from(headerRow.querySelectorAll("button")).filter((b) => b !== headerBtn); + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + const headerRow = alphaBtn.closest("[class*='border-b']") ?? alphaBtn.parentElement!.parentElement!; + const actionBtns = Array.from(headerRow.querySelectorAll("button")).filter((b) => b !== alphaBtn); const perOrgSelectAll = actionBtns.find((b) => b.textContent === "Select All") as HTMLButtonElement; const perOrgDeselectAll = actionBtns.find((b) => b.textContent === "Deselect All") as HTMLButtonElement; - // All repos are already selected — Select All must be disabled expect(perOrgSelectAll).not.toBeUndefined(); expect(perOrgSelectAll.disabled).toBe(true); - - // All repos are selected — Deselect All must be enabled expect(perOrgDeselectAll).not.toBeUndefined(); expect(perOrgDeselectAll.disabled).toBe(false); }); + it("'N selected' badge counts all selected repos in org, not just filtered ones", async () => { + const user = userEvent.setup(); + const alphaSelected: RepoRef[] = [ + { owner: "alpha-org", name: "alpha-org-repo", fullName: "alpha-org/alpha-org-repo" }, + ]; + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.textContent).toContain("1 selected"); + + // Type a filter that does NOT match alpha-org's repo + const filterInput = screen.getByPlaceholderText(/Filter repos/i); + await user.type(filterInput, "nonexistent"); + + await waitFor(() => { + expect(alphaBtn.textContent).toContain("0 repos"); + }); + + // Badge should still show "1 selected" (unfiltered count) + expect(alphaBtn.textContent).toContain("1 selected"); + }); + it("per-org Deselect All is disabled when no repos are selected in that org", async () => { render(() => ( { screen.getByText("alpha-org-repo"); }); - const headerBtn = document.getElementById("accordion-header-alpha-org")!; - const headerRow = headerBtn.parentElement!; - const actionBtns = Array.from(headerRow.querySelectorAll("button")).filter((b) => b !== headerBtn); + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + const headerRow = alphaBtn.closest("[class*='border-b']") ?? alphaBtn.parentElement!.parentElement!; + const actionBtns = Array.from(headerRow.querySelectorAll("button")).filter((b) => b !== alphaBtn); const perOrgSelectAll = actionBtns.find((b) => b.textContent === "Select All") as HTMLButtonElement; const perOrgDeselectAll = actionBtns.find((b) => b.textContent === "Deselect All") as HTMLButtonElement; - // No repos selected — Select All must be enabled expect(perOrgSelectAll).not.toBeUndefined(); expect(perOrgSelectAll.disabled).toBe(false); - - // No repos selected — Deselect All must be disabled expect(perOrgDeselectAll).not.toBeUndefined(); expect(perOrgDeselectAll.disabled).toBe(true); }); @@ -1153,9 +1168,8 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // Deselect All is in the header bar when expanded - const headerBtn = document.getElementById("accordion-header-alpha-org")!; - const headerRow = headerBtn.parentElement!; + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + const headerRow = alphaBtn.closest("[class*='border-b']") ?? alphaBtn.parentElement!.parentElement!; const perOrgDeselectAll = Array.from(headerRow.querySelectorAll("button")).find( (b) => b.textContent === "Deselect All" ) as HTMLButtonElement; @@ -1164,11 +1178,9 @@ describe("RepoSelector — org accordion", () => { fireEvent.click(perOrgDeselectAll); - // onChange should have been called with alpha-org's repo removed expect(onChange).toHaveBeenCalledOnce(); const result = onChange.mock.calls[0][0] as RepoRef[]; expect(result.map((r) => r.fullName)).not.toContain("alpha-org/alpha-org-repo"); - // beta-org's repo must still be selected expect(result.map((r) => r.fullName)).toContain("beta-org/beta-org-repo"); }); @@ -1243,7 +1255,7 @@ describe("RepoSelector — org accordion", () => { } }); - it("expanded org panel does not have inert; collapsed org panels do have inert", async () => { + it("expanded org has aria-expanded=true; collapsed orgs have aria-expanded=false", async () => { render(() => ( { screen.getByText("alpha-org-repo"); }); - // In accordion mode, expanded org is alpha-org (first alphabetically) - // The panel id is on the inner div that also carries the inert attribute - const alphaPanel = document.getElementById("accordion-panel-alpha-org") as HTMLElement; - expect(alphaPanel).not.toBeNull(); - expect(alphaPanel.hasAttribute("inert")).toBe(false); + // alpha-org trigger is expanded + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); - // All other org panels should have inert + // All other org triggers are collapsed for (const org of sixOrgs.filter((o) => o !== "alpha-org")) { - const panel = document.getElementById(`accordion-panel-${org}`) as HTMLElement; - expect(panel).not.toBeNull(); - expect(panel.hasAttribute("inert")).toBe(true); + const btn = screen.getByRole("button", { name: new RegExp(org) }); + expect(btn.getAttribute("aria-expanded")).toBe("false"); } }); @@ -1289,7 +1298,7 @@ describe("RepoSelector — org accordion", () => { screen.getByText("alpha-org-repo"); }); - // Expand the 3rd org (gamma-org) + // Expand gamma-org const gammaBtn = screen.getByRole("button", { name: /gamma-org/ }); fireEvent.click(gammaBtn); expect(gammaBtn.getAttribute("aria-expanded")).toBe("true"); @@ -1301,7 +1310,6 @@ describe("RepoSelector — org accordion", () => { Promise.resolve(makeOrgRepos(org as string)) ); - // Re-render to 7 orgs first setSelectedOrgs(sevenOrgs); setOrgEntries(sevenOrgEntries); @@ -1309,12 +1317,11 @@ describe("RepoSelector — org accordion", () => { screen.getByText("eta-org-repo"); }); - // Expand gamma-org const gammaBtnAgain = screen.getByRole("button", { name: /gamma-org/ }); fireEvent.click(gammaBtnAgain); expect(gammaBtnAgain.getAttribute("aria-expanded")).toBe("true"); - // Now remove gamma-org (still 6 orgs remain → still accordion) + // Remove gamma-org (still 6 orgs remain → still accordion) const remainingOrgs = sevenOrgs.filter((o) => o !== "gamma-org"); const remainingEntries = remainingOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); setSelectedOrgs(remainingOrgs); @@ -1324,8 +1331,161 @@ describe("RepoSelector — org accordion", () => { expect(screen.queryByRole("button", { name: /gamma-org/ })).toBeNull(); }); - // expandedOrg should fall back to states[0] (alpha-org) + // safeExpandedOrg should fall back to first selectedOrg still present const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); expect(alphaBtn.getAttribute("aria-expanded")).toBe("true"); }); + + it("transitions back to flat mode when org count drops from 6 to 5", async () => { + const { createSignal } = await import("solid-js"); + const [selectedOrgs, setSelectedOrgs] = createSignal(sixOrgs); + const [orgEntries, setOrgEntries] = createSignal(sixOrgEntries); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Verify accordion mode is active (aria-expanded attributes present) + expect(document.querySelectorAll("[aria-expanded]").length).toBeGreaterThan(0); + + // Remove one org to drop to 5 — below the >= 6 threshold + const fiveOrgs = sixOrgs.slice(0, 5); + const fiveOrgEntries = fiveOrgs.map((login) => ({ login, avatarUrl: "", type: "org" as const })); + setSelectedOrgs(fiveOrgs); + setOrgEntries(fiveOrgEntries); + + await waitFor(() => { + // Flat mode: no aria-expanded attributes + expect(document.querySelectorAll("[aria-expanded]")).toHaveLength(0); + // All 5 org repos visible + screen.getByText("alpha-org-repo"); + screen.getByText("epsilon-org-repo"); + expect(screen.queryByText("zeta-org-repo")).toBeNull(); + }); + + // Re-add the 6th org — accordion mode should re-activate + setSelectedOrgs(sixOrgs); + setOrgEntries(sixOrgEntries); + + await waitFor(() => { + expect(document.querySelectorAll("[aria-expanded]").length).toBeGreaterThan(0); + }); + }); + + it("shows 'N selected' badge on expanded accordion header when repos are selected", async () => { + const alphaSelected: RepoRef[] = [ + { owner: "alpha-org", name: "alpha-org-repo", fullName: "alpha-org/alpha-org-repo" }, + ]; + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + const alphaBtn = screen.getByRole("button", { name: /alpha-org/ }); + expect(alphaBtn.textContent).toContain("1 selected"); + }); + + it("hides per-org bulk actions and shows loading spinner in accordion header while org loads", async () => { + let resolveZeta!: (repos: RepoEntry[]) => void; + const zetaPending = new Promise((res) => { resolveZeta = res; }); + + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => { + if (org === "zeta-org") return zetaPending; + return Promise.resolve(makeOrgRepos(org as string)); + }); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Expand zeta-org to see its header state + const zetaBtn = screen.getByRole("button", { name: /zeta-org/ }); + fireEvent.click(zetaBtn); + + await waitFor(() => { + // The spinner should be visible in the trigger button + expect(zetaBtn.querySelector(".loading-spinner")).not.toBeNull(); + }); + + // Per-org bulk actions must NOT appear while loading + const headerRow = zetaBtn.closest("[class*='border-b']") ?? zetaBtn.parentElement!.parentElement!; + const actionBtns = Array.from(headerRow.querySelectorAll("button")).filter( + (b) => b !== zetaBtn && (b.textContent === "Select All" || b.textContent === "Deselect All") + ); + expect(actionBtns).toHaveLength(0); + + // Resolve zeta — spinner disappears, bulk actions appear + resolveZeta(makeOrgRepos("zeta-org")); + await waitFor(() => { + screen.getByText("zeta-org-repo"); + }); + }); + + it("expanding a failed accordion org panel shows Retry button and allows re-fetch", async () => { + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => { + if (org === "zeta-org") return Promise.reject(new Error("zeta load failed")); + return Promise.resolve(makeOrgRepos(org as string)); + }); + + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("alpha-org-repo"); + }); + + // Expand zeta-org (which failed) + const zetaBtn = screen.getByRole("button", { name: /zeta-org/ }); + fireEvent.click(zetaBtn); + + // Error message and Retry should be visible + await waitFor(() => { + screen.getByText(/zeta load failed/); + screen.getByText("Retry"); + }); + + // Set up the retry to succeed + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => { + return Promise.resolve(makeOrgRepos(org as string)); + }); + + fireEvent.click(screen.getByText("Retry")); + + await waitFor(() => { + screen.getByText("zeta-org-repo"); + }); + }); }); diff --git a/tests/lib/scroll.test.ts b/tests/lib/scroll.test.ts index d70e0114..dd4584eb 100644 --- a/tests/lib/scroll.test.ts +++ b/tests/lib/scroll.test.ts @@ -4,11 +4,11 @@ import { withScrollLock, withFlipAnimation } from "../../src/app/lib/scroll"; describe("withScrollLock", () => { afterEach(() => { vi.restoreAllMocks(); - document.documentElement.scrollTop = 0; + Object.defineProperty(window, "scrollY", { value: 0, configurable: true }); }); it("restores scroll position after fn() completes", () => { - document.documentElement.scrollTop = 500; + Object.defineProperty(window, "scrollY", { value: 500, configurable: true }); vi.spyOn(window, "scrollTo"); withScrollLock(() => {}); @@ -17,7 +17,7 @@ describe("withScrollLock", () => { }); it("restores scroll position even when fn() throws", () => { - document.documentElement.scrollTop = 300; + Object.defineProperty(window, "scrollY", { value: 300, configurable: true }); vi.spyOn(window, "scrollTo"); expect(() => @@ -31,11 +31,11 @@ describe("withScrollLock", () => { describe("withFlipAnimation", () => { afterEach(() => { vi.restoreAllMocks(); - document.documentElement.scrollTop = 0; + Object.defineProperty(window, "scrollY", { value: 0, configurable: true }); }); it("falls back to withScrollLock when prefers-reduced-motion is set", () => { - document.documentElement.scrollTop = 400; + Object.defineProperty(window, "scrollY", { value: 400, configurable: true }); vi.spyOn(window, "scrollTo"); vi.spyOn(window, "matchMedia").mockReturnValue({ matches: true } as MediaQueryList); @@ -55,4 +55,105 @@ describe("withFlipAnimation", () => { // No data-repo-group elements → no deltas → no scrollTo (FLIP is a no-op) expect(window.scrollTo).not.toHaveBeenCalled(); }); + + it("reads all rects before scrollTo (no layout thrash), then animates", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList); + + // Track call order to verify reads-before-writes + const callOrder: string[] = []; + vi.spyOn(window, "scrollTo").mockImplementation(() => { callOrder.push("scrollTo"); }); + + let rafCallback: FrameRequestCallback | null = null; + vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { + rafCallback = cb; + return 0; + }); + + const elA = document.createElement("div"); + elA.dataset.repoGroup = "org/repo-a"; + const elB = document.createElement("div"); + elB.dataset.repoGroup = "org/repo-b"; + document.body.appendChild(elA); + document.body.appendChild(elB); + + const beforeRectA = { top: 100, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + const beforeRectB = { top: 200, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + const afterRectA = { top: 200, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + const afterRectB = { top: 100, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + + vi.spyOn(elA, "getBoundingClientRect").mockImplementation(() => { + callOrder.push("rectA"); + // First call returns before, second returns after + return callOrder.filter(c => c === "rectA").length <= 1 ? beforeRectA : afterRectA; + }); + vi.spyOn(elB, "getBoundingClientRect").mockImplementation(() => { + callOrder.push("rectB"); + return callOrder.filter(c => c === "rectB").length <= 1 ? beforeRectB : afterRectB; + }); + + const animateA = vi.fn().mockReturnValue({ finished: Promise.resolve() }); + const animateB = vi.fn().mockReturnValue({ finished: Promise.resolve() }); + elA.animate = animateA; + elB.animate = animateB; + + // scrollY is 300, no drift (stays 300 in rAF since we mock it) + Object.defineProperty(window, "scrollY", { value: 300, configurable: true }); + + withFlipAnimation(() => {}); + + expect(rafCallback).not.toBeNull(); + expect(animateA).not.toHaveBeenCalled(); + + rafCallback!(0); + + // Verify reads-before-writes: all getBoundingClientRect calls happen before scrollTo + const scrollToIdx = callOrder.indexOf("scrollTo"); + const lastRectIdx = Math.max( + callOrder.lastIndexOf("rectA"), + callOrder.lastIndexOf("rectB"), + ); + expect(lastRectIdx).toBeLessThan(scrollToIdx); + + expect(window.scrollTo).toHaveBeenCalledWith(0, 300); + + // scrollDrift is 0 (scrollY unchanged), so dy = old.top - now.top - 0 + expect(animateA).toHaveBeenCalledWith( + [{ transform: "translateY(-100px)" }, { transform: "translateY(0)" }], + { duration: 200, easing: "ease-in-out" }, + ); + expect(animateB).toHaveBeenCalledWith( + [{ transform: "translateY(100px)" }, { transform: "translateY(0)" }], + { duration: 200, easing: "ease-in-out" }, + ); + + document.body.removeChild(elA); + document.body.removeChild(elB); + }); + + it("skips elements whose position delta is less than 1px", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList); + + let rafCallback: FrameRequestCallback | null = null; + vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { + rafCallback = cb; + return 0; + }); + + const el = document.createElement("div"); + el.dataset.repoGroup = "org/repo-stable"; + document.body.appendChild(el); + + const stableRect = { top: 50, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + vi.spyOn(el, "getBoundingClientRect").mockReturnValue(stableRect); + + const animateSpy = vi.fn(); + el.animate = animateSpy; + + withFlipAnimation(() => {}); + rafCallback!(0); + + expect(animateSpy).not.toHaveBeenCalled(); + + document.body.removeChild(el); + }); });