diff --git a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx index e0da739ef2..af642672d8 100644 --- a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx +++ b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx @@ -46,6 +46,12 @@ class TagList extends React.PureComponent { this.collectionRef = React.createRef(); this.scrollToTopOnNextUpdate = false; this.prevTagCount = 0; + this.focusedTagIndex = null; + this.lastFocusedTagIndex = null; + this.resolveTagsRenderedPromise = null; + this.state = { + scrollToCell: null + }; } componentDidUpdate(prevProps) { @@ -159,7 +165,7 @@ class TagList extends React.PureComponent { const { onDragOver, onDragExit, onDrop } = this.props.dragObserver; - var className = 'tag-selector-item'; + var className = 'tag-selector-item keyboard-clickable'; if (tag.selected) { className += ' selected'; } @@ -177,10 +183,13 @@ class TagList extends React.PureComponent { className, onClick: ev => !tag.disabled && this.props.onSelect(tag.name, ev), onContextMenu: ev => this.props.onTagContext(tag, ev), - onKeyDown: ev => this.props.onKeyDown(ev), onDragOver, onDragExit, onDrop, + onFocus: (_) => { + this.lastFocusedTagIndex = this.focusedTagIndex; + this.focusedTagIndex = index; + } }; props.style = { @@ -214,6 +223,84 @@ class TagList extends React.PureComponent { ); }; + + // Try to refocus a focused tag that was removed due to windowing + refocusTag() { + let tagsList = document.querySelector('.tag-selector-list'); + let tagsNodes = [...tagsList.querySelectorAll(".tag-selector-item")]; + let tagToFocus = this.props.tags[this.focusedTagIndex]; + let nodeToFocus = tagsNodes.find(node => node.textContent == tagToFocus.name); + if (nodeToFocus) { + nodeToFocus.focus(); + } + } + + waitForSectionRender() { + return new Promise((resolve, _) => { + this.resolveTagsRenderedPromise = resolve; + }); + } + + handleSectionRendered = ({ indices }) => { + let tagsList = document.querySelector('.tag-selector-list'); + // sets role="grid" which is not semantically correct + tagsList.setAttribute("role", "group"); + + if (this.focusedTagIndex === null) return; + // If the focused tag does not changed, the scrollToCell won't change + // either, so the won't scroll to the desired tag if we don't reset it. + // E.g. second arrowLeft keypress when first tag is focused won't scroll to it. + if (this.lastFocusedTagIndex === this.focusedTagIndex) { + this.setState({ scrollToCell: null }); + } + // Check if the tag that is supposed to be focused is within the rendered tags range. + // If it is, make sure it is focused. If it is not - focus the tags list. + if (indices.includes(this.focusedTagIndex)) { + this.refocusTag(); + if (this.resolveTagsRenderedPromise) { + this.resolveTagsRenderedPromise(); + } + } + else { + tagsList.focus(); + } + }; + + handleBlur = (event) => { + // If the focus leaves the tags list, clear the last focused tag index + let tagsList = document.querySelector('.tag-selector-list'); + if (!tagsList.contains(event.relatedTarget)) { + this.focusedTagIndex = null; + this.lastFocusedTagIndex = null; + } + }; + + async handleKeyDown(e) { + if (!["ArrowRight", "ArrowLeft"].includes(e.key)) return; + // If the windowing kicks in, the node of the initially-focused tag may not + // exist, so first we may need to scroll to it. + if (!document.activeElement.classList.contains("tag-selector-item")) { + this.setState({ scrollToCell: this.focusedTagIndex }); + // Even after the re-renders, the new tag nodes may not be rendered yet. + // So we have to wait for handleSectionRendered to run before proceeding. + await this.waitForSectionRender(); + } + // Sanity check to make sure that now a tag node is focused + if (!document.activeElement.classList.contains("tag-selector-item")) return; + // Handle arrow navigation + let nextTag = (node) => { + if (e.key == "ArrowRight") return node.nextElementSibling; + return node.previousElementSibling; + }; + let nextOne = nextTag(document.activeElement); + // Skip disabled tags + while (nextOne && nextOne.classList.contains("disabled")) { + nextOne = nextTag(nextOne); + } + if (nextOne) { + nextOne.focus(); + } + } render() { Zotero.debug("Rendering tag list"); @@ -252,12 +339,14 @@ class TagList extends React.PureComponent { width={this.props.width} height={this.props.height - filterBarHeight} aria-label={document.querySelector("#zotero-tag-selector").getAttribute("label") || ""} + onSectionRendered={this.handleSectionRendered} + scrollToCell={Number.isInteger(this.state.scrollToCell) ? this.state.scrollToCell : undefined} /> ); } return ( -
+
{tagList}
); diff --git a/chrome/content/zotero/containers/tagSelectorContainer.jsx b/chrome/content/zotero/containers/tagSelectorContainer.jsx index 729f099c52..1159d3936d 100644 --- a/chrome/content/zotero/containers/tagSelectorContainer.jsx +++ b/chrome/content/zotero/containers/tagSelectorContainer.jsx @@ -580,34 +580,6 @@ Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent { } } - handleKeyDown = (e) => { - if (["ArrowRight", "ArrowLeft"].includes(e.key)) { - let nextTag = (node) => { - if (e.key == "ArrowRight") return node.nextElementSibling; - return node.previousElementSibling; - }; - let nextOne = nextTag(e.target); - // Skip disabled tags - while (nextOne && nextOne.classList.contains("disabled")) { - nextOne = nextTag(nextOne); - } - if (nextOne) { - nextOne.focus(); - } - } - else if (e.key == "Tab" && !e.shiftKey) { - this.focusTextbox(); - e.preventDefault(); - } - else if (e.key == "Tab" && e.shiftKey) { - document.querySelector('.tag-selector-list').focus(); - e.preventDefault(); - } - if ([" ", "Enter"].includes(e.key)) { - e.target.click(); - } - } - handleSearch = (searchString) => { this.setState({searchString}); } diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 544c752754..795018660b 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -419,9 +419,12 @@ var ZoteroPane = new function() if (tagContainer.getAttribute('collapsed') == "true") { return document.getElementById('zotero-tb-add'); } - // If tag selector is collapsed, go to "New item" button, otherwise - // default to focusing on tag selector - return tagContainer.querySelector(".tag-selector-list"); + // If tag selector is collapsed, go to "New item" button, otherwise focus tag selector + let firstNonDisabledTag = tagSelector.querySelector('.tag-selector-item:not(.disabled)'); + if (firstNonDisabledTag) { + return firstNonDisabledTag; + } + return tagSelector.querySelector(".search-input"); }, Escape: clearCollectionSearch } @@ -452,20 +455,14 @@ var ZoteroPane = new function() }, 'tag-selector-item': { Tab: () => tagSelector.querySelector(".search-input"), - ShiftTab: () => tagSelector.querySelector(".tag-selector-list"), + ShiftTab: () => document.getElementById("collection-tree"), }, 'tag-selector-actions': { Tab: () => document.getElementById('zotero-tb-add'), ShiftTab: () => tagSelector.querySelector(".search-input") }, 'tag-selector-list': { - Tab: () => { - let firstNonDisabledTag = tagSelector.querySelector('.tag-selector-item:not(.disabled)'); - if (firstNonDisabledTag) { - return firstNonDisabledTag; - } - return tagSelector.querySelector(".search-input"); - }, + Tab: () => tagSelector.querySelector(".search-input"), ShiftTab: () => document.getElementById("collection-tree"), } }; diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 8aed0a6cfa..5235482171 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1534,7 +1534,6 @@ describe("ZoteroPane", function() { "tag-selector-actions", "search-input", "tag-selector-item", - "tag-selector-list", "collection-tree", "zotero-collections-search", "zotero-tb-collection-add",