| author | |
| committer | |
| log | 68607c5a99147710b92228e0642f6c0a642e32a6 |
| tree | 717af660714f9a3b4cb63081ca1847293d1f6fbe |
| parent | f22331240ed1a97fae560e314bffb75fe39db8ca |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 194 insertions(+), 155 deletions(-)
apps/web/src/components/ChatView.logic.test.ts+1-130| ... | ... | @@ -12,7 +12,7 @@ import { |
| 12 | 12 | TurnId, |
| 13 | 13 | type WorktreeSetupSnapshot, |
| 14 | 14 | } from "@t3tools/contracts"; |
| 15 | import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; | |
| 15 | import { afterEach, describe, expect, it, vi } from "vite-plus/test"; | |
| 16 | 16 | import { Atom, AsyncResult } from "effect/unstable/reactivity"; |
| 17 | 17 | import { appAtomRegistry } from "../rpc/atomRegistry"; |
| 18 | 18 | import { environmentThreadDetails } from "../state/threads"; |
| ... | ... | @@ -86,7 +86,6 @@ import { |
| 86 | 86 | shouldShowBranchMismatchBanner, |
| 87 | 87 | shouldShowPlanFollowUpPrompt, |
| 88 | 88 | shouldWriteThreadErrorToCurrentServerThread, |
| 89 | toolGroupConsumesUpwardNavigation, | |
| 90 | 89 | waitForRevertedMessage, |
| 91 | 90 | prepareRevertedMessageAttachments, |
| 92 | 91 | } from "./ChatView.logic"; |
| ... | ... | @@ -446,134 +445,6 @@ describe("proactive panels", () => { |
| 446 | 445 | }); |
| 447 | 446 | }); |
| 448 | 447 | |
| 449 | describe("toolGroupConsumesUpwardNavigation", () => { | |
| 450 | class ScrollElement extends EventTarget { | |
| 451 | scrollTop = 0; | |
| 452 | scrollHeight = 100; | |
| 453 | clientHeight = 100; | |
| 454 | overflowY = "visible"; | |
| 455 | ||
| 456 | constructor( | |
| 457 | readonly parentElement: ScrollElement | null = null, | |
| 458 | readonly isToolGroup = false, | |
| 459 | ) { | |
| 460 | super(); | |
| 461 | } | |
| 462 | ||
| 463 | closest(selector: string): ScrollElement | null { | |
| 464 | if (selector !== "[data-tool-group-scroll]") return null; | |
| 465 | return this.isToolGroup ? this : (this.parentElement?.closest(selector) ?? null); | |
| 466 | } | |
| 467 | } | |
| 468 | ||
| 469 | beforeEach(() => { | |
| 470 | vi.stubGlobal("Element", ScrollElement); | |
| 471 | vi.stubGlobal("getComputedStyle", (element: ScrollElement) => ({ | |
| 472 | overflowY: element.overflowY, | |
| 473 | })); | |
| 474 | }); | |
| 475 | afterEach(() => vi.unstubAllGlobals()); | |
| 476 | ||
| 477 | it("releases upward navigation when an overflowing group is at the top", () => { | |
| 478 | const group = Object.assign(new ScrollElement(null, true), { | |
| 479 | overflowY: "auto", | |
| 480 | scrollHeight: 300, | |
| 481 | }); | |
| 482 | ||
| 483 | expect(toolGroupConsumesUpwardNavigation(new ScrollElement(group))).toBe(false); | |
| 484 | }); | |
| 485 | ||
| 486 | it.each([ | |
| 487 | { overflowY: "auto", scrollTop: 1 }, | |
| 488 | { overflowY: "auto", scrollTop: 0.25 }, | |
| 489 | { overflowY: "scroll", scrollTop: 80 }, | |
| 490 | ])("consumes upward navigation within a scrolled group: %j", (scroll) => { | |
| 491 | const group = Object.assign(new ScrollElement(null, true), { | |
| 492 | scrollHeight: 300, | |
| 493 | ...scroll, | |
| 494 | }); | |
| 495 | ||
| 496 | expect(toolGroupConsumesUpwardNavigation(group)).toBe(true); | |
| 497 | }); | |
| 498 | ||
| 499 | it.each([100, 300])( | |
| 500 | "consumes scrolling in a nested result with a group content height of %i", | |
| 501 | (scrollHeight) => { | |
| 502 | const group = Object.assign(new ScrollElement(null, true), { | |
| 503 | overflowY: "auto", | |
| 504 | scrollHeight, | |
| 505 | }); | |
| 506 | const result = Object.assign(new ScrollElement(group), { | |
| 507 | overflowY: "auto", | |
| 508 | scrollHeight: 300, | |
| 509 | scrollTop: 0.25, | |
| 510 | }); | |
| 511 | ||
| 512 | expect(toolGroupConsumesUpwardNavigation(new ScrollElement(result))).toBe(true); | |
| 513 | }, | |
| 514 | ); | |
| 515 | ||
| 516 | it("releases upward navigation when the group and nested result are both at the top", () => { | |
| 517 | const group = Object.assign(new ScrollElement(null, true), { | |
| 518 | overflowY: "auto", | |
| 519 | scrollHeight: 300, | |
| 520 | }); | |
| 521 | const result = Object.assign(new ScrollElement(group), { | |
| 522 | overflowY: "scroll", | |
| 523 | scrollHeight: 300, | |
| 524 | }); | |
| 525 | ||
| 526 | expect(toolGroupConsumesUpwardNavigation(new ScrollElement(result))).toBe(false); | |
| 527 | }); | |
| 528 | ||
| 529 | it("ignores targets outside a tool group and non-element targets", () => { | |
| 530 | const outside = Object.assign(new ScrollElement(), { | |
| 531 | overflowY: "auto", | |
| 532 | scrollHeight: 300, | |
| 533 | scrollTop: 40, | |
| 534 | }); | |
| 535 | ||
| 536 | expect(toolGroupConsumesUpwardNavigation(outside)).toBe(false); | |
| 537 | expect(toolGroupConsumesUpwardNavigation(new EventTarget())).toBe(false); | |
| 538 | expect(toolGroupConsumesUpwardNavigation(null)).toBe(false); | |
| 539 | }); | |
| 540 | ||
| 541 | it("does not consume scrolling from an ancestor beyond the tool group", () => { | |
| 542 | const timeline = Object.assign(new ScrollElement(), { | |
| 543 | overflowY: "auto", | |
| 544 | scrollHeight: 300, | |
| 545 | scrollTop: 40, | |
| 546 | }); | |
| 547 | const group = new ScrollElement(timeline, true); | |
| 548 | ||
| 549 | expect(toolGroupConsumesUpwardNavigation(new ScrollElement(group))).toBe(false); | |
| 550 | }); | |
| 551 | ||
| 552 | it.each(["hidden", "clip", "visible"])( | |
| 553 | "ignores a non-scrollable child with overflow-y %s", | |
| 554 | (overflowY) => { | |
| 555 | const group = new ScrollElement(null, true); | |
| 556 | const result = Object.assign(new ScrollElement(group), { | |
| 557 | overflowY, | |
| 558 | scrollHeight: 300, | |
| 559 | scrollTop: 40, | |
| 560 | }); | |
| 561 | ||
| 562 | expect(toolGroupConsumesUpwardNavigation(new ScrollElement(result))).toBe(false); | |
| 563 | }, | |
| 564 | ); | |
| 565 | ||
| 566 | it("does not consume programmatic scrolling on an overflow-hidden group", () => { | |
| 567 | const group = Object.assign(new ScrollElement(null, true), { | |
| 568 | overflowY: "hidden", | |
| 569 | scrollHeight: 300, | |
| 570 | scrollTop: 40, | |
| 571 | }); | |
| 572 | ||
| 573 | expect(toolGroupConsumesUpwardNavigation(group)).toBe(false); | |
| 574 | }); | |
| 575 | }); | |
| 576 | ||
| 577 | 448 | const environmentId = EnvironmentId.make("environment-local"); |
| 578 | 449 | const projectId = ProjectId.make("project-1"); |
| 579 | 450 | const threadId = ThreadId.make("thread-1"); |
apps/web/src/components/ChatView.logic.ts-16| ... | ... | @@ -239,22 +239,6 @@ export function shouldReleaseTimelineAnchorForToolActivity(input: { |
| 239 | 239 | }); |
| 240 | 240 | } |
| 241 | 241 | |
| 242 | export function toolGroupConsumesUpwardNavigation(target: EventTarget | null): boolean { | |
| 243 | const elementTarget = target instanceof Element ? target : null; | |
| 244 | const group = elementTarget?.closest<HTMLElement>("[data-tool-group-scroll]"); | |
| 245 | if (!group) return false; | |
| 246 | ||
| 247 | // A nested result or the group itself can consume an upward scroll. | |
| 248 | for (let element = elementTarget; element; element = element.parentElement) { | |
| 249 | if (element.scrollTop > 0) { | |
| 250 | const overflowY = getComputedStyle(element).overflowY; | |
| 251 | if (overflowY === "auto" || overflowY === "scroll") return true; | |
| 252 | } | |
| 253 | if (element === group) break; | |
| 254 | } | |
| 255 | return false; | |
| 256 | } | |
| 257 | ||
| 258 | 242 | export { |
| 259 | 243 | findRecordedWorktreeSetup, |
| 260 | 244 | resolveVisibleWorktreeSetup, |
apps/web/src/components/ChatView.tsx+13-7| ... | ... | @@ -365,6 +365,7 @@ import { |
| 365 | 365 | import { environmentShell } from "../state/shell"; |
| 366 | 366 | import { ChatComposer, type ChatComposerHandle } from "./chat/ChatComposer"; |
| 367 | 367 | import { createPageScrollController, type PageScrollKey } from "./chat/pageScrollController"; |
| 368 | import { isTimelineScrollTarget } from "./chat/timelineScrollTarget"; | |
| 368 | 369 | import { DraftHeroHeadline } from "./chat/DraftHeroHeadline"; |
| 369 | 370 | import { ExpandedImageDialog } from "./chat/ExpandedImageDialog"; |
| 370 | 371 | import { PullRequestThreadDialog } from "./PullRequestThreadDialog"; |
| ... | ... | @@ -474,7 +475,6 @@ import { |
| 474 | 475 | shouldWriteThreadErrorToCurrentServerThread, |
| 475 | 476 | startNewThreadForProject, |
| 476 | 477 | codexArtifactTemplatePromptToAppend, |
| 477 | toolGroupConsumesUpwardNavigation, | |
| 478 | 478 | waitForStartedServerThread, |
| 479 | 479 | shouldRefocusComposerOnWindowFocus, |
| 480 | 480 | } from "./ChatView.logic"; |
| ... | ... | @@ -5466,6 +5466,8 @@ export default function ChatView(props: ChatViewProps) { |
| 5466 | 5466 | // Only an upward wheel is a navigation intent; wheeling down while |
| 5467 | 5467 | // following either does nothing (at the end) or moves toward it. |
| 5468 | 5468 | const handleWheel = (event: WheelEvent) => { |
| 5469 | if (event.ctrlKey || !isTimelineScrollTarget(event.target, scrollNode, event.deltaY)) | |
| 5470 | return; | |
| 5469 | 5471 | if (event.deltaY > 0) { |
| 5470 | 5472 | timelineScrollIntentRef.current = "toward-end"; |
| 5471 | 5473 | if (isAtEndRef.current) { |
| ... | ... | @@ -5474,11 +5476,7 @@ export default function ChatView(props: ChatViewProps) { |
| 5474 | 5476 | } else if (event.deltaY < 0) { |
| 5475 | 5477 | timelineScrollIntentRef.current = "away-from-end"; |
| 5476 | 5478 | } |
| 5477 | if ( | |
| 5478 | event.deltaY < 0 && | |
| 5479 | contentScrollsUp() && | |
| 5480 | !toolGroupConsumesUpwardNavigation(event.target) | |
| 5481 | ) { | |
| 5479 | if (event.deltaY < 0 && contentScrollsUp()) { | |
| 5482 | 5480 | handleManualNavigation(); |
| 5483 | 5481 | } |
| 5484 | 5482 | }; |
| ... | ... | @@ -5528,12 +5526,20 @@ export default function ChatView(props: ChatViewProps) { |
| 5528 | 5526 | ) { |
| 5529 | 5527 | return; |
| 5530 | 5528 | } |
| 5529 | if (!["PageUp", "Home", "ArrowUp", "PageDown", "End", "ArrowDown"].includes(event.key)) | |
| 5530 | return; | |
| 5531 | const scrollDirection = ["PageUp", "Home", "ArrowUp"].includes(event.key) ? -1 : 1; | |
| 5532 | if ( | |
| 5533 | scrollNode.contains(event.target) && | |
| 5534 | !isTimelineScrollTarget(event.target, scrollNode, scrollDirection) | |
| 5535 | ) | |
| 5536 | return; | |
| 5531 | 5537 | switch (event.key) { |
| 5532 | 5538 | case "PageUp": |
| 5533 | 5539 | case "Home": |
| 5534 | 5540 | case "ArrowUp": |
| 5535 | 5541 | timelineScrollIntentRef.current = "away-from-end"; |
| 5536 | if (contentScrollsUp() && !toolGroupConsumesUpwardNavigation(event.target)) { | |
| 5542 | if (contentScrollsUp()) { | |
| 5537 | 5543 | handleManualNavigation(); |
| 5538 | 5544 | composerRef.current?.collapseForTimelineScrollKey(event.key); |
| 5539 | 5545 | } |
apps/web/src/components/chat/ChatComposer.tsx+7-2| ... | ... | @@ -310,6 +310,7 @@ import { |
| 310 | 310 | import { ComposerPromptLengthValidation } from "./ComposerPromptLengthValidation"; |
| 311 | 311 | import { PierreEntryIcon } from "./PierreEntryIcon"; |
| 312 | 312 | import { pendingDraftWork } from "./pendingDraftWork"; |
| 313 | import { isTimelineScrollTarget } from "./timelineScrollTarget"; | |
| 313 | 314 | import { |
| 314 | 315 | createComposerScrollGestureState, |
| 315 | 316 | recordComposerScrollGestureEvent, |
| ... | ... | @@ -4894,8 +4895,12 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) |
| 4894 | 4895 | |
| 4895 | 4896 | const scrollNode = getTimelineScrollableNode(); |
| 4896 | 4897 | if (!scrollNode) return; |
| 4897 | const targetsTimeline = scrollNode.contains(event.target); | |
| 4898 | if (!targetsTimeline && !composerScrollGestureRef.current.collapseSuppressed) return; | |
| 4898 | const targetsTimeline = isTimelineScrollTarget(event.target, scrollNode, event.deltaY); | |
| 4899 | if ( | |
| 4900 | !scrollNode.contains(event.target) && | |
| 4901 | !composerScrollGestureRef.current.collapseSuppressed | |
| 4902 | ) | |
| 4903 | return; | |
| 4899 | 4904 | |
| 4900 | 4905 | if (composerScrollCollapseTimeoutRef.current !== null) { |
| 4901 | 4906 | window.clearTimeout(composerScrollCollapseTimeoutRef.current); |
apps/web/src/components/chat/timelineScrollTarget.test.ts created+142| ... | ... | @@ -0,0 +1,142 @@ |
| 1 | import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; | |
| 2 | ||
| 3 | import { | |
| 4 | createComposerScrollGestureState, | |
| 5 | recordComposerScrollGestureEvent, | |
| 6 | } from "./composerScrollGesture"; | |
| 7 | import { isTimelineScrollTarget } from "./timelineScrollTarget"; | |
| 8 | ||
| 9 | class ScrollElement extends EventTarget { | |
| 10 | scrollTop = 0; | |
| 11 | scrollHeight = 100; | |
| 12 | clientHeight = 100; | |
| 13 | overflowY = "visible"; | |
| 14 | overscrollBehaviorY = "auto"; | |
| 15 | ||
| 16 | constructor(readonly parentElement: ScrollElement | null = null) { | |
| 17 | super(); | |
| 18 | } | |
| 19 | ||
| 20 | contains(target: ScrollElement): boolean { | |
| 21 | return ( | |
| 22 | target === this || (target.parentElement !== null && this.contains(target.parentElement)) | |
| 23 | ); | |
| 24 | } | |
| 25 | } | |
| 26 | ||
| 27 | function targetsTimeline(target: EventTarget | null, timeline: ScrollElement, deltaY: number) { | |
| 28 | return isTimelineScrollTarget(target, timeline as unknown as HTMLElement, deltaY); | |
| 29 | } | |
| 30 | ||
| 31 | function setup() { | |
| 32 | const timeline = Object.assign(new ScrollElement(), { | |
| 33 | overflowY: "auto", | |
| 34 | scrollHeight: 1500, | |
| 35 | clientHeight: 500, | |
| 36 | scrollTop: 1000, | |
| 37 | }); | |
| 38 | const group = Object.assign(new ScrollElement(timeline), { | |
| 39 | overflowY: "auto", | |
| 40 | scrollHeight: 300, | |
| 41 | scrollTop: 80, | |
| 42 | }); | |
| 43 | return { timeline, group, content: new ScrollElement(group) }; | |
| 44 | } | |
| 45 | ||
| 46 | beforeEach(() => { | |
| 47 | vi.stubGlobal("Element", ScrollElement); | |
| 48 | vi.stubGlobal("getComputedStyle", (element: ScrollElement) => element); | |
| 49 | }); | |
| 50 | afterEach(() => vi.unstubAllGlobals()); | |
| 51 | ||
| 52 | describe("timeline scroll targets", () => { | |
| 53 | it.each([-30, 30])("keeps a nested tool group's scroll out of the timeline: %i", (deltaY) => { | |
| 54 | const { timeline, group, content } = setup(); | |
| 55 | expect(targetsTimeline(content, timeline, deltaY)).toBe(false); | |
| 56 | expect(targetsTimeline(group, timeline, deltaY)).toBe(false); | |
| 57 | }); | |
| 58 | ||
| 59 | it.each([ | |
| 60 | { scrollTop: 0, deltaY: -30 }, | |
| 61 | { scrollTop: 200, deltaY: 30 }, | |
| 62 | ])("allows chaining only past the matching edge: %j", ({ scrollTop, deltaY }) => { | |
| 63 | const { timeline, group, content } = setup(); | |
| 64 | group.scrollTop = scrollTop; | |
| 65 | expect(targetsTimeline(content, timeline, deltaY)).toBe(true); | |
| 66 | expect(targetsTimeline(content, timeline, -deltaY)).toBe(false); | |
| 67 | }); | |
| 68 | ||
| 69 | it.each(["contain", "none"])("respects overscroll-y %s at either edge", (overscrollBehaviorY) => { | |
| 70 | const { timeline, group, content } = setup(); | |
| 71 | group.overscrollBehaviorY = overscrollBehaviorY; | |
| 72 | group.scrollTop = 0; | |
| 73 | expect(targetsTimeline(content, timeline, -30)).toBe(false); | |
| 74 | group.scrollTop = 200; | |
| 75 | expect(targetsTimeline(content, timeline, 30)).toBe(false); | |
| 76 | group.scrollTop = 0; | |
| 77 | group.scrollHeight = group.clientHeight; | |
| 78 | expect(targetsTimeline(content, timeline, 30)).toBe(false); | |
| 79 | }); | |
| 80 | ||
| 81 | it("checks nested results even when the tool group cannot scroll", () => { | |
| 82 | const { timeline, group } = setup(); | |
| 83 | group.scrollTop = 0; | |
| 84 | group.scrollHeight = group.clientHeight; | |
| 85 | const result = Object.assign(new ScrollElement(group), { | |
| 86 | overflowY: "scroll", | |
| 87 | scrollHeight: 300, | |
| 88 | scrollTop: 0.25, | |
| 89 | }); | |
| 90 | expect(targetsTimeline(new ScrollElement(result), timeline, -30)).toBe(false); | |
| 91 | result.scrollTop = 0; | |
| 92 | expect(targetsTimeline(result, timeline, -30)).toBe(true); | |
| 93 | }); | |
| 94 | ||
| 95 | it("checks an outer group when an inner result reaches its edge", () => { | |
| 96 | const { timeline, group } = setup(); | |
| 97 | const result = Object.assign(new ScrollElement(group), { overflowY: "auto" }); | |
| 98 | expect(targetsTimeline(result, timeline, -30)).toBe(false); | |
| 99 | group.scrollTop = 0; | |
| 100 | expect(targetsTimeline(result, timeline, -30)).toBe(true); | |
| 101 | }); | |
| 102 | ||
| 103 | it.each(["visible", "hidden", "clip"])("ignores overflow-y %s", (overflowY) => { | |
| 104 | const { timeline, group, content } = setup(); | |
| 105 | group.overflowY = overflowY; | |
| 106 | expect(targetsTimeline(content, timeline, -30)).toBe(true); | |
| 107 | }); | |
| 108 | ||
| 109 | it("allows ordinary message content and the outer viewport", () => { | |
| 110 | const { timeline } = setup(); | |
| 111 | expect(targetsTimeline(new ScrollElement(timeline), timeline, -30)).toBe(true); | |
| 112 | expect(targetsTimeline(timeline, timeline, 30)).toBe(true); | |
| 113 | }); | |
| 114 | ||
| 115 | it("rejects outside targets, non-elements, and horizontal-only scrolling", () => { | |
| 116 | const { timeline, content } = setup(); | |
| 117 | expect(targetsTimeline(new ScrollElement(), timeline, -30)).toBe(false); | |
| 118 | expect(targetsTimeline(new EventTarget(), timeline, -30)).toBe(false); | |
| 119 | expect(targetsTimeline(null, timeline, -30)).toBe(false); | |
| 120 | expect(targetsTimeline(content, timeline, 0)).toBe(false); | |
| 121 | }); | |
| 122 | ||
| 123 | it("does not accumulate nested scrolling toward composer collapse", () => { | |
| 124 | const { timeline, group, content } = setup(); | |
| 125 | const state = createComposerScrollGestureState(); | |
| 126 | const record = (target: ScrollElement, now: number, deltaPx: number) => | |
| 127 | recordComposerScrollGestureEvent(state, { | |
| 128 | now, | |
| 129 | deltaPx, | |
| 130 | collapseThresholdPx: 24, | |
| 131 | collapseEligible: targetsTimeline(target, timeline, -deltaPx), | |
| 132 | canScrollInGestureDirection: timeline.scrollTop > 0, | |
| 133 | scrollsTowardLogicalEnd: false, | |
| 134 | }); | |
| 135 | ||
| 136 | expect(record(timeline, 0, 20)).toBe(false); | |
| 137 | expect(record(content, 20, 30)).toBe(false); | |
| 138 | group.scrollTop = 0; | |
| 139 | expect(record(content, 40, 10)).toBe(false); | |
| 140 | expect(record(content, 60, 14)).toBe(true); | |
| 141 | }); | |
| 142 | }); |
apps/web/src/components/chat/timelineScrollTarget.ts created+31| ... | ... | @@ -0,0 +1,31 @@ |
| 1 | // A gesture inside the timeline may belong to a nested tool result or code | |
| 2 | // block. Only treat it as timeline navigation if it can chain to the outer list. | |
| 3 | export function isTimelineScrollTarget( | |
| 4 | target: EventTarget | null, | |
| 5 | timeline: HTMLElement, | |
| 6 | deltaY: number, | |
| 7 | ): boolean { | |
| 8 | if (!(target instanceof Element) || !timeline.contains(target) || deltaY === 0) return false; | |
| 9 | ||
| 10 | for ( | |
| 11 | let element: Element | null = target; | |
| 12 | element && element !== timeline; | |
| 13 | element = element.parentElement | |
| 14 | ) { | |
| 15 | const style = getComputedStyle(element); | |
| 16 | if (style.overflowY !== "auto" && style.overflowY !== "scroll") continue; | |
| 17 | ||
| 18 | const canScroll = | |
| 19 | deltaY < 0 | |
| 20 | ? element.scrollTop > 0 | |
| 21 | : element.scrollTop < element.scrollHeight - element.clientHeight; | |
| 22 | if ( | |
| 23 | canScroll || | |
| 24 | style.overscrollBehaviorY === "contain" || | |
| 25 | style.overscrollBehaviorY === "none" | |
| 26 | ) { | |
| 27 | return false; | |
| 28 | } | |
| 29 | } | |
| 30 | return true; | |
| 31 | } |