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));