From c7c3784413a04a1a0a94b1742550b91c4b6068fb Mon Sep 17 00:00:00 2001 From: abaevbog Date: Sun, 14 Apr 2024 02:16:16 -0400 Subject: [PATCH] fix focus test breakage after zotero@6859614 (#3977) - edits to zoteroPaneTest.js focus tests to expect the updated focus sequence: selected tab -> tabs menu -> sync button -> collectionTree toolbar -> collectionTree -> tags selector -> itemTree toolbar - updated tags selector keydown handling to explicitly handle all tab/shift-tab events using moveFocus. It is more readable and explicit focus handling for all components is required for programmic tab/shiftTab events dispatched in tests to actually move focus Fixes: #3975 --- chrome/content/zotero/components/search.jsx | 1 + chrome/content/zotero/zoteroPane.js | 55 +++++++++-------- test/tests/zoteroPaneTest.js | 66 +++++++++++++-------- 3 files changed, 73 insertions(+), 49 deletions(-) diff --git a/chrome/content/zotero/components/search.jsx b/chrome/content/zotero/components/search.jsx index 4b3718f614..daddbddd2b 100644 --- a/chrome/content/zotero/components/search.jsx +++ b/chrome/content/zotero/components/search.jsx @@ -92,6 +92,7 @@ class Search extends React.PureComponent { onChange={this.handleChange} onKeyDown={this.handleKeyDown} value={this.state.immediateValue} + className="search-input" {...pick(this.props, p => p.startsWith('data-') || p.startsWith('aria-'))} /> {this.state.immediateValue !== '' diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index aeecb95bd0..9c6f0a392c 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -414,7 +414,7 @@ var ZoteroPane = new function() } // If tag selector is collapsed, go to "New item" button, otherwise // default to focusing on tag selector - return false; + return tagContainer.querySelector(".tag-selector-list"); }, Escape: clearCollectionSearch } @@ -432,30 +432,37 @@ var ZoteroPane = new function() }); tagSelector.addEventListener("keydown", (e) => { - // Tab from the scrollable tag list or Shift-Tab from the input field focuses the first - // non-disabled tag. If there are none, the tags are skipped - if ((e.target.classList.contains("tag-selector-list") && e.key == "Tab" && !e.shiftKey) - || e.target.tagName == "input" && e.key == "Tab" && e.shiftKey) { - let firstNonDisabledTag = document.querySelector('.tag-selector-item:not(.disabled)'); - if (firstNonDisabledTag) { - firstNonDisabledTag.focus(); + let actionsMap = { + 'search-input': { + Tab: () => tagSelector.querySelector('.tag-selector-actions'), + ShiftTab: () => { + let firstNonDisabledTag = tagSelector.querySelector('.tag-selector-item:not(.disabled)'); + if (firstNonDisabledTag) { + return firstNonDisabledTag; + } + return document.getElementById("collection-tree"); + }, + }, + 'tag-selector-item': { + Tab: () => tagSelector.querySelector(".search-input"), + ShiftTab: () => tagSelector.querySelector(".tag-selector-list"), + }, + '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"); + }, + ShiftTab: () => document.getElementById("collection-tree"), } - else if (e.target.classList.contains("tag-selector-list")) { - tagSelector.querySelector("input").focus(); - } - else { - tagSelector.querySelector(".tag-selector-list").focus(); - } - - e.preventDefault(); - e.stopPropagation(); - } - // Special treatment for tag selector button because it has no id - if (e.target.tagName == "button" && e.key == "Tab" && !e.shiftKey) { - document.getElementById('zotero-tb-add').focus(); - e.preventDefault(); - e.stopPropagation(); - } + }; + moveFocus(actionsMap, e); }); } diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index ba02aa3232..8aed0a6cfa 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1495,7 +1495,15 @@ describe("ZoteroPane", function() { var collection = new Zotero.Collection; collection.name = "Focus Test"; await collection.saveTx(); + // Make sure there is a tag + var item = new Zotero.Item('newspaperArticle'); + item.setCollections([collection.id]); + await item.setTags(["Tag"]); + await item.saveTx({ + skipSelect: true + }); await waitForItemsLoad(win); + await zp.collectionsView.selectLibrary(userLibraryID); }); var tab = new KeyboardEvent('keydown', { @@ -1519,27 +1527,38 @@ describe("ZoteroPane", function() { bubbles: true }); - // TEMP: https://github.com/zotero/zotero/issues/3975 - it.skip("should shift-tab through the toolbar to item-tree", async function () { + // Focus sequence for Zotero Pane + let sequence = [ + "zotero-tb-search-dropmarker", + "zotero-tb-add", + "tag-selector-actions", + "search-input", + "tag-selector-item", + "tag-selector-list", + "collection-tree", + "zotero-collections-search", + "zotero-tb-collection-add", + "zotero-tb-sync", + "zotero-tb-tabs-menu" + ]; + it("should shift-tab across the zotero pane", async function () { let searchBox = doc.getElementById('zotero-tb-search-textbox'); searchBox.focus(); - let sequence = [ - "zotero-tb-search-dropmarker", - "zotero-tb-add", - "zotero-collections-search", - "zotero-tb-collection-add", - "zotero-tb-sync", - "zotero-tb-tabs-menu" - ]; - for (let id of sequence) { doc.activeElement.dispatchEvent(shiftTab); // Wait for collection search to be revealed if (id === "zotero-collections-search") { await Zotero.Promise.delay(250); } - assert.equal(doc.activeElement.id, id); + // Some elements don't have id, so use classes to verify they're focused + if (doc.activeElement.id) { + assert.equal(doc.activeElement.id, id); + } + else { + let clases = [...doc.activeElement.classList]; + assert.include(clases, id); + } // Wait for collection search to be hidden for subsequent tests if (id === "zotero-tb-collection-add") { await Zotero.Promise.delay(50); @@ -1552,26 +1571,23 @@ describe("ZoteroPane", function() { assert.equal(doc.activeElement.id, "item-tree-main-default"); }); - // TEMP: https://github.com/zotero/zotero/issues/3975 - it.skip("should tab through the toolbar to collection-tree", async function () { + it("should tab across the zotero pane", async function () { win.Zotero_Tabs.moveFocus("current"); - let sequence = [ - "zotero-tb-tabs-menu", - "zotero-tb-sync", - "zotero-tb-collection-add", - "zotero-collections-search", - "zotero-tb-add", - "zotero-tb-search-dropmarker", - 'zotero-tb-search-textbox', - 'collection-tree', - ]; + sequence.reverse(); for (let id of sequence) { doc.activeElement.dispatchEvent(tab); // Wait for collection search to be revealed if (id === "zotero-collections-search") { await Zotero.Promise.delay(250); } - assert.equal(doc.activeElement.id, id); + // Some elements don't have id, so use classes to verify they're focused + if (doc.activeElement.id) { + assert.equal(doc.activeElement.id, id); + } + else { + let clases = [...doc.activeElement.classList]; + assert.include(clases, id); + } } });