diff --git a/chrome/content/zotero/components/virtualized-table.jsx b/chrome/content/zotero/components/virtualized-table.jsx index 4b7007b35c..bf625ac3e7 100644 --- a/chrome/content/zotero/components/virtualized-table.jsx +++ b/chrome/content/zotero/components/virtualized-table.jsx @@ -1312,7 +1312,12 @@ class VirtualizedTable extends React.Component { } let jsWindowProps = { id: this._jsWindowID, - className: "virtualized-table-body", + // Reserve a scrollbar gutter when there are sticky section headers (see CSS), so the + // scrollbar doesn't float over the content. Without it a macOS overlay scrollbar + // overlaps the rows, and the opaque pinned header -- which must paint above the rows + // to occlude them, and so above the scrollbar -- covers the scrollbar's edge. + className: "virtualized-table-body" + + (this.props.stickySectionHeaders ? " has-sticky-section-headers" : ""), onFocus: (e) => { if (e.target.id == this._jsWindowID) { // Focus should always remain on the list itself. @@ -1325,18 +1330,23 @@ class VirtualizedTable extends React.Component {
{columnDragMarker} {header} -
- {this.props.stickySectionHeaders - &&
this._stickyHeader = ref} - aria-hidden="true" - > -
this._stickyHeaderContent = ref} - /> -
} +
+ {/* Pinned copy of the current section's header. Lives inside the scrolling + body (as its first child, before the windowed-list) and pins with + position: sticky, so its width tracks the body's content box automatically + and it lines up with the rows without any JS geometry. */} + {this.props.stickySectionHeaders + &&
this._stickyHeader = ref} + aria-hidden="true" + > +
this._stickyHeaderContent = ref} + /> +
} +
); } @@ -1420,13 +1430,15 @@ class VirtualizedTable extends React.Component { /** * Pin the header of the section currently at the top of the view, pushing it up as - * the next section's header scrolls into it. The pinned header lives outside the - * scrolling body (so it doesn't scroll itself) and reuses the consumer's renderItem - * to match the real header row's appearance. + * the next section's header scrolls into it, and reuse the consumer's renderItem so the + * pinned copy matches the real header row's appearance. * - * The outer element is a fixed clip region anchored to the top of the body; the inner - * (opaque) element holds the rendered header and is the part that translates, so the - * push is clipped at the top of the body rather than spilling over the column header. + * The pinned header is the first child of the scrolling body and stays put via position: + * sticky, taking its width from the body's content box so it lines up with the rows. The + * outer element has zero height (so it adds no space to the flow); the inner (opaque) + * element holds the rendered header, overflows downward over the rows, and is the part that + * translates, so a header pushed up by the next section is clipped at the top of the body + * (by the body's own overflow) rather than spilling over the column header. */ _updateStickySectionHeader = () => { if (!this.props.stickySectionHeaders || !this._stickyHeader || !this._jsWindow) { @@ -1474,18 +1486,10 @@ class VirtualizedTable extends React.Component { content.textContent = ''; content.appendChild(node); } - // Anchor the clip region over the top of the list body (below the column header, - // inside the scrollbar). Its height covers the pinned header plus the body's top - // padding, so a header pushed up is clipped at the body's top edge. Use the body's - // sub-pixel top (not the integer-rounded offsetTop) so the overlay lands exactly at - // the body's edge and never creeps over the column header's bottom divider. - let body = this._jsWindow.targetElement; - let insetTop = parseFloat(window.getComputedStyle(body).paddingTop) || 0; - let bodyTop = body.getBoundingClientRect().top - this._topDiv.getBoundingClientRect().top; - clip.style.top = bodyTop + 'px'; - clip.style.left = body.offsetLeft + 'px'; - clip.style.width = body.clientWidth + 'px'; - clip.style.height = (this._rowHeight + insetTop) + 'px'; + // Geometry is all CSS now: the clip is the first child of the scrolling body, has zero + // height (so it takes no space in the flow and the rows below aren't shifted down), and + // pins itself with position: sticky. Its content overflows downward over the rows and + // gets its width from the body's content box, so it lines up with the real rows. // Push the pinned header up as the next section's header approaches the top let translateY = 0; if (nextIndex != -1) { diff --git a/scss/components/_item-tree.scss b/scss/components/_item-tree.scss index 6bf78cbb12..431266426b 100644 --- a/scss/components/_item-tree.scss +++ b/scss/components/_item-tree.scss @@ -17,6 +17,16 @@ padding: 4px 8px 8px; } + // With sticky section headers, the body can't have top padding: overflow clips at the + // padding box, so rows would scroll up into the padding and show above the pinned header + // (which sticks to the content-box top). Drop the top padding so rows clip exactly where the + // header pins, and keep the gap below the column header as a margin -- outside the scroll + // area, where rows can't reach it. + .virtualized-table-body.has-sticky-section-headers { + padding-top: 0; + margin-top: 4px; + } + .virtualized-table-body { scrollbar-color: var(--color-scrollbar) var(--color-scrollbar-background); } diff --git a/scss/components/_virtualized-table.scss b/scss/components/_virtualized-table.scss index 268e399553..9b186cc7db 100644 --- a/scss/components/_virtualized-table.scss +++ b/scss/components/_virtualized-table.scss @@ -358,22 +358,23 @@ } } -// Pinned copy of the current section's header row. The outer element is a fixed clip -// region anchored over the top of the body, aligned to the rows' container so the pinned -// row lines up with the real rows (positioned/sized in virtualized-table.jsx). The inner -// element is opaque and translates, so a header pushed up by the next section is clipped -// at the body's top edge instead of spilling over the column header. +// Pinned copy of the current section's header row. It's the first child of the scrolling +// body and pins itself with position: sticky, so its width comes from the body's content box +// for free and it lines up with the real rows without any JS geometry. It has zero height -- +// so it takes no space in the flow, doesn't shift the rows down, and pins flush with the top +// of the rows -- and its content overflows downward over the rows. (The body's scrollbar is +// kept clear of it by the reserved scrollbar gutter, not by paint order; see below.) .virtualized-table-sticky-section-header { - position: absolute; + position: sticky; + top: 0; + height: 0; // 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; .virtualized-table-sticky-section-header-content { box-sizing: border-box; - padding: 4px 8px 0; // Opaque so the pinned header occludes the rows (and the real header row) beneath it background-color: var(--material-background); @@ -389,6 +390,17 @@ overflow: auto; padding: 4px 8px 8px; + // With sticky section headers, reserve the scrollbar's track so it doesn't float over the + // content. The pinned header is opaque and must paint above the rows to occlude them, which + // in Gecko also paints it above the scrollbar; keeping the scrollbar out of the content box + // is what stops it from covering the scrollbar's edge (see the sticky header rules above). + // The body must also have no top padding so rows can't scroll up into it and show above the + // pinned header; that's handled per-consumer (e.g. #zotero-items-tree) where the padding is + // set with higher specificity. + &.has-sticky-section-headers { + scrollbar-gutter: stable; + } + .cell { // NOTE: Do not add padding here. See #5198 text-overflow: ellipsis;