From 2835d6fe83fb750e74886e05edada14bbb2e0012 Mon Sep 17 00:00:00 2001 From: abaevbog Date: Thu, 18 Apr 2024 01:39:36 -0400 Subject: [PATCH] tag selector focus edits, fix windowing breakage (#3984) * tag selector focus edits - no tabstop on the tag selector scrollable area - change tag selector's role from default "grid" to "group". "grid" is not quite correct semantically and leads to voiceover suggesting irrelevant commands. - move all keyboard handling logic to tagSelectorList.jsx. Tabbing through tag selector is now handled in ZoteroPane, so the only logic left there is arrow navigation between tags, and there's no reason to not have it together with the tags list. - a workaround to deal with focused tags when windowing kicks in. When a tag is focused, record its index. Each time tags are re-rendered, if the saved index is not among rendered tags, refocus it, otherwise, move focus to the tags list. --- .../tagSelector/tagSelectorList.jsx | 95 ++++++++++++++++++- .../containers/tagSelectorContainer.jsx | 28 ------ chrome/content/zotero/zoteroPane.js | 19 ++-- test/tests/zoteroPaneTest.js | 1 - 4 files changed, 100 insertions(+), 43 deletions(-) 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",