From b6649f9fb9c2e0789fd71801c1ebb4a81fb654e4 Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Fri, 20 Jan 2023 14:37:17 +1300 Subject: [PATCH] Bug 1811408 - Check the number of rows in a list if it resizes. r=aleca Differential Revision: https://phabricator.services.mozilla.com/D167341 --HG-- extra : rebase_source : f065331aede0ed4eaf8eb491b3f86508eab2978f --- .../content/widgets/tree-view-listbox.mjs | 38 ++++----- .../test/browser/browser_treeViewListbox.js | 85 +++++++++++++++++++ 2 files changed, 100 insertions(+), 23 deletions(-) diff --git a/mail/base/content/widgets/tree-view-listbox.mjs b/mail/base/content/widgets/tree-view-listbox.mjs index c40559414c..5147135663 100644 --- a/mail/base/content/widgets/tree-view-listbox.mjs +++ b/mail/base/content/widgets/tree-view-listbox.mjs @@ -14,9 +14,6 @@ const { JSTreeSelection } = ChromeUtils.import( const accelKeyName = AppConstants.platform == "macosx" ? "metaKey" : "ctrlKey"; const otherKeyName = AppConstants.platform == "macosx" ? "ctrlKey" : "metaKey"; -// Animation variables for expanding and collapsing child lists. -let reducedMotionMedia = matchMedia("(prefers-reduced-motion)"); - /** * Main tree view container that takes care of generating the main scrollable * DIV and the tree table. @@ -915,14 +912,13 @@ class TreeViewListbox extends HTMLTableSectionElement { } }); + // Ensure that there are enough rows for scrolling/resizing to appear + // seamless, but don't do it more frequently than 10 times per second, + // as it's expensive. let lastTime = 0; + let lastHeight = 0; let timer = null; - this.scrollable.addEventListener("scroll", () => { - if (reducedMotionMedia.matches) { - this._ensureVisibleRowsAreDisplayed(); - return; - } - + let throttledUpdate = () => { let now = Date.now(); let diff = now - lastTime; @@ -936,10 +932,16 @@ class TreeViewListbox extends HTMLTableSectionElement { timer = null; }, 100 - diff); } + }; + this.scrollable.addEventListener("scroll", () => throttledUpdate()); + this.resizeObserver = new ResizeObserver(entries => { + // There's not much point in reducing the number of rows on resize. + if (this.scrollable.clientHeight > lastHeight) { + throttledUpdate(); + } + lastHeight = this.scrollable.clientHeight; }); - - window.addEventListener("load", this); - window.addEventListener("resize", this); + this.resizeObserver.observe(this.scrollable); } disconnectedCallback() { @@ -952,17 +954,7 @@ class TreeViewListbox extends HTMLTableSectionElement { this.lastChild.remove(); } - window.removeEventListener("load", this); - window.removeEventListener("resize", this); - } - - handleEvent(event) { - switch (event.type) { - case "load": - case "resize": - this._ensureVisibleRowsAreDisplayed(); - break; - } + this.resizeObserver.disconnect(); } attributeChangedCallback(name, oldValue, newValue) { diff --git a/mail/base/test/browser/browser_treeViewListbox.js b/mail/base/test/browser/browser_treeViewListbox.js index 8ff873ce53..ab1faa6c11 100644 --- a/mail/base/test/browser/browser_treeViewListbox.js +++ b/mail/base/test/browser/browser_treeViewListbox.js @@ -1572,3 +1572,88 @@ async function subtestRowClassChange() { Assert.ok(!row.classList.contains("current")); } } + +/** + * Checks that resizing the widget automatically adds more rows if necessary. + */ +add_task(async function testResize() { + let tab = tabmail.openTab("contentTab", { + url: + "chrome://mochitests/content/browser/comm/mail/base/test/browser/files/treeViewListbox.xhtml", + }); + + await BrowserTestUtils.browserLoaded(tab.browser); + tab.browser.focus(); + + await SpecialPowers.spawn(tab.browser, [], subtestResize); + + tabmail.closeTab(tab); +}); + +async function subtestResize() { + const { TestUtils } = ChromeUtils.importESModule( + "resource://testing-common/TestUtils.sys.mjs" + ); + + let doc = content.document; + + let list = doc.querySelector(`[is="tree-view-listbox"]`); + Assert.ok(!!list, "the list exists"); + + let rowCount = function() { + return list.querySelectorAll(`tr[is="test-listrow"]`).length; + }; + + let originalHeight = list.scrollable.clientHeight; + + // Start by scrolling to somewhere in the middle of the list, so that we + // don't have to think about buffer rows that don't exist at the ends. + list.scrollable.scrollBy(0, 650); + + // The list has enough space for 13 visible rows, and 10 buffer rows should + // exist above and below. + await TestUtils.waitForCondition( + () => rowCount() == 33, + "the list should the right number of rows" + ); + + // Make the list shorter by 5 rows. This should not affect the number of rows, + // but this is a bit flaky, so check we have at least the minimum required. + list.scrollable.style.height = `${originalHeight - 250}px`; + await new Promise(resolve => content.setTimeout(resolve, 500)); + Assert.greaterOrEqual( + rowCount(), + 28, + "making the list shorter should not change the number of rows" + ); + + // Scrolling the list by any amount should remove excess rows. + list.scrollable.scrollBy(0, 50); + await TestUtils.waitForCondition( + () => rowCount() == 28, + "scrolling the list after resize should remove the excess rows" + ); + + // Return to the original height. More buffer rows should be added. + list.scrollable.style.height = `${originalHeight}px`; + await TestUtils.waitForCondition( + () => rowCount() == 33, + "making the list taller should change the number of rows" + ); + + // Make the list taller by 5 rows. + list.scrollable.style.height = `${originalHeight + 250}px`; + await TestUtils.waitForCondition( + () => rowCount() == 38, + "making the list taller should change the number of rows" + ); + + // Scrolling the list should not affect the number of rows. + list.scrollable.scrollBy(0, 50); + await new Promise(resolve => content.setTimeout(resolve, 1000)); + Assert.equal( + rowCount(), + 38, + "scrolling the list should not change the number of rows" + ); +}