From 2f4a232c41aa3955bba093828b71b9aadc44a21d Mon Sep 17 00:00:00 2001 From: abaevbog Date: Fri, 29 Mar 2024 03:57:36 -0400 Subject: [PATCH] fx115: fix tooltips for tabs, tabs menu and virtualized table (#3877) Fix and improve tooltip logic - fx115: move main html-tooltip outside of the deck, otherwise it only shows up in the library tab. - Zotero_Tooltip is not required in the tabs bar and the button of the tag selector. Setting the tooltip attribute on the closest XUL parent and adding title attribute makes the tooltip properly appear. - Remove manual handling of the hover effect from tabs manu. The hover effect doesn't stick around after drag-drop with fx115 anymore, so it's not required. And then the usual title attribute works for the tooltip. - Set title instead of tooltiptext attribute in tagSelectorList so that the tooltip appears After zotero@8e2790e, the pointer effects don't fire on the actual cells of the table (only on the parent row), so the tooltips do not appear for the cells. This is a (hopefully) temporary solution to handle mousemove events over the row of the table, find the right cell that the mouse is over, and use Zotero_Tooltip to manually display the tooltip. Tweaked Zotero_Tooltip to create the fake tooltip and place it into the DOM. Zotero_Tooltip is also imported by virtualized-table as a module because one shouldn't have to load it in .xhtml file for every new window where the virtualized-table is used. --- chrome/content/zotero/components/button.jsx | 13 ---- chrome/content/zotero/components/tabBar.jsx | 17 +----- .../tagSelector/tagSelectorList.jsx | 4 +- .../zotero/{ => components}/tooltip.js | 14 ++++- .../zotero/components/virtualized-table.jsx | 59 +++++++++++++++++++ chrome/content/zotero/tabs.js | 24 +------- chrome/content/zotero/zoteroPane.xhtml | 6 +- chrome/locale/en-US/zotero/zotero.ftl | 2 +- scss/components/_tabsMenu.scss | 5 +- 9 files changed, 79 insertions(+), 65 deletions(-) rename chrome/content/zotero/{ => components}/tooltip.js (84%) diff --git a/chrome/content/zotero/components/button.jsx b/chrome/content/zotero/components/button.jsx index 1782f0bdfa..318b436c3f 100644 --- a/chrome/content/zotero/components/button.jsx +++ b/chrome/content/zotero/components/button.jsx @@ -89,22 +89,9 @@ class Button extends PureComponent { if (!this.props.isDisabled) { attr.onMouseDown = (event) => { - // Hide tooltip on mousedown - if (this.title) { - window.Zotero_Tooltip.stop(); - } return this.handleMouseDown(event); }; attr.onClick = this.handleClick - // Fake tooltip behavior as long as 'title' doesn't work for HTML-in-XUL elements - if (this.title) { - attr.onMouseOver = () => { - window.Zotero_Tooltip.start(this.title); - }; - attr.onMouseOut = () => { - window.Zotero_Tooltip.stop(); - }; - } } return attr diff --git a/chrome/content/zotero/components/tabBar.jsx b/chrome/content/zotero/components/tabBar.jsx index 44aba9f848..ad0682cedf 100644 --- a/chrome/content/zotero/components/tabBar.jsx +++ b/chrome/content/zotero/components/tabBar.jsx @@ -225,18 +225,6 @@ const TabBar = forwardRef(function (props, ref) { event.stopPropagation(); } - function handleTabMouseMove(title) { - // Fix `title` not working for HTML-in-XUL. Using `mousemove` ensures we restart the tooltip - // after just a small movement even when the active tab has changed under the cursor, which - // matches behavior in Firefox. - window.Zotero_Tooltip.start(title); - } - - function handleTabBarMouseOut() { - // Hide any possibly open `title` tooltips when mousing out of any tab or the tab bar as a - // whole. `mouseout` bubbles up from element you moved out of, so it covers both cases. - window.Zotero_Tooltip.stop(); - } function handleWheel(event) { // Normalize wheel speed @@ -280,7 +268,6 @@ const TabBar = forwardRef(function (props, ref) { data-id={id} className={cx('tab', { selected, dragging: dragging && id === dragIDRef.current })} draggable={true} - onMouseMove={() => handleTabMouseMove(title)} onMouseDown={(event) => handleTabMouseDown(event, id)} onClick={(event) => handleTabClick(event, id)} onAuxClick={(event) => handleTabClick(event, id)} @@ -289,7 +276,7 @@ const TabBar = forwardRef(function (props, ref) { tabIndex="-1" > {icon} -
{title}
+
{title}
handleTabClose(event, id)} @@ -310,7 +297,6 @@ const TabBar = forwardRef(function (props, ref) {
{tabs.length ? renderTab(tabs[0], 0) : null}
@@ -332,7 +318,6 @@ const TabBar = forwardRef(function (props, ref) { ref={tabsRef} className="tabs" onDragOver={handleTabBarDragOver} - onMouseOut={handleTabBarMouseOut} onScroll={updateScrollArrows} dir={Zotero.dir} > diff --git a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx index 92767475a8..f02af93b6e 100644 --- a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx +++ b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx @@ -196,10 +196,8 @@ class TagList extends React.PureComponent { delete props.style.width; } else { - // Setting this via props doesn't seem to work in XUL, but setting it on hover does. - // Hopefully in an HTML window we'll be able to just set 'title'. props.onMouseOver = function (event) { - event.target.setAttribute('tooltiptext', tag.name); + event.target.setAttribute('title', tag.name); }; } diff --git a/chrome/content/zotero/tooltip.js b/chrome/content/zotero/components/tooltip.js similarity index 84% rename from chrome/content/zotero/tooltip.js rename to chrome/content/zotero/components/tooltip.js index 9ad98c94ba..b58131fd40 100644 --- a/chrome/content/zotero/tooltip.js +++ b/chrome/content/zotero/components/tooltip.js @@ -67,12 +67,24 @@ var Zotero_Tooltip = new function () { function handleMouseStop() { var tooltipElem = document.getElementById('fake-tooltip'); + // Create the fake tooltip if it does not exist + if (!tooltipElem) { + tooltipElem = document.createXULElement("tooltip"); + tooltipElem.id = "fake-tooltip"; + // The tooltip location is important. If the tooltip is placed + // within a lower level component, it may be not visible + document.documentElement.appendChild(tooltipElem); + } tooltipElem.setAttribute('label', text); tooltipElem.openPopupAtScreen(x, y, false, null); } function hidePopup() { var tooltipElem = document.getElementById('fake-tooltip'); - tooltipElem.hidePopup(); + if (tooltipElem) { + tooltipElem.hidePopup(); + } } }; + +export { Zotero_Tooltip }; diff --git a/chrome/content/zotero/components/virtualized-table.jsx b/chrome/content/zotero/components/virtualized-table.jsx index f7b7dac7d0..22c26d4e45 100644 --- a/chrome/content/zotero/components/virtualized-table.jsx +++ b/chrome/content/zotero/components/virtualized-table.jsx @@ -31,6 +31,7 @@ const cx = require('classnames'); const WindowedList = require('./windowed-list'); const Draggable = require('./draggable'); const { CSSIcon, getDOMElement } = require('components/icons'); +const { Zotero_Tooltip } = require('./tooltip'); const TYPING_TIMEOUT = 1000; const MINIMUM_ROW_HEIGHT = 20; // px @@ -909,6 +910,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 textElem = elem.querySelector('.label, .cell-text'); @@ -929,6 +933,59 @@ 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(); @@ -1193,6 +1250,8 @@ 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", { resizing: this.state.resizing, 'multi-select': this.props.multiSelect diff --git a/chrome/content/zotero/tabs.js b/chrome/content/zotero/tabs.js index 7ff330550b..2868aff7fd 100644 --- a/chrome/content/zotero/tabs.js +++ b/chrome/content/zotero/tabs.js @@ -128,8 +128,6 @@ var Zotero_Tabs = new function () { return; } document.title = (tab.title.length ? tab.title + ' - ' : '') + Zotero.appName; - // Hide any tab `title` tooltips that might be open - window.Zotero_Tooltip.stop(); if (this.isTabsMenuVisible()) { this.refreshTabsMenuList(); if (document.activeElement.id !== "zotero-tabs-menu-filter") { @@ -639,7 +637,6 @@ var Zotero_Tabs = new function () { this._openMenu = function (x, y, id) { var { tab, tabIndex } = this._getTab(id); - window.Zotero_Tooltip.stop(); let menuitem; let popup = document.createXULElement('menupopup'); document.querySelector('popupset').appendChild(popup); @@ -851,7 +848,7 @@ var Zotero_Tabs = new function () { tabName.setAttribute('class', 'zotero-tabs-menu-entry title'); tabName.setAttribute('tabindex', `${index++}`); tabName.setAttribute('aria-label', tab.title); - tabName.setAttribute('tooltiptext', tab.title); + tabName.setAttribute('title', tab.title); // Cross button to close a tab let closeButton = document.createElement('div'); @@ -907,25 +904,6 @@ var Zotero_Tabs = new function () { this.tabsMenuPanel.hidePopup(); this.select(tab.id); }); - // Manually handle hover effects as a workaround for a likely mozilla bug that - // keeps :hover at the location of dragstart after drop. - for (let node of [tabName, closeButton]) { - node.addEventListener('mouseenter', (_) => { - if (this._tabsMenuIgnoreMouseover) { - return; - } - node.classList.add('hover'); - // If the mouse moves over a tab, send focus back to the panel - // to avoid having two fields that appear greyed out. - if (document.activeElement.id !== "zotero-tabs-menu-filter") { - this._tabsMenuFocusedIndex = -1; - this.tabsMenuPanel.focus(); - } - }); - node.addEventListener('mouseleave', (_) => { - node.classList.remove('hover'); - }); - } row.appendChild(tabName); row.appendChild(closeButton); diff --git a/chrome/content/zotero/zoteroPane.xhtml b/chrome/content/zotero/zoteroPane.xhtml index 93c6a1d316..5720f8ff89 100644 --- a/chrome/content/zotero/zoteroPane.xhtml +++ b/chrome/content/zotero/zoteroPane.xhtml @@ -85,7 +85,6 @@ Services.scriptloader.loadSubScript("chrome://zotero/content/lookup.js", this); Services.scriptloader.loadSubScript("chrome://zotero/content/locateMenu.js", this); Services.scriptloader.loadSubScript("chrome://zotero/content/menuAccessKey.js", this); - Services.scriptloader.loadSubScript("chrome://zotero/content/tooltip.js", this); Services.scriptloader.loadSubScript("chrome://zotero/content/containers/tagSelectorContainer.js", this); @@ -837,7 +836,7 @@ tabindex="0" data-l10n-id="zotero-tabs-menu-filter" /> - + @@ -876,6 +875,7 @@