From a73035c848668d2c03a7e81a7897e2a67e5b7b2d Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Wed, 12 Jun 2024 00:26:42 -0400 Subject: [PATCH] vpat 66-68: fix focus within tagsBox popup (#4231) - focus will always enter the tagsBox popup opened from the reader - escape from within the popup with tagsBox will close the popup (vpat 67) - one should not be able to collapse the tagsBox. Added `collapsible` getter and setters to the collapsible panel to prevent the section from changing its open status. This may be also used in other cases, such as to prevent itemBox from being collapsed in duplicates mode. - patched a glitch where tab from the last empty tab input would loose focus. - changed the `menupopup` for `panel` to display the `tagsbox` because `menupopup` has implicit `role="menu"` which is not meant to contain inputs, so voiceover completely looses cursor when inputs are focused inside of the popup. Also, tweaked spacing a bit to avoid the focus-ring getting cutoff. Addresses: #4222 Fixes: #4230 Fixes: #4226 Addresses: #4388 --- .../zotero/elements/collapsibleSection.js | 15 ++++++++- .../zotero/elements/itemPaneSection.js | 8 +++++ chrome/content/zotero/elements/tagsBox.js | 12 +++++++ chrome/content/zotero/xpcom/reader.js | 33 +++++++++++++------ scss/components/_reader.scss | 11 ++----- 5 files changed, 60 insertions(+), 19 deletions(-) diff --git a/chrome/content/zotero/elements/collapsibleSection.js b/chrome/content/zotero/elements/collapsibleSection.js index 015e7e7bda..d2ffcae73b 100644 --- a/chrome/content/zotero/elements/collapsibleSection.js +++ b/chrome/content/zotero/elements/collapsibleSection.js @@ -43,7 +43,7 @@ set open(newOpen) { newOpen = !!newOpen; let oldOpen = this.open; - if (oldOpen === newOpen || this.empty) return; + if (oldOpen === newOpen || this.empty || !this.collapsible) return; this.render(); // Force open before getting scrollHeight, so we get the right value @@ -106,6 +106,19 @@ this.setAttribute('summary', val); } + get collapsible() { + return !this.getAttribute("no-collapse"); + } + + set collapsible(val) { + if (val) { + this.removeAttribute('no-collapse'); + } + else { + this.setAttribute('no-collapse', val); + } + } + static get observedAttributes() { return ['open', 'empty', 'label', 'summary', 'extra-buttons']; } diff --git a/chrome/content/zotero/elements/itemPaneSection.js b/chrome/content/zotero/elements/itemPaneSection.js index d5651ffef7..1cf01e442a 100644 --- a/chrome/content/zotero/elements/itemPaneSection.js +++ b/chrome/content/zotero/elements/itemPaneSection.js @@ -84,6 +84,14 @@ class ItemPaneSectionElementBase extends XULElementBase { this._section.open = val; } } + + get collapsible() { + return this._section.collapsible; + } + + set collapsible(val) { + this._section.collapsible = !!val; + } connectedCallback() { super.connectedCallback(); diff --git a/chrome/content/zotero/elements/tagsBox.js b/chrome/content/zotero/elements/tagsBox.js index c5579ebec3..892697d487 100644 --- a/chrome/content/zotero/elements/tagsBox.js +++ b/chrome/content/zotero/elements/tagsBox.js @@ -342,6 +342,18 @@ focusField.focus(); } } + else if (event.key == "Tab" && !event.shiftKey) { + // On tab from the last empty tag row, the minus icon will be focused + // and the row will be immediately removed in this.saveTag, so focus will be lost. + // To avoid that, on tab from the last tag input that is empty, focus the next + // element after the tag row. + let allTags = [...this.querySelectorAll(".row")]; + let isLastTag = target.closest(".row") == allTags[allTags.length - 1]; + if (isLastTag && !target.closest("editable-text").value.length) { + Services.focus.moveFocus(window, target.closest(".row").lastChild, Services.focus.MOVEFOCUS_FORWARD, 0); + event.preventDefault(); + } + } }; // Intercept paste, check for newlines, and convert textbox diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index f1317910a2..c14655ed1e 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -896,30 +896,43 @@ class ReaderInstance { } _openTagsPopup(item, x, y) { - let menupopup = this._window.document.createXULElement('menupopup'); - menupopup.addEventListener('popuphidden', function (event) { - if (event.target === menupopup) { - menupopup.remove(); + let tagsPopup = this._window.document.createXULElement('panel'); + tagsPopup.addEventListener('popuphidden', function (event) { + if (event.target === tagsPopup) { + tagsPopup.remove(); } }); - menupopup.className = 'tags-popup'; - menupopup.setAttribute('ignorekeys', true); + tagsPopup.addEventListener('keydown', function (event) { + if (event.key == "Escape") { + tagsPopup.hidePopup(); + } + }); + tagsPopup.className = 'tags-popup'; let tagsbox = this._window.document.createXULElement('tags-box'); - menupopup.appendChild(tagsbox); + tagsPopup.appendChild(tagsbox); tagsbox.setAttribute('flex', '1'); - this._popupset.appendChild(menupopup); + this._popupset.appendChild(tagsPopup); let rect = this._iframe.getBoundingClientRect(); x += rect.left; y += rect.top; tagsbox.editable = true; tagsbox.item = item; tagsbox.render(); - menupopup.openPopup(null, 'before_start', x, y, true); - setTimeout(() => { + // remove unnecessary tabstop from the section header + tagsbox.querySelector(".head").removeAttribute("tabindex"); + tagsPopup.addEventListener("popupshown", (_) => { + // Ensure tagsbox is open + tagsbox.open = true; if (tagsbox.count == 0) { tagsbox.newTag(); } + else { + // Focus + button + Services.focus.setFocus(tagsbox.querySelector("toolbarbutton"), Services.focus.FLAG_NOSHOWRING); + } + tagsbox.collapsible = false; }); + tagsPopup.openPopup(null, 'before_start', x, y, true); } async _openContextMenu({ x, y, itemGroups }) { diff --git a/scss/components/_reader.scss b/scss/components/_reader.scss index 0ab71eedab..e853871b5e 100644 --- a/scss/components/_reader.scss +++ b/scss/components/_reader.scss @@ -1,12 +1,7 @@ -menupopup.tags-popup { +.tags-popup { font: inherit; min-width: 300px; - padding-inline: 8px; - - @media (-moz-platform: windows) { - & > tags-box { - // padding-inline does not work on Windows - margin-inline: 8px; - } + & > tags-box { + padding-inline: 8px; } }