From 00e845cf62383c84ab1e75d477c094668cbed939 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 17 Jun 2026 21:26:52 -0400 Subject: [PATCH] Fix sticky section header focus ring, layering, and doubling at the top Pin the header only once its real row scrolls under the top of the view (no doubled title at the top edge or during overscroll), keep the pinned copy above focused rows so they scroll underneath it, and strip stale selection/focus state from the pinned copy. --- .../zotero/components/virtualized-table.jsx | 14 +++++++---- scss/components/_virtualized-table.scss | 4 +++- test/tests/collectionViewItemTreeTest.js | 24 +++++++++++++++---- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/chrome/content/zotero/components/virtualized-table.jsx b/chrome/content/zotero/components/virtualized-table.jsx index 9e7e4efb03..4b7007b35c 100644 --- a/chrome/content/zotero/components/virtualized-table.jsx +++ b/chrome/content/zotero/components/virtualized-table.jsx @@ -1449,16 +1449,17 @@ class VirtualizedTable extends React.Component { break; } } - // Nothing to pin if there are no headers or the view is scrolled above the first - if (currentIndex == -1) { + // Show the pinned copy only once the header row has scrolled up past the top edge of + // the view + let stuck = currentIndex != -1 + && scrollTop > this._jsWindow._getItemPosition(currentIndex); + if (!stuck) { clip.style.display = 'none'; this._stickyHeaderIndex = null; return; } clip.style.display = ''; - // Mark the header "stuck" once its section has scrolled up under it, so the - // consumer can show a divider (hairline) only while scrolling - clip.classList.toggle('stuck', scrollTop > this._jsWindow._getItemPosition(currentIndex)); + clip.classList.add('stuck'); // Re-render only when the pinned section changes. Use _renderItem (not the raw // renderItem prop) so the pinned copy gets the same post-processing as a real row // (e.g. the tree's indent/twisty spacer), then drop its id to avoid duplicating the @@ -1467,6 +1468,9 @@ class VirtualizedTable extends React.Component { this._stickyHeaderIndex = currentIndex; let node = this._renderItem(currentIndex); node.removeAttribute('id'); + // Strip the focus ring: focus defaults to row 0, which can be a header, but a + // pinned header shouldn't show focus + node.classList.remove('focused'); content.textContent = ''; content.appendChild(node); } diff --git a/scss/components/_virtualized-table.scss b/scss/components/_virtualized-table.scss index 6d964e595b..268e399553 100644 --- a/scss/components/_virtualized-table.scss +++ b/scss/components/_virtualized-table.scss @@ -365,7 +365,9 @@ // at the body's top edge instead of spilling over the column header. .virtualized-table-sticky-section-header { position: absolute; - z-index: 1; + // Above the rows scrolling under it, including a focused row (z-index: 10000), so they + // pass underneath the pinned header rather than over it + z-index: 10001; overflow: hidden; pointer-events: none; diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index 44db36fa11..ef3e8d5d5f 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -2669,16 +2669,22 @@ describe("CollectionViewItemTree", function () { let view = zp.itemsView; let tree = view.tree; let body = tree._jsWindow.targetElement; + let userHeaderRow = view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID); let groupHeaderRow = view.getRowIndexByID("L" + group.libraryID); - // At the top of the list, the first (user library) header is pinned + // At the very top, the real header is in place, so nothing is pinned -- a pinned + // copy would just double the header body.scrollTop = 0; tree._updateStickySectionHeader(); + assert.equal(tree._stickyHeader.style.display, 'none'); + + // Scrolling the first (user library) header up under the top pins it + body.scrollTop = tree._jsWindow._getItemPosition(userHeaderRow) + 5; + tree._updateStickySectionHeader(); assert.include(tree._stickyHeader.textContent, Zotero.Libraries.getName(Zotero.Libraries.userLibraryID)); // The pinned header lines up horizontally with the real header row - let userHeaderRow = view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID); let realIcon = tree._jsWindow.getElementByIndex(userHeaderRow).querySelector('.icon-item-type'); let stickyIcon = tree._stickyHeaderContent.querySelector('.icon-item-type'); assert.equal( @@ -2687,8 +2693,18 @@ describe("CollectionViewItemTree", function () { "Pinned header icon should align with the real header icon" ); - // Scrolling the group header to the top pins the group library header instead - body.scrollTop = tree._jsWindow._getItemPosition(groupHeaderRow); + // A focused header row renders with the focus class, but the pinned copy must + // not carry that focus ring + tree.selection.focused = userHeaderRow; + assert.isTrue(tree._renderItem(userHeaderRow).classList.contains('focused'), + "Setup: a focused header row renders with the focus class"); + tree._stickyHeaderIndex = null; + tree._updateStickySectionHeader(); + assert.isFalse(tree._stickyHeaderContent.querySelector('.row').classList.contains('focused'), + "Pinned header should not show a focus ring"); + + // Scrolling the group header up under the top pins the group library header instead + body.scrollTop = tree._jsWindow._getItemPosition(groupHeaderRow) + 5; tree._updateStickySectionHeader(); assert.include(tree._stickyHeader.textContent, Zotero.Libraries.getName(group.libraryID));