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.
This commit is contained in:
Dan Stillman 2026-06-17 21:26:52 -04:00
parent 0a891abcec
commit 00e845cf62
3 changed files with 32 additions and 10 deletions

View file

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

View file

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

View file

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