diff --git a/apps/web/src/components/timeline/AllocationPopover.test.tsx b/apps/web/src/components/timeline/AllocationPopover.test.tsx index 0be8e38..631728a 100644 --- a/apps/web/src/components/timeline/AllocationPopover.test.tsx +++ b/apps/web/src/components/timeline/AllocationPopover.test.tsx @@ -42,6 +42,14 @@ vi.mock("~/hooks/useViewportPopover.js", () => ({ }), })); +vi.mock("~/components/ui/DateInput.js", () => ({ + DateInput: () => null, +})); + +vi.mock("~/components/ui/ProjectCombobox.js", () => ({ + ProjectCombobox: () => null, +})); + describe("AllocationPopover", () => { beforeEach(() => { mockUseQuery.mockReset(); @@ -93,4 +101,67 @@ describe("AllocationPopover", () => { expect(html).toContain("data-testid=\"timeline-allocation-popover-unavailable\""); expect(html).toContain("The selected booking could not be resolved from the current timeline data."); }); + + it("renders a loading skeleton when the allocation is being fetched", () => { + mockUseQuery.mockReturnValue({ + data: undefined, + isLoading: true, + error: null, + }); + + const html = renderToStaticMarkup( + {}} + onOpenPanel={() => {}} + />, + ); + + expect(html).toContain("data-testid=\"timeline-allocation-popover-loading\""); + }); + + it("renders allocation data when provided as initialAllocation", () => { + // useQuery is always called (with enabled: false), so we need a baseline return value + mockUseQuery.mockReturnValue({ + data: undefined, + isLoading: false, + error: null, + }); + + const allocation = { + id: "assignment_1", + entityId: "assignment_1", + projectId: "project_1", + resourceId: "resource_1", + startDate: new Date("2026-01-01"), + endDate: new Date("2026-01-31"), + hoursPerDay: 8, + role: "Developer", + metadata: null, + resource: { + displayName: "Alice Smith", + eid: "alice.smith", + lcrCents: 0, + }, + }; + + const html = renderToStaticMarkup( + {}} + onOpenPanel={() => {}} + />, + ); + + expect(html).not.toContain("data-testid=\"timeline-allocation-popover-error\""); + expect(html).not.toContain("data-testid=\"timeline-allocation-popover-unavailable\""); + expect(html).not.toContain("data-testid=\"timeline-allocation-popover-loading\""); + }); }); diff --git a/apps/web/src/components/timeline/AllocationPopover.tsx b/apps/web/src/components/timeline/AllocationPopover.tsx index b5324ee..2dcf93d 100644 --- a/apps/web/src/components/timeline/AllocationPopover.tsx +++ b/apps/web/src/components/timeline/AllocationPopover.tsx @@ -1,6 +1,6 @@ "use client"; -import React from "react"; +import React, { type RefObject } from "react"; import { clsx } from "clsx"; import { useEffect, useState } from "react"; import { createPortal } from "react-dom"; @@ -21,6 +21,7 @@ interface AllocationPopoverProps { anchorX: number; anchorY: number; contextDate?: Date; + ignoreScrollContainers?: RefObject[]; } type AllocationPopoverAssignment = Assignment; @@ -34,6 +35,7 @@ export function AllocationPopover({ anchorX, anchorY, contextDate, + ignoreScrollContainers, }: AllocationPopoverProps) { const utils = trpc.useUtils(); const invalidateTimeline = useInvalidateTimeline(); @@ -42,13 +44,14 @@ export function AllocationPopover({ width: 300, estimatedHeight: 360, onClose, + ...(ignoreScrollContainers ? { ignoreScrollContainers } : {}), }); const shouldLoadAllocation = !initialAllocation; const allocationQuery = trpc.allocation.getAssignmentById.useQuery( { id: allocationId }, { - staleTime: 10_000, + staleTime: 0, enabled: shouldLoadAllocation, retry: false, }, diff --git a/apps/web/src/components/timeline/DemandPopover.tsx b/apps/web/src/components/timeline/DemandPopover.tsx index eeed6f5..b656ac4 100644 --- a/apps/web/src/components/timeline/DemandPopover.tsx +++ b/apps/web/src/components/timeline/DemandPopover.tsx @@ -1,5 +1,6 @@ "use client"; +import type { RefObject } from "react"; import { createPortal } from "react-dom"; import type { TimelineDemandEntry } from "./TimelineContext.js"; import { formatCents, formatDateLong } from "~/lib/format.js"; @@ -12,6 +13,7 @@ interface DemandPopoverProps { onFillDemand: (demand: TimelineDemandEntry) => void; anchorX: number; anchorY: number; + ignoreScrollContainers?: RefObject[]; } export function DemandPopover({ @@ -21,12 +23,14 @@ export function DemandPopover({ onFillDemand, anchorX, anchorY, + ignoreScrollContainers, }: DemandPopoverProps) { const { ref, style } = useViewportPopover({ anchor: { kind: "point", x: anchorX, y: anchorY }, width: 300, estimatedHeight: 340, onClose, + ...(ignoreScrollContainers ? { ignoreScrollContainers } : {}), }); const roleName = demand.roleEntity?.name ?? demand.role ?? "Unspecified"; diff --git a/apps/web/src/components/timeline/NewAllocationPopover.tsx b/apps/web/src/components/timeline/NewAllocationPopover.tsx index 729047c..61b629d 100644 --- a/apps/web/src/components/timeline/NewAllocationPopover.tsx +++ b/apps/web/src/components/timeline/NewAllocationPopover.tsx @@ -1,6 +1,7 @@ "use client"; import { clsx } from "clsx"; +import type { RefObject } from "react"; import { useState } from "react"; import { createPortal } from "react-dom"; import { AllocationStatus } from "@capakraken/shared"; @@ -20,6 +21,7 @@ interface NewAllocationPopoverProps { anchorY: number; onClose: () => void; onCreated: () => void; + ignoreScrollContainers?: RefObject[]; } function toDateInput(d: Date): string { @@ -38,6 +40,7 @@ export function NewAllocationPopover({ anchorY, onClose, onCreated, + ignoreScrollContainers, }: NewAllocationPopoverProps) { const { ref, style } = useViewportPopover({ anchor: { kind: "point", x: anchorX - 10, y: anchorY }, @@ -45,6 +48,7 @@ export function NewAllocationPopover({ estimatedHeight: 440, onClose, ignoreSelectors: ["[data-entity-combobox-overlay='true']"], + ...(ignoreScrollContainers ? { ignoreScrollContainers } : {}), }); const invalidateTimeline = useInvalidateTimeline(); diff --git a/apps/web/src/components/timeline/TimelineView.tsx b/apps/web/src/components/timeline/TimelineView.tsx index 2097a2c..fea127b 100644 --- a/apps/web/src/components/timeline/TimelineView.tsx +++ b/apps/web/src/components/timeline/TimelineView.tsx @@ -942,6 +942,7 @@ function TimelineViewContent({ }} anchorX={popover.x} anchorY={popover.y} + ignoreScrollContainers={[scrollContainerRef]} /> ); } @@ -957,6 +958,7 @@ function TimelineViewContent({ }} anchorX={popover.x} anchorY={popover.y} + ignoreScrollContainers={[scrollContainerRef]} {...(popover.contextDate ? { contextDate: popover.contextDate } : {})} /> ); @@ -988,6 +990,7 @@ function TimelineViewContent({ }} anchorX={demandPopover.x} anchorY={demandPopover.y} + ignoreScrollContainers={[scrollContainerRef]} /> )} @@ -1002,6 +1005,7 @@ function TimelineViewContent({ anchorY={newAllocPopover.anchorY} onClose={() => setNewAllocPopover(null)} onCreated={() => setNewAllocPopover(null)} + ignoreScrollContainers={[scrollContainerRef]} /> )} diff --git a/apps/web/src/hooks/useViewportPopover.test.ts b/apps/web/src/hooks/useViewportPopover.test.ts new file mode 100644 index 0000000..f0cce9b --- /dev/null +++ b/apps/web/src/hooks/useViewportPopover.test.ts @@ -0,0 +1,222 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Capture effect callbacks so we can run them manually (same pattern as useTimelineSSE.test.ts) +const effectCleanups: Array<() => void> = []; + +vi.mock("react", async () => { + const actual = await vi.importActual("react"); + return { + ...actual, + useEffect: (callback: () => void | (() => void)) => { + const cleanup = callback(); + if (typeof cleanup === "function") { + effectCleanups.push(cleanup); + } + }, + useRef: (value: T) => ({ current: value }), + useState: (initializer: T | (() => T)) => { + const value = typeof initializer === "function" ? (initializer as () => T)() : initializer; + return [value, vi.fn()] as [T, ReturnType]; + }, + }; +}); + +import { useViewportPopover } from "./useViewportPopover.js"; + +// ---- Minimal window event emitter used in tests ---- + +type AnyHandler = (event: Event) => void; + +class MockWindow { + private readonly listeners = new Map>(); + + addEventListener(type: string, handler: AnyHandler) { + if (!this.listeners.has(type)) { + this.listeners.set(type, new Set()); + } + this.listeners.get(type)!.add(handler); + } + + removeEventListener(type: string, handler: AnyHandler) { + this.listeners.get(type)?.delete(handler); + } + + emit(type: string, event: Partial = {}) { + for (const handler of this.listeners.get(type) ?? []) { + handler(event as Event); + } + } + + readonly innerWidth = 1280; + readonly innerHeight = 800; + readonly visualViewport = null; +} + +// In the node test environment `Node` is not defined. Provide a stub class so that +// `scrollTarget instanceof Node` works as expected in handleScroll. +class MockNode { + // subclasses can override to signal membership +} + +/** + * Creates a pair of mock Node-like objects where `container.contains(inner)` + * returns true but `container.contains(outside)` returns false. + * This satisfies the `r.current.contains(scrollTarget)` check in handleScroll + * without needing a real DOM. + */ +function makeMockNodes() { + const inner = new MockNode() as unknown as Node; + const outside = new MockNode() as unknown as Node; + const container = Object.assign(new MockNode(), { + contains(node: Node | null) { + return node === inner; + }, + }) as unknown as HTMLElement; + return { container, inner, outside }; +} + +describe("useViewportPopover", () => { + let mockWindow: MockWindow; + let onClose: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + effectCleanups.length = 0; + + mockWindow = new MockWindow(); + onClose = vi.fn(); + + // Stub globals needed by useViewportPopover effects + vi.stubGlobal("window", mockWindow); + vi.stubGlobal("document", { + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }); + // `Node` is unavailable in the node test environment; stub it so that + // `scrollTarget instanceof Node` works correctly in handleScroll. + vi.stubGlobal("Node", MockNode); + vi.stubGlobal("ResizeObserver", class { + observe() {} + disconnect() {} + }); + vi.stubGlobal("requestAnimationFrame", (cb: () => void) => { + cb(); + return 1; + }); + vi.stubGlobal("cancelAnimationFrame", () => {}); + }); + + afterEach(() => { + while (effectCleanups.length > 0) { + effectCleanups.pop()?.(); + } + vi.unstubAllGlobals(); + }); + + // Test 1: anchor "point" → onClose called on window scroll + it("calls onClose when window scroll fires for a point anchor", () => { + useViewportPopover({ + anchor: { kind: "point", x: 100, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + }); + + // Fire a scroll event with a non-Node target so the ignore-check is irrelevant + mockWindow.emit("scroll", { target: null }); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + // Test 2: anchor "point" + ignoreScrollContainers → onClose NOT called when scroll target is inside the container + it("does NOT call onClose when scroll target is inside an ignored scroll container", () => { + const { container, inner } = makeMockNodes(); + const containerRef = { current: container }; + + useViewportPopover({ + anchor: { kind: "point", x: 100, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + ignoreScrollContainers: [containerRef], + }); + + // Scroll event whose target is inside the ignored container + mockWindow.emit("scroll", { target: inner }); + + expect(onClose).not.toHaveBeenCalled(); + }); + + // Test 3: anchor "point" + ignoreScrollContainers → onClose IS called when scroll target is outside container + it("calls onClose when scroll target is outside the ignored scroll container", () => { + const { container, outside } = makeMockNodes(); + const containerRef = { current: container }; + + useViewportPopover({ + anchor: { kind: "point", x: 100, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + ignoreScrollContainers: [containerRef], + }); + + // Scroll event whose target is NOT inside the container + mockWindow.emit("scroll", { target: outside }); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + // Test 4: anchor "point" → onClose called on window resize + it("calls onClose when window resize fires for a point anchor", () => { + useViewportPopover({ + anchor: { kind: "point", x: 100, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + }); + + mockWindow.emit("resize", {}); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + // Test 5: initial style for anchor "point" with x=100, y=50 → position fixed, left/top set + it("computes fixed position style with left and top for a point anchor", () => { + const result = useViewportPopover({ + anchor: { kind: "point", x: 100, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + viewportPadding: 16, + }); + + const style = result.style as import("react").CSSProperties; + + expect(style.position).toBe("fixed"); + // anchor.x = 100, align = "start" → raw left = 100 + // clamped: min(max(100, 16), max(16, 1280-300-16)) = min(100, 964) = 100 + expect(style.left).toBe(100); + // anchor.y = 50, offset = 8 → raw top = 58 + // clamped: min(max(58, 16), max(16, 800-200-16)) = min(58, 584) = 58 + expect(style.top).toBe(58); + }); + + // Test 6: bounds clamping — if anchor x + width > viewport width, left is clamped + it("clamps left so the popover stays within viewport bounds", () => { + // anchor at x=1250, width=300, innerWidth=1280, padding=16 + // raw left = 1250, maxLeft = max(16, 1280 - 300 - 16) = 964 → clamped to 964 + const result = useViewportPopover({ + anchor: { kind: "point", x: 1250, y: 50 }, + width: 300, + estimatedHeight: 200, + onClose, + viewportPadding: 16, + }); + + const style = result.style as import("react").CSSProperties; + expect(style.position).toBe("fixed"); + expect(style.left as number).toBeLessThanOrEqual(1280 - 300 - 16); + // Should be clamped to 964 + expect(style.left).toBe(964); + }); +}); diff --git a/apps/web/src/hooks/useViewportPopover.ts b/apps/web/src/hooks/useViewportPopover.ts index 45eeb2a..6636c1d 100644 --- a/apps/web/src/hooks/useViewportPopover.ts +++ b/apps/web/src/hooks/useViewportPopover.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, type CSSProperties } from "react"; +import React, { useEffect, useRef, useState, type CSSProperties } from "react"; type PopoverAnchor = | { kind: "point"; x: number; y: number } @@ -18,6 +18,7 @@ interface UseViewportPopoverOptions { viewportPadding?: number; ignoreElements?: Array; ignoreSelectors?: string[]; + ignoreScrollContainers?: React.RefObject[]; zIndex?: number; } @@ -32,6 +33,7 @@ export function useViewportPopover({ viewportPadding = 16, ignoreElements = [], ignoreSelectors = [], + ignoreScrollContainers, zIndex = 9998, }: UseViewportPopoverOptions) { const ref = useRef(null); @@ -182,8 +184,19 @@ export function useViewportPopover({ updateOrClose(); - const handleScroll = () => { + const handleScroll = (event: Event) => { if (closeOnViewportChange) { + const scrollTarget = (event as Event).target; + if ( + ignoreScrollContainers?.some( + (r) => + r.current != null && + scrollTarget instanceof Node && + r.current.contains(scrollTarget), + ) + ) { + return; + } cancelScheduledFrame(); onClose(); return; @@ -215,6 +228,7 @@ export function useViewportPopover({ align, anchor, estimatedHeight, + ignoreScrollContainers, offset, onClose, side,