diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index 2a2219508..fdc7fbd81 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -34,6 +34,7 @@ When a change has user-facing documentation, include a canonical tasknotes.dev l ## Fixed +- Fixed long Task List views embedded in a note rendering only their first rows until the list itself was scrolled. The visible rows now follow the note's own scrolling, including inside multi-column callouts. - (#1894) Fixed relative date changes (+1/-1 day and week) dropping scheduled and due times from task cards. Date-only tasks remain date-only. See [Task dates](https://tasknotes.dev/features/task-management/#task-dates). - Thanks to @gemx-world for reporting. - (#2140) Fixed edits to visible nested task tags being silently discarded by the task edit modal. Hidden identification tags and the exact task tag remain protected. See [Hide Identification Tags](https://tasknotes.dev/settings/general/#hide-identification-tags). diff --git a/src/utils/VirtualScroller.ts b/src/utils/VirtualScroller.ts index ed3e5c76b..9d93b5636 100644 --- a/src/utils/VirtualScroller.ts +++ b/src/utils/VirtualScroller.ts @@ -161,11 +161,20 @@ export class VirtualScroller { const sampleHeights: number[] = []; // Temporarily render sample items for measurement + // A container with no width yet (a Base embed that has not been laid out) would + // measure every card as one word per line, many times its real height. + if (this.container.getBoundingClientRect().width === 0) { + this.estimatedHeight = 60; + return; + } + const tempContainer = this.contentContainer.createDiv({ cls: "virtual-scroller__sample", }); tempContainer.style.cssText = ` position: absolute; + left: 0; + right: 0; visibility: hidden; pointer-events: none; `; @@ -221,7 +230,13 @@ export class VirtualScroller { } private updateSpacerHeight(): void { - this.spacer.style.height = `${this.totalHeight + this.getContainerBottomPadding()}px`; + const height = `${this.totalHeight + this.getContainerBottomPadding()}px`; + this.spacer.style.height = height; + // The spacer is absolutely positioned, so it does not give the container any + // height. When no ancestor bounds the list (a Base embedded in a note), the + // container must reserve the list's full height itself, or the note cannot + // scroll to the items that are not rendered yet. + this.container.style.minHeight = height; } /** @@ -365,7 +380,9 @@ export class VirtualScroller { let insertAt = currentEntries.length; if (position !== "end") { - const targetIndex = currentEntries.findIndex((entry) => entry.key === options.targetKey); + const targetIndex = currentEntries.findIndex( + (entry) => entry.key === options.targetKey + ); if (targetIndex === -1) { return null; } @@ -471,12 +488,58 @@ export class VirtualScroller { } if (heightsChanged) { + // Items not rendered yet are sized by the estimate, and the container + // reserves their height, so keep it at the average of what was measured. + let measuredTotal = 0; + for (const height of this.itemHeights.values()) { + measuredTotal += height; + } + this.estimatedHeight = Math.ceil(measuredTotal / this.itemHeights.size); this.rebuildPositionCache(); } } private attachScrollListener(): void { - this.scrollContainer.addEventListener("scroll", this.handleScroll); + // Scroll events do not bubble, but a capturing listener on the document sees + // every scroll: the list's own scroll container and the note scrolling a Base + // that is embedded in it. + const doc = this.container.ownerDocument; + doc.addEventListener("scroll", this.handleScroll, { capture: true, passive: true }); + doc.defaultView?.addEventListener("resize", this.handleScroll); + } + + /** + * The part of the list that is on screen, in the container's own coordinates: + * the viewport, clipped by every ancestor that clips its content. Returns null + * when the container has no layout yet (not attached, hidden, or a test DOM). + */ + private getVisibleWindow(): { top: number; bottom: number } | null { + const rect = this.container.getBoundingClientRect(); + if (rect.width === 0 && rect.height === 0) { + return null; + } + + const view = this.container.ownerDocument.defaultView ?? window; + let top = 0; + let bottom = view.innerHeight; + for (let current = this.container.parentElement; current; current = current.parentElement) { + const style = view.getComputedStyle(current); + // An element with `display: contents` has no box and clips nothing, though its + // computed overflow may say otherwise (a multi-column callout's content). + if ( + style.display === "contents" || + !["auto", "scroll", "hidden", "clip"].includes(style.overflowY) + ) { + continue; + } + const clip = current.getBoundingClientRect(); + top = Math.max(top, clip.top); + bottom = Math.min(bottom, clip.bottom); + } + + const visibleTop = Math.min(Math.max(0, top - rect.top), this.totalHeight); + const visibleBottom = Math.min(Math.max(visibleTop, bottom - rect.top), this.totalHeight); + return { top: visibleTop, bottom: visibleBottom }; } private handleScroll = (): void => { @@ -492,15 +555,22 @@ export class VirtualScroller { }; private updateVisibleRange(): void { - const scrollTop = this.scrollContainer.scrollTop; - let containerHeight = this.scrollContainer.clientHeight; + // Measure what is actually on screen. The nearest ancestor with overflow is not + // always the element that scrolls the list: Obsidian gives an embedded Base's + // view `overflow-x: auto`, which computes `overflow-y` to `auto` as well, yet + // that element grows with its content while the note scrolls it. + const visibleWindow = this.getVisibleWindow(); + const scrollTop = visibleWindow ? visibleWindow.top : this.scrollContainer.scrollTop; + let containerHeight = visibleWindow + ? visibleWindow.bottom - visibleWindow.top + : this.scrollContainer.clientHeight; // If container height is 0, try to get a better estimate - if (containerHeight === 0) { + if (!visibleWindow && containerHeight === 0) { // Try parent element height containerHeight = this.scrollContainer.parentElement?.clientHeight || 0; } - if (containerHeight === 0) { + if (!visibleWindow && containerHeight === 0) { // Fall back to window height as last resort containerHeight = window.innerHeight; tasknotesLogger.warn("[VirtualScroller] Using window height as fallback:", { @@ -881,7 +951,9 @@ export class VirtualScroller { this.resizeObserver.disconnect(); this.resizeObserver = null; } - this.scrollContainer.removeEventListener("scroll", this.handleScroll); + const doc = this.container.ownerDocument; + doc.removeEventListener("scroll", this.handleScroll, { capture: true }); + doc.defaultView?.removeEventListener("resize", this.handleScroll); this.renderedElements.clear(); this.contentContainer.empty(); this.itemHeights.clear(); diff --git a/tests/unit/utils/VirtualScroller.test.ts b/tests/unit/utils/VirtualScroller.test.ts index 93c74bcdc..bd70dc953 100644 --- a/tests/unit/utils/VirtualScroller.test.ts +++ b/tests/unit/utils/VirtualScroller.test.ts @@ -48,6 +48,20 @@ function createTestScrollerWithContainer(container: HTMLElement, items: TestItem return { renderItem, scroller }; } +function rectAt(top: number, bottom: number): DOMRect { + return { + top, + bottom, + left: 0, + right: 300, + width: 300, + height: bottom - top, + x: 0, + y: top, + toJSON: () => ({}), + } as DOMRect; +} + function renderedKeys(container: HTMLElement): string[] { return Array.from(container.querySelectorAll("[data-key]")).map( (element) => element.dataset.key ?? "" @@ -71,6 +85,87 @@ describe("VirtualScroller", () => { ).toBe("172px"); }); + describe("when a Base is embedded in a note", () => { + let raf: jest.SpyInstance; + let note: HTMLElement; + let container: HTMLElement; + let listTop: number; + + beforeEach(() => { + raf = jest.spyOn(window, "requestAnimationFrame").mockImplementation((callback) => { + callback(0); + return 1; + }); + // The note scrolls; the embedded view between it and the list grows with + // its content, so it never scrolls the list itself. + note = document.createElement("div"); + note.style.overflowY = "auto"; + const embeddedView = document.createElement("div"); + container = document.createElement("div"); + embeddedView.appendChild(container); + note.appendChild(embeddedView); + document.body.appendChild(note); + + listTop = 300; + note.getBoundingClientRect = () => rectAt(0, 500); + container.getBoundingClientRect = () => rectAt(listTop, listTop + 1000); + }); + + afterEach(() => { + raf.mockRestore(); + note.remove(); + }); + + const items = Array.from({ length: 50 }, (_, index) => ({ + id: `i${index}`, + label: `I${index}`, + })); + + it("renders the rows inside the note's viewport, measured from the list's own top", () => { + const { scroller } = createTestScrollerWithContainer(container, items); + + const keys = renderedKeys(container); + expect(keys[0]).toBe("i0"); + expect(keys[keys.length - 1]).toBe("i10"); + scroller.destroy(); + }); + + it("renders the rows the note scrolls into view", () => { + const { scroller } = createTestScrollerWithContainer(container, items); + + listTop = -400; + note.dispatchEvent(new Event("scroll")); + + const keys = renderedKeys(container); + expect(keys[0]).toBe("i19"); + expect(keys[keys.length - 1]).toBe("i45"); + scroller.destroy(); + }); + + it("ignores an ancestor with no box, such as a multi-column callout's content", () => { + const columns = document.createElement("div"); + columns.style.display = "contents"; + columns.style.overflowY = "auto"; + columns.getBoundingClientRect = () => rectAt(0, 0); + const embeddedView = container.parentElement as HTMLElement; + note.replaceChild(columns, embeddedView); + columns.appendChild(embeddedView); + + const { scroller } = createTestScrollerWithContainer(container, items); + + const keys = renderedKeys(container); + expect(keys[keys.length - 1]).toBe("i10"); + scroller.destroy(); + }); + + it("reserves the list's full height so the note can scroll to unrendered rows", () => { + const { scroller } = createTestScrollerWithContainer(container, items); + + expect(container.style.minHeight).toBe("1000px"); + scroller.destroy(); + }); + }); + it("reorders keyed items without rerendering existing visible elements", () => { const { container, renderItem, scroller } = createTestScroller([ { id: "a", label: "A" },