From 015769a9f85990a896b288954524f097af307678 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Thu, 4 Sep 2025 09:42:04 -0700 Subject: [PATCH] fx140: allow pointer events on cells in virt table (#5518) Revert change from https://github.com/zotero/zotero/commit/8e2790e2d2a1d8b15efbf84935f0a80d58db4e44. Post fx140, dragstart fires fine on rows that are not yet selected. This allows us to remove the workaround that limited pointer events to the rows of virtualized table (vs its cells). With it, we no longer need most of the manual handling of the tooltip in virtualized table - just adding title attribute is enough. We also don't need to add extra mouse events handling on clickable button columns as in citationDialog - we can use proper :hover and :activate effects, as well as attach click handler directly to the button. --- .../zotero/components/virtualized-table.jsx | 72 +++--------------- .../zotero/integration/citationDialog.js | 73 ++++--------------- chrome/content/zotero/itemTree.jsx | 1 - scss/components/_citationDialog.scss | 55 -------------- scss/components/_virtualized-table.scss | 53 ++++++++++++-- 5 files changed, 74 insertions(+), 180 deletions(-) diff --git a/chrome/content/zotero/components/virtualized-table.jsx b/chrome/content/zotero/components/virtualized-table.jsx index df3c3a4243..ec16c2af7c 100644 --- a/chrome/content/zotero/components/virtualized-table.jsx +++ b/chrome/content/zotero/components/virtualized-table.jsx @@ -31,7 +31,6 @@ const cx = require('classnames'); const WindowedList = require('./windowed-list'); const Draggable = require('./draggable'); const { CSSIcon, getCSSIcon } = require('components/icons'); -const { Zotero_Tooltip } = require('./tooltip'); const TYPING_TIMEOUT = 1000; const MINIMUM_ROW_HEIGHT = 20; // px @@ -726,6 +725,14 @@ class VirtualizedTable extends React.Component { this.focus(); } + // Prevent clicks on buttons in cells from selecting the row + _captureMouseUpDown = (event) => { + let clickableCell = event.target.closest('.cell.clickable'); + if (clickableCell) { + event.stopPropagation(); + } + }; + _handleContextMenu = async (e, index) => { if (e.target.localName === 'input') { // Do not hijack context menu on inputs, fixes #5374 @@ -922,11 +929,9 @@ class VirtualizedTable extends React.Component { * @param event */ _handleMouseOver = (event) => { - // On scroll, mouse position does not change, so _handleMouseMove does not fire - // to close the fake tooltip. Make sure it is closed here. - Zotero_Tooltip.stop(); let elem = event.target; - if (!elem.classList.contains('cell') || elem.classList.contains('cell-icon')) return; + let cell = elem.closest('.cell'); + if (!cell || elem.classList.contains('cell-icon')) return; let textElem = elem.querySelector('.label, .cell-text'); // .label is used in the header, .cell-text on primary cells, // otherwise the .cell element if its immediate child is a text node @@ -945,59 +950,6 @@ class VirtualizedTable extends React.Component { } } - /** - * Manually handle tooltip setting for table cells with overflowing values. - * Temporary, after - * https://github.com/zotero/zotero/commit/8e2790e2d2a1d8b15efbf84935f0a80d58db4e44. - * @param event - */ - _handleMouseMove = (event) => { - let tgt = event.target; - // Mouse left the previous cell - close the tooltip - if (!tgt.classList.contains("row") - || event.clientX < parseInt(tgt.dataset.mouseLeft) - || event.clientX > parseInt(tgt.dataset.mouseRight)) { - delete tgt.dataset.mouseLeft; - delete tgt.dataset.mouseRight; - Zotero_Tooltip.stop(); - } - - if (!tgt.classList.contains("row")) return; - let cells = tgt.querySelectorAll(".cell"); - let targetCell; - // Find the cell the mouse is over - for (let cell of cells) { - let rect = cell.getBoundingClientRect(); - if (event.clientX >= rect.left && event.clientX <= rect.right) { - targetCell = cell; - tgt.dataset.mouseLeft = rect.left; - tgt.dataset.mouseRight = rect.right; - break; - } - } - if (!targetCell) return; - // Primary cell will .cell-text child node - let textCell = targetCell.querySelector(".cell-text") || targetCell; - // If the cell has overflowing content, display the fake tooltip - if (textCell.offsetWidth < textCell.scrollWidth) { - Zotero_Tooltip.stop(); - Zotero_Tooltip.start(textCell.textContent); - } - }; - - /** - * Remove manually added fake tooltip from _handleMouseMove when the - * mouse leaves the row completely. - */ - _handleMouseLeave = (_) => { - Zotero_Tooltip.stop(); - let lastRow = document.querySelector("[mouseLeft][mouseRight]"); - if (lastRow) { - delete lastRow.dataset.mouseLeft; - delete lastRow.dataset.mouseRight; - } - }; - _handleResizerDragStop = (event) => { event.stopPropagation(); const result = this._getResizeColumns(); @@ -1161,6 +1113,8 @@ class VirtualizedTable extends React.Component { node.addEventListener('dragstart', e => this._onDragStart(e, index), { passive: true }); node.addEventListener('dragend', e => this._onDragEnd(e, index), { passive: true }); node.addEventListener('mousedown', e => this._handleMouseDown(e, index), { passive: true }); + node.addEventListener('mouseup', this._captureMouseUpDown, { capture: true }); + node.addEventListener('mousedown', this._captureMouseUpDown, { capture: true }); node.addEventListener('contextmenu', e => this._handleContextMenu(e, index), { passive: true }); node.addEventListener('mouseup', e => this._handleMouseUp(e, index), { passive: true }); node.addEventListener('dblclick', e => this._activateNode(e, [index]), { passive: true }); @@ -1265,8 +1219,6 @@ class VirtualizedTable extends React.Component { onDrop: e => this.props.onDrop && this.props.onDrop(e), onFocus: e => this.props.onFocus && this.props.onFocus(e), onMouseOver: e => this._handleMouseOver(e), - onMouseMove: e => this._handleMouseMove(e), - onMouseLeave: e => this._handleMouseLeave(e), className: cx(["virtualized-table", // For selected items icon color, see scss mixin svgicon and focus-states "focus-states-target", diff --git a/chrome/content/zotero/integration/citationDialog.js b/chrome/content/zotero/integration/citationDialog.js index 3c889861fb..875b1c2b87 100644 --- a/chrome/content/zotero/integration/citationDialog.js +++ b/chrome/content/zotero/integration/citationDialog.js @@ -551,6 +551,9 @@ class LibraryLayout extends Layout { doc.l10n.setAttributes(cell, "integration-citationDialog-items-table"); } iconWrapper.append(icon); + iconWrapper.addEventListener("click", () => { + this._handleItemsViewIconClick(index); + }); return cell; } }); @@ -606,11 +609,6 @@ class LibraryLayout extends Layout { } }); doc.querySelector("item-tree-menu-bar").init(this.itemsView); - // handle icon click to add/remove items - itemsTree.addEventListener("mousedown", event => this._handleItemsViewRowClick(event), true); - itemsTree.addEventListener("mouseup", event => this._handleItemsViewRowClick(event), true); - // manually handle hover effect on +/- icon, since css :hover applies to the entire row - itemsTree.addEventListener("mousemove", event => this._handleItemsViewMouseMove(event)); // handle backspace/delete to remove an item from citation itemsTree.addEventListener("keypress", event => this._handleItemsViewKeyPress(event)); // only highlight bubbles of selected rows when the focus is in itemTree @@ -680,59 +678,6 @@ class LibraryLayout extends Layout { this.itemsView.clearItemsPaneMessage(); } - // Handle mouseup and mousedown events on a row in itemTree to enable clicking on +/- button - // On mousedown, add .active effect to the +/- button - // On mouseup, add/remove the clicked item from the citation - // This specific handling is required, since :active effect fires on the row and not the child button - _handleItemsViewRowClick(event) { - // only trigger on left mouse click - if (event.button !== 0) return; - let row = event.target; - // find which icon we hovered over - let hoveredOverIcon = row.querySelector(".icon-action.hover"); - if (!hoveredOverIcon) return; - if (event.type == "mouseup") { - let rowIndex = row.id.split("-")[4]; - let clickedItem = this.itemsView.getRow(rowIndex).ref; - hoveredOverIcon.classList.remove("active"); - let rowTopBeforeRefresh = row.getBoundingClientRect().top; - this.itemsView.selection.clearSelection(); - IOManager.addItemsToCitation([clickedItem]).then(() => { - this._scrollItemTreeToRow(row.id, rowTopBeforeRefresh); - }); - } - else if (event.type == "mousedown") { - hoveredOverIcon.classList.add("active"); - } - // stop propagation to not select the row - event.stopPropagation(); - // do not move focus into the table - event.preventDefault(); - } - - // Add .hover effect to +/- button when the mouse is above it - // This handling is required, since :hover effect fires on the row and not the actual button - _handleItemsViewMouseMove(event) { - let { clientY, clientX, target } = event; - let actionIcons = [...event.target.querySelectorAll(".icon-action")]; - if (!actionIcons.length) return; - // find which icon we hovered over - let hoveredOverIcon = actionIcons.find((icon) => { - let iconRect = icon.getBoundingClientRect(); - // event.target is the actual row, so check if the click happened - // within the bounding box of the +/- icon and handle it same as a double click - let overIcon = clientX > iconRect.left && clientX < iconRect.right - && clientY > iconRect.top && clientY < iconRect.bottom; - return overIcon; - }); - if (!target.classList.contains("row") || !hoveredOverIcon) { - _id('zotero-items-tree').querySelector(".icon-action.hover")?.classList.remove("hover"); - _id('zotero-items-tree').querySelector(".icon-action.active")?.classList.remove("active"); - return; - } - hoveredOverIcon.classList.add("hover"); - } - // backspace/delete in itemsView deletes items from the citation _handleItemsViewKeyPress(event) { if (event.key == "Delete" || Zotero.isMac && event.key == "Backspace") { @@ -746,6 +691,18 @@ class LibraryLayout extends Layout { } } + // click on + icon will add the item to the citation + _handleItemsViewIconClick(index) { + let rowNode = doc.querySelector(`#item-tree-citationDialog-row-${index}`); + let rowTopBeforeRefresh = rowNode.getBoundingClientRect().top; + this.itemsView.selection.clearSelection(); + let row = this.itemsView.getRow(index); + // after adding the item, try to keep the mouse over it even if the bubble-input gets taller + IOManager.addItemsToCitation([row.ref]).then(() => { + this._scrollItemTreeToRow(rowNode.id, rowTopBeforeRefresh); + }); + } + // Highlight/de-highlight selected rows async _refreshItemsViewHighlightedRows() { let selectedIDs = CitationDataManager.getCitedLibraryItemIDs(); diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 344edc4a5b..06e2ec52c0 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -3142,7 +3142,6 @@ var ItemTree = class ItemTree extends LibraryTree { if (this.isContainerOpen(index)) { twisty.classList.add('open'); } - twisty.style.pointerEvents = 'auto'; twisty.addEventListener('mousedown', event => event.stopPropagation()); twisty.addEventListener('mouseup', event => this.handleTwistyMouseUp(event, index), { passive: true }); diff --git a/scss/components/_citationDialog.scss b/scss/components/_citationDialog.scss index 91f364a3b3..5c86cd73b4 100644 --- a/scss/components/_citationDialog.scss +++ b/scss/components/_citationDialog.scss @@ -344,61 +344,6 @@ #item-tree-container { min-height: 100%; flex: 1; /* expand all the way to the right */ - - // the column with the + button - .clickable { - // make sure the button is centered - display: flex; - justify-content: center; - padding: 0; - - // the actual clickable button - .icon-action { - display: flex; - justify-content: center; - width: 20px; - height: 20px; - align-items: center; - border-radius: 6px; - .icon { - width: 16px; - height: 16px; - } - // class to handle hover and active effect, since :hover is applied to the entire row - &.hover:not([disabled]) { - background-color: var(--fill-quinary); - } - &.active:not([disabled]) { - background-color: var(--fill-quarternary); - } - &[disabled] { - color: var(--color-gray-50); - } - } - } - // lighter hover and active effects on + buttons in selected rows - .row.selected { - .icon-action { - &.hover:not([disabled]) { - background-color: #ffffff1a; - } - &.active:not([disabled]) { - background-color: #ffffff33; - } - } - } - .row.highlighted { - border-radius: 0; - - } - .row.first-highlighted { - border-top-left-radius: 5px; - border-top-right-radius: 5px; - } - .row.last-highlighted { - border-bottom-left-radius: 5px; - border-bottom-right-radius: 5px; - } } } } diff --git a/scss/components/_virtualized-table.scss b/scss/components/_virtualized-table.scss index 4b6f703323..fa8ae176b2 100644 --- a/scss/components/_virtualized-table.scss +++ b/scss/components/_virtualized-table.scss @@ -371,12 +371,6 @@ text-overflow: ellipsis; overflow: hidden; max-height: 100%; - // TEMP: Disables tooltip display for truncated cells, but without this rule - // you cannot drag items before selecting them first because the ondrag event is - // not called if any children under the drag element are removed for which - // pointer-events are enabled, and we need to rerender the rows for selection - // before dragging. - pointer-events: none; } } @@ -428,3 +422,50 @@ } } } + +// Styling for clickable button-cells +.virtualized-table { + // the cell with the clickable button + .cell.clickable { + // make sure the button is centered + display: flex; + justify-content: center; + padding: 0; + + // the actual clickable button + .icon-action { + display: flex; + justify-content: center; + width: 20px; + height: 20px; + align-items: center; + border-radius: 6px; + .icon { + width: 16px; + height: 16px; + } + &:hover:not([disabled]) { + background-color: var(--fill-quinary); + } + &:active:not([disabled]) { + background-color: var(--fill-quarternary); + } + &[disabled] { + color: var(--color-gray-50); + } + } + } + + // lighter hover and active effects on clickable buttons in selected rows + .row.selected { + .icon-action { + &:hover:not([disabled]) { + background-color: #ffffff1a; + } + &:active:not([disabled]) { + background-color: #ffffff33; + } + } + } +} +