Full document, spreadsheet, slideshow, and diagram tooling
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge pull request 'fix(sheets): eliminate scroll jank v3 — atomic swap, pinned height' (#80) from fix/scroll-jank-v3 into main

scott 2ff30c6f 58e44c2c

+156 -26
+26 -26
src/sheets/main.ts
··· 451 451 let tbodyHtml = ''; 452 452 453 453 // --- Pre-compute conditional formatting and striped state --- 454 - const cfRules = getCfRulesArray(); 454 + // Skip expensive CF/validation during scroll-only renders for performance 455 + const cfRules = scrollOnly ? [] : getCfRulesArray(); 455 456 const stripedEnabled = getStripedRows(); 456 457 457 458 // --- Virtual scrolling: calculate visible row range (#146) --- ··· 469 470 const renderStartRow = Math.max(freezeR + 1, visibleRange.startRow); 470 471 const renderEndRow = visibleRange.endRow; 471 472 472 - // Top spacer: account for skipped rows above the visible range (after frozen rows) 473 - const skippedAbove = renderStartRow - freezeR - 1; 474 - const hiddenAboveAdjust = hiddenRowsSpacerAdjustment(freezeR + 1, renderStartRow - 1, hiddenRowSet, bodyRowHeight); 475 - if (skippedAbove > 0) { 476 - // Sum actual row heights for the skipped region, excluding hidden rows 477 - let spacerHeight = 0; 478 - for (let sr = freezeR + 1; sr < renderStartRow; sr++) { 479 - if (!hiddenRowSet.has(sr)) spacerHeight += rh(sr); 480 - } 481 - tbodyHtml += '<tr class="virtual-spacer-top" style="height:' + spacerHeight + 'px"><td colspan="' + (colCount + 1) + '"></td></tr>'; 473 + // Top spacer: always present (even with 0 height) for consistent DOM structure 474 + let topSpacerHeight = 0; 475 + for (let sr = freezeR + 1; sr < renderStartRow; sr++) { 476 + if (!hiddenRowSet.has(sr)) topSpacerHeight += rh(sr); 482 477 } 478 + tbodyHtml += '<tr class="virtual-spacer-top" style="height:' + topSpacerHeight + 'px"><td colspan="' + (colCount + 1) + '"></td></tr>'; 483 479 484 480 // --- Body rows (frozen rows always rendered, then visible range) --- 485 481 const frozenRowsToRender = []; ··· 610 606 prevRenderedRow = r; 611 607 } 612 608 613 - // Bottom spacer: account for skipped rows below the visible range 614 - const skippedBelow = rowCount - renderEndRow; 615 - const hiddenBelowAdjust = hiddenRowsSpacerAdjustment(renderEndRow + 1, rowCount, hiddenRowSet, bodyRowHeight); 616 - if (skippedBelow > 0) { 617 - // Sum actual row heights for non-hidden rows below the visible range 618 - let bottomSpacerHeight = 0; 619 - for (let sr = renderEndRow + 1; sr <= rowCount; sr++) { 620 - if (!hiddenRowSet.has(sr)) bottomSpacerHeight += rh(sr); 621 - } 622 - tbodyHtml += '<tr class="virtual-spacer-bottom" style="height:' + bottomSpacerHeight + 'px"><td colspan="' + (colCount + 1) + '"></td></tr>'; 609 + // Bottom spacer: always present (even with 0 height) for consistent DOM structure 610 + let bottomSpacerHeight = 0; 611 + for (let sr = renderEndRow + 1; sr <= rowCount; sr++) { 612 + if (!hiddenRowSet.has(sr)) bottomSpacerHeight += rh(sr); 623 613 } 614 + tbodyHtml += '<tr class="virtual-spacer-bottom" style="height:' + bottomSpacerHeight + 'px"><td colspan="' + (colCount + 1) + '"></td></tr>'; 624 615 625 616 // --- Apply to DOM --- 617 + // Compute total body height for all non-hidden rows (pinned on the table to prevent 618 + // scroll position clamping during DOM swap — the table height never changes) 619 + let totalBodyHeight = 0; 620 + for (let r = 1; r <= rowCount; r++) { 621 + if (!hiddenRowSet.has(r)) totalBodyHeight += rh(r); 622 + } 623 + grid.style.minHeight = (headerRowHeight + totalBodyHeight) + 'px'; 624 + 626 625 const savedScrollTop = sheetContainer ? sheetContainer.scrollTop : 0; 627 626 const savedScrollLeft = sheetContainer ? sheetContainer.scrollLeft : 0; 628 627 629 628 if (scrollOnly) { 630 - // Scroll-only: replace just tbody content, keeping colgroup+thead stable 631 - const tbody = grid.querySelector('tbody'); 632 - if (tbody) { 633 - tbody.innerHTML = tbodyHtml; 629 + // Scroll-only: atomic tbody swap — build offscreen, then replaceChild 630 + const oldTbody = grid.querySelector('tbody'); 631 + if (oldTbody) { 632 + const newTbody = document.createElement('tbody'); 633 + newTbody.innerHTML = tbodyHtml; 634 + grid.replaceChild(newTbody, oldTbody); 634 635 } else { 635 - // Fallback: no tbody yet, do full render 636 636 grid.innerHTML = headHtml + '<tbody>' + tbodyHtml + '</tbody>'; 637 637 } 638 638 } else {
+130
tests/scroll-stability.test.ts
··· 751 751 expect(range.endRow).toBeGreaterThanOrEqual(lastVisible); 752 752 }); 753 753 }); 754 + 755 + // ============================================================ 756 + // ALWAYS-PRESENT SPACERS (v3 fix) 757 + // ============================================================ 758 + 759 + describe('always-present spacers', () => { 760 + const uniform26 = (_r: number) => 26; 761 + 762 + it('spacers are defined (>= 0) even at top of sheet', () => { 763 + const range = calculateVisibleRange({ 764 + scrollTop: 0, 765 + viewportHeight: 600, 766 + totalRows: 200, 767 + getRowHeight: uniform26, 768 + bufferRows: 15, 769 + }); 770 + const result = computeSpacerHeights(200, uniform26, range); 771 + expect(result.topSpacerHeight).toBeGreaterThanOrEqual(0); 772 + expect(result.bottomSpacerHeight).toBeGreaterThanOrEqual(0); 773 + // At top, top spacer should be 0 774 + expect(result.topSpacerHeight).toBe(0); 775 + // Bottom spacer should cover remaining rows 776 + expect(result.bottomSpacerHeight).toBeGreaterThan(0); 777 + }); 778 + 779 + it('spacers are defined (>= 0) even at bottom of sheet', () => { 780 + const range = calculateVisibleRange({ 781 + scrollTop: 200 * 26, 782 + viewportHeight: 600, 783 + totalRows: 200, 784 + getRowHeight: uniform26, 785 + bufferRows: 15, 786 + }); 787 + const result = computeSpacerHeights(200, uniform26, range); 788 + expect(result.topSpacerHeight).toBeGreaterThanOrEqual(0); 789 + expect(result.bottomSpacerHeight).toBeGreaterThanOrEqual(0); 790 + }); 791 + 792 + it('spacers always defined with buffer=15 at all positions', () => { 793 + const rows = 500; 794 + for (let scrollTop = 0; scrollTop <= rows * 26; scrollTop += 100) { 795 + const range = calculateVisibleRange({ 796 + scrollTop, 797 + viewportHeight: 600, 798 + totalRows: rows, 799 + getRowHeight: uniform26, 800 + bufferRows: 15, 801 + }); 802 + const result = computeSpacerHeights(rows, uniform26, range); 803 + // Both spacers must always be non-negative (always rendered) 804 + expect(result.topSpacerHeight).toBeGreaterThanOrEqual(0); 805 + expect(result.bottomSpacerHeight).toBeGreaterThanOrEqual(0); 806 + // Total must always equal the full sheet height 807 + expect(result.totalHeight).toBe(rows * 26); 808 + } 809 + }); 810 + }); 811 + 812 + // ============================================================ 813 + // PINNED TABLE HEIGHT (v3 fix) 814 + // ============================================================ 815 + 816 + describe('pinned table height', () => { 817 + const uniform26 = (_r: number) => 26; 818 + 819 + it('total non-hidden row height is constant regardless of scroll position', () => { 820 + const rows = 300; 821 + const varHeight = (r: number) => (r % 10 === 0 ? 40 : 26); 822 + const expected = totalRowHeights(rows, varHeight); 823 + 824 + // Verify at many positions — this is what grid.style.minHeight pins to 825 + for (let st = 0; st <= 10000; st += 250) { 826 + expect(totalRowHeights(rows, varHeight)).toBe(expected); 827 + } 828 + }); 829 + 830 + it('total non-hidden row height excludes hidden rows consistently', () => { 831 + const rows = 200; 832 + const hidden = new Set([10, 20, 30, 40, 50]); 833 + const expected = totalRowHeights(rows, uniform26, hidden); 834 + expect(expected).toBe((200 - 5) * 26); 835 + }); 836 + }); 837 + 838 + // ============================================================ 839 + // BUFFER=15 REDUCES RE-RENDERS 840 + // ============================================================ 841 + 842 + describe('buffer=15 re-render reduction', () => { 843 + const uniform26 = (_r: number) => 26; 844 + 845 + it('scrolling 300px from top does not change range with buffer=15', () => { 846 + const base = calculateVisibleRange({ 847 + scrollTop: 0, 848 + viewportHeight: 600, 849 + totalRows: 500, 850 + getRowHeight: uniform26, 851 + bufferRows: 15, 852 + }); 853 + const scrolled = calculateVisibleRange({ 854 + scrollTop: 300, 855 + viewportHeight: 600, 856 + totalRows: 500, 857 + getRowHeight: uniform26, 858 + bufferRows: 15, 859 + }); 860 + // With buffer=15, first 15 rows are buffer. Scrolling 300px (~11 rows) 861 + // should still be within the buffered range 862 + expect(scrolled.startRow).toBe(base.startRow); 863 + }); 864 + 865 + it('scrolling 390px+ from top changes the range', () => { 866 + const base = calculateVisibleRange({ 867 + scrollTop: 0, 868 + viewportHeight: 600, 869 + totalRows: 500, 870 + getRowHeight: uniform26, 871 + bufferRows: 15, 872 + }); 873 + const scrolled = calculateVisibleRange({ 874 + scrollTop: 500, 875 + viewportHeight: 600, 876 + totalRows: 500, 877 + getRowHeight: uniform26, 878 + bufferRows: 15, 879 + }); 880 + // After scrolling ~19 rows, the range should have shifted 881 + expect(scrolled.startRow).toBeGreaterThan(base.startRow); 882 + }); 883 + });