diff --git a/apps/web/src/components/HistorySwipeNavigation.browser.test.tsx b/apps/web/src/components/HistorySwipeNavigation.browser.test.tsx index 10bf817b7363f..176ddc1c203ff 100644 --- a/apps/web/src/components/HistorySwipeNavigation.browser.test.tsx +++ b/apps/web/src/components/HistorySwipeNavigation.browser.test.tsx @@ -1,74 +1,129 @@ import { describe, expect, it, vi } from "vite-plus/test"; -import { render } from "../../test/render"; +import { render, stubGlobal } from "../../test/render"; import { HistorySwipeNavigation } from "./HistorySwipeNavigation"; -const navigateHistory = vi.hoisted(() => vi.fn()); -vi.mock("../navigationHistory", () => ({ canNavigateHistory: () => true, navigateHistory })); +type Scroller = "shadow" | "none" | "shadow-dialog"; +type Step = readonly [ + event: number | "begin" | "end" | "show-scroller" | "gain-overflow", + after?: "remove" | "close-dialog", +]; -function swipe(target: Element, deltaX: number) { - for (const delta of [deltaX, 0]) { - target.dispatchEvent(new WheelEvent("wheel", { deltaX: delta, bubbles: true, composed: true })); - } +function mountPanel(container: HTMLElement, scroller: Scroller, scrollLeft: number) { + const panel = container.appendChild(document.createElement("div")); + const root = scroller.startsWith("shadow") ? panel.attachShadow({ mode: "open" }) : panel; + const parent = + scroller === "shadow-dialog" + ? root.appendChild(Object.assign(document.createElement("div"), { role: "dialog" })) + : root; + const overflows = scroller === "shadow"; + const viewport = parent.appendChild(document.createElement("div")); + viewport.style.cssText = `width: 200px; overflow-x: ${overflows ? "auto" : "visible"}`; + const content = viewport.appendChild(document.createElement("div")); + content.style.cssText = `height: 20px; width: ${overflows ? 1000 : 100}px`; + viewport.scrollLeft = scrollLeft; + return { viewport, content }; } describe("HistorySwipeNavigation", () => { - it.each([ - ["scrolling left inside a shadow-root scroller keeps the thread", "shadow", 100, -130, false], - ["scrolling right inside a shadow-root scroller keeps the thread", "shadow", 100, 130, false], + it.each<[string, Scroller, number, readonly Step[], "back" | "forward" | null]>([ + ["scrolling left inside a shadow-root scroller", "shadow", 400, [[-130]], null], + ["swiping left at a shadow-root scroller's left edge", "shadow", 0, [[-130]], null], + ["a scroller removed after a small first event", "shadow", 400, [[1, "remove"], [130]], null], [ - "scrolling after a shadow scroller remount keeps the thread", - "shadow-remount", - 100, - 130, - false, + "a history gesture after a content scroll ends", + "shadow", + 400, + [[1, "remove"], ["end"], ["begin"], [130]], + "forward", ], - ["scrolling left inside a scroller keeps the thread", "light", 100, -130, false], - ["swiping back at a shadow-root scroller's left edge navigates", "shadow", 0, -130, true], - ["swiping back over a panel without overflow navigates", "none", 0, -130, true], - ["a swipe that started in a closed dialog keeps the thread", "closed-dialog", 0, -130, false], - ] as const)("%s", async (_name, scroller, scrollLeft, deltaX, navigates) => { - navigateHistory.mockReset(); + [ + "content appearing under tracking that a trailing history wheel started", + "none", + 0, + [["begin"], [130], ["end"], [18], ["show-scroller"], ["begin"], [7], [130], ["end"]], + "forward", + ], + [ + "a gesture that starts in a dialog before it closes", + "shadow-dialog", + 0, + [[-1, "close-dialog"], [-130]], + null, + ], + [ + "a history swipe over content replaced without overflow, released over a scroller", + "none", + 0, + [[-60, "remove"], [-70], ["show-scroller"]], + "back", + ], + [ + "a history swipe whose target gains overflow", + "none", + 0, + [[-60], ["gain-overflow"], [-70]], + "back", + ], + ])("%s", async (_name, scroller, scrollLeft, steps, navigates) => { using _platform = vi.spyOn(navigator, "platform", "get").mockReturnValue("MacIntel"); + const listeners = { begin: new Set<() => void>(), end: new Set<() => void>() }; + using _bridge = stubGlobal("desktopBridge", { + onTrackpadScrollBegin: (listener: () => void) => { + listeners.begin.add(listener); + return () => listeners.begin.delete(listener); + }, + onTrackpadScrollEnd: (listener: () => void) => { + listeners.end.add(listener); + return () => listeners.end.delete(listener); + }, + }); await using screen = await render(); - const panel = screen.container.appendChild(document.createElement("div")); - const root = scroller.startsWith("shadow") ? panel.attachShadow({ mode: "open" }) : panel; - const viewport = root.appendChild(document.createElement("div")); - const noOverflow = scroller === "none" || scroller === "closed-dialog"; - viewport.style.cssText = `width: 200px; overflow-x: ${noOverflow ? "visible" : "auto"}`; - const content = viewport.appendChild(document.createElement("div")); - content.style.cssText = `height: 20px; width: ${noOverflow ? 100 : 1000}px`; - viewport.scrollLeft = scrollLeft; + using back = vi.spyOn(window.history, "back").mockImplementation(() => undefined); + using forward = vi.spyOn(window.history, "forward").mockImplementation(() => undefined); + using _canGoBack = vi.spyOn(window.navigation, "canGoBack", "get").mockReturnValue(true); + using _canGoForward = vi.spyOn(window.navigation, "canGoForward", "get").mockReturnValue(true); + let { viewport, content } = mountPanel(screen.container, scroller, scrollLeft); - if (scroller === "closed-dialog") { - panel.setAttribute("role", "dialog"); - content.dispatchEvent(new WheelEvent("wheel", { deltaX: -1, bubbles: true, composed: true })); - panel.remove(); - screen.container.appendChild(viewport); + for (const [event, after] of [...steps, [0] as const]) { + if (event === "begin") { + for (const listener of listeners.begin) listener(); + } else if (event === "end") { + for (const listener of listeners.end) listener(); + } else if (event === "show-scroller") { + ({ viewport, content } = mountPanel(screen.container, "shadow", 400)); + } else if (event === "gain-overflow") { + viewport.style.overflowX = "auto"; + content.style.width = "1000px"; + } else { + content.dispatchEvent( + new WheelEvent("wheel", { deltaX: event, bubbles: true, composed: true }), + ); + } + if (after === "remove") { + viewport.remove(); + ({ viewport, content } = mountPanel(screen.container, "none", 0)); + } else if (after === "close-dialog") { + viewport.parentElement!.remove(); + screen.container.appendChild(viewport); + } } - if (scroller === "shadow-remount") { - content.dispatchEvent(new WheelEvent("wheel", { deltaX: 1, bubbles: true, composed: true })); - const replacement = viewport.cloneNode(true) as HTMLDivElement; - viewport.replaceWith(replacement); - replacement.scrollLeft = scrollLeft; - swipe(replacement, deltaX); - } else { - swipe(content, deltaX); - } - - expect(navigateHistory).toHaveBeenCalledTimes(navigates ? 1 : 0); + expect(back).toHaveBeenCalledTimes(navigates === "back" ? 1 : 0); + expect(forward).toHaveBeenCalledTimes(navigates === "forward" ? 1 : 0); }); it("a horizontal mouse wheel tick off macOS keeps the thread", async () => { - navigateHistory.mockReset(); using _platform = vi.spyOn(navigator, "platform", "get").mockReturnValue("Win32"); await using screen = await render(); - const content = screen.container.appendChild(document.createElement("div")); - content.style.cssText = "height: 20px; width: 100px"; + using back = vi.spyOn(window.history, "back").mockImplementation(() => undefined); + using _canGoBack = vi.spyOn(window.navigation, "canGoBack", "get").mockReturnValue(true); + const { content } = mountPanel(screen.container, "none", 0); - swipe(content, -120); + for (const deltaX of [-120, 0]) { + content.dispatchEvent(new WheelEvent("wheel", { deltaX, bubbles: true, composed: true })); + } - expect(navigateHistory).not.toHaveBeenCalled(); + expect(back).not.toHaveBeenCalled(); }); }); diff --git a/apps/web/src/components/HistorySwipeNavigation.tsx b/apps/web/src/components/HistorySwipeNavigation.tsx index e1adc04491a91..113b36b169109 100644 --- a/apps/web/src/components/HistorySwipeNavigation.tsx +++ b/apps/web/src/components/HistorySwipeNavigation.tsx @@ -19,6 +19,7 @@ type SwipeGesture = deltas: number[]; y: number; bounds: SwipeBounds; + target: EventTarget | undefined; } // Not a swipe, or one already decided: the rest of its events are ignored. | { kind: "ignored" }; @@ -37,30 +38,25 @@ function readMainContentBounds(): SwipeBounds { return { left, top, width, height }; } -// `event.target` stops at a shadow host; the composed path also reaches scrollers inside it. -function canScrollHorizontally(path: readonly EventTarget[], deltaX: number): boolean { - for (const element of path) { - if (!(element instanceof Element) || element.scrollWidth <= element.clientWidth) continue; +// The composed path reaches scrollers inside shadow roots, such as the diff viewer's. +// A scroller at its edge still owns the gesture, so history never takes a content scroll. +function isInHorizontalScroller(path: readonly EventTarget[]): boolean { + return path.some((element) => { + if (!(element instanceof Element) || element.scrollWidth <= element.clientWidth) return false; const { overflowX } = getComputedStyle(element); - if (overflowX !== "auto" && overflowX !== "scroll") continue; - const canScroll = - deltaX < 0 - ? element.scrollLeft > 0 - : element.scrollLeft + element.clientWidth < element.scrollWidth - 1; - if (canScroll) return true; - } - return false; + return overflowX === "auto" || overflowX === "scroll"; + }); } function lockSwipeAxis( pending: Extract, + target: EventTarget | undefined, ): Exclude { const { path, sumX, sumY } = pending; if (Math.abs(sumX) <= Math.abs(sumY)) return { kind: "ignored" }; if (path.some((node) => node instanceof Element && node.matches('[role="dialog"]'))) { return { kind: "ignored" }; } - if (canScrollHorizontally(path, sumX)) return { kind: "ignored" }; const direction = sumX < 0 ? "back" : "forward"; if (!canNavigateHistory(direction)) return { kind: "ignored" }; return { @@ -70,6 +66,7 @@ function lockSwipeAxis( deltas: [], y: pending.y, bounds: readMainContentBounds(), + target, }; } @@ -125,15 +122,29 @@ export function HistorySwipeNavigation() { : { kind: "pending", path: event.composedPath(), y: event.clientY, sumX: 0, sumY: 0 }; } if (gesture.kind === "ignored") return; + if (event.deltaX === 0 && event.deltaY === 0) { + if (gesture.kind === "tracking") release(); + return; + } + const path = event.composedPath(); + // Pending wheels are checked while a scroller is still laid out, before it can be removed. + // Tracking rechecks only a new target, so overflow gained under a swipe stays history. + if ( + (gesture.kind === "pending" || path[0] !== gesture.target) && + isInHorizontalScroller(path) + ) { + gesture = { kind: "ignored" }; + return hideIndicator(); + } if (gesture.kind === "pending") { gesture.sumX += event.deltaX; gesture.sumY += event.deltaY; if (Math.abs(gesture.sumX) + Math.abs(gesture.sumY) < SWIPE_AXIS_LOCK_DISTANCE) return; - gesture = lockSwipeAxis({ ...gesture, path: [...gesture.path, ...event.composedPath()] }); + gesture = lockSwipeAxis({ ...gesture, path: [...gesture.path, ...path] }, path[0]); if (gesture.kind === "ignored") return; } else { + gesture.target = path[0]; const delta = gesture.direction === "back" ? -event.deltaX : event.deltaX; - if (event.deltaX === 0 && event.deltaY === 0) return release(); gesture.deltas = [...gesture.deltas.slice(1 - MOMENTUM_SAMPLE_SIZE), delta]; if (isMomentumTail(gesture.deltas)) return release(); gesture.distance = Math.max(0, gesture.distance + delta); diff --git a/apps/web/src/components/RightPanelTabs.browser.test.tsx b/apps/web/src/components/RightPanelTabs.browser.test.tsx index f803cb41520bc..a5facb2938346 100644 --- a/apps/web/src/components/RightPanelTabs.browser.test.tsx +++ b/apps/web/src/components/RightPanelTabs.browser.test.tsx @@ -64,7 +64,7 @@ const surfaces: RightPanelSurface[] = Array.from({ length: 16 }, (_, index) => ( })); describe("RightPanelTabs", () => { - it("a flick that scrolls the tab strip to its left edge keeps the thread, a swipe from the edge goes back", async () => { + it("a flick that scrolls the tab strip to its left edge and a swipe from that edge keep the thread", async () => { navigateHistory.mockReset(); const { bridge, scroll } = trackpad(); using _bridge = stubGlobal("desktopBridge", bridge); @@ -135,7 +135,7 @@ describe("RightPanelTabs", () => { viewport.scrollLeft = 0; scroll(viewport.querySelector("[data-active-tab]")!, [-40, -40, -40, -40, -40]); - expect(navigateHistory).toHaveBeenCalledExactlyOnceWith("back"); + expect(navigateHistory).not.toHaveBeenCalled(); }); });