fix tab not scrolling into view (#4404)

After react 18 update, the tab node may not yet be rendered
by tabBar.jsx when we try to scroll it into view in Zotero_Tabs.select.

- To make sure scrolling happens when rendering is done, move scroll-related
logic into a useEffect of tabBar.jxs. It also makes sure that we'll scroll to
a selected tab if it is moved via context menu to the very beginning or the end.

- Added a small scroll-padding to tabs container to make sure the border
does not get cutoff after the tab is scrolled into view instead of
JS code accounting for the border.

- Fixed a glitch where the pinned library tab would not get selected
on shift-tab from opened tabs menu.

Fixes: #4382
This commit is contained in:
abaevbog 2024-07-17 23:19:07 -07:00 committed by GitHub
parent 40fd5efe05
commit e6b5ba60dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 13 additions and 24 deletions

View file

@ -115,6 +115,13 @@ const TabBar = forwardRef(function (props, ref) {
};
}, []);
useEffect(() => {
// Scroll selected tab into view
let selectedTabNode = tabsInnerContainerRef.current.querySelector(".tab.selected");
if (!selectedTabNode || dragging) return;
selectedTabNode.scrollIntoView({ behavior: 'smooth' });
}, [tabs]);
useLayoutEffect(updateScrollArrows);
useLayoutEffect(updateOverflowing, [tabs]);
@ -414,6 +421,7 @@ TabBar.displayName = 'TabBar';
TabBar.propTypes = {
onTabSelect: PropTypes.func.isRequired,
onTabClose: PropTypes.func.isRequired,
onLoad: PropTypes.func.isRequired,
onTabMove: PropTypes.func.isRequired,
refocusReader: PropTypes.func.isRequired,
onContextMenu: PropTypes.func.isRequired,

View file

@ -493,24 +493,6 @@ var Zotero_Tabs = new function () {
tabNode.focus();
}
}
// Allow React to create a tab node
setTimeout(() => {
tabNode.scrollIntoView({ behavior: 'smooth' });
});
// Border is not included when scrolling element node into view, therefore we do it manually.
// TODO: `scroll-padding` since Firefox 68 can probably be used instead
setTimeout(() => {
if (!tabNode) {
return;
}
let tabsContainerNode = document.querySelector('#tab-bar-container .tabs');
if (tabNode.offsetLeft + tabNode.offsetWidth - tabsContainerNode.offsetWidth + 1 >= tabsContainerNode.scrollLeft) {
document.querySelector('#tab-bar-container .tabs').scrollLeft += 1;
}
else if (tabNode.offsetLeft - 1 <= tabsContainerNode.scrollLeft) {
document.querySelector('#tab-bar-container .tabs').scrollLeft -= 1;
}
}, 500);
tab.timeSelected = Zotero.Date.getUnixTimestamp();
// Without `setTimeout` the tab closing that happens in `unloadUnusedTabs` results in
// tabs deck selection index bigger than the deck children count. It feels like something
@ -633,14 +615,12 @@ var Zotero_Tabs = new function () {
if (tabIndexToFocus !== null) {
const nextTab = this._tabs[tabIndexToFocus];
// There may be duplicate tabs - in normal tab array and in pinned tabs
// So to get the right one, fetch all tabs with a given id and filter out one
// that's visible
// Go through all candidates and try to focus the visible one
let candidates = document.querySelectorAll(`[data-id="${nextTab.id}"]`);
for (let node of candidates) {
if (node.offsetParent) {
node.focus();
return;
}
node.focus();
// Visible tab was found and focused
if (document.activeElement == node) return;
}
}
};

View file

@ -147,6 +147,7 @@
padding: 4px 1px;
column-gap: 4px;
-moz-window-dragging: drag;
scroll-padding: 2px; // ensures a tab is scrolled into view without cutoff borders
.pinned-tabs & {
padding: 4px 0;