From ceb1dd7da3b0e1efdbc824506f65a5c500a909cf Mon Sep 17 00:00:00 2001
From: abaevbog <bogdan@zotero.org>
Date: Tue, 6 Feb 2024 12:51:06 -0500
Subject: [PATCH] additions to keyboard nav for toolbar buttons and reader
 (#3602)

- handle space and enter in zotero pane to always click on toolbarbutton.
It ensures consistent behavior across all components to avoid Enter
triggering click on some buttons but not others.
- address the issue of focus not being able to leave the reader once it
gets there. On Escape, when reader is opened, instead of re-focusing the reader,
focus the selected tab. From there, Enter will focus the reader, and tabbing
allows you to reach the itemPane.
- Tab/Shift-tab moves focus between Sync button and itemPane when
reader is opened
---
 chrome/content/zotero/contextPane.js   |  6 ++--
 chrome/content/zotero/itemPane.js      | 17 +++++----
 chrome/content/zotero/zoteroPane.js    | 48 +++++++++++++++++---------
 chrome/content/zotero/zoteroPane.xhtml |  2 +-
 scss/elements/_paneHeader.scss         |  3 ++
 5 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/chrome/content/zotero/contextPane.js b/chrome/content/zotero/contextPane.js
index 8b7bd1d048..d32f0e3d50 100644
--- a/chrome/content/zotero/contextPane.js
+++ b/chrome/content/zotero/contextPane.js
@@ -277,8 +277,9 @@ var ZoteroContextPane = new function () {
 
 		if (splitter.getAttribute('state') != 'collapsed') {
 			if (_panesDeck.selectedIndex == 0) {
-				var node = _itemPaneDeck.selectedPanel;
-				node.focus();
+				// Focus the title in the header
+				var header = _itemPaneDeck.selectedPanel.querySelector("pane-header editable-text");
+				header.focus();
 				return true;
 			}
 			else {
@@ -873,6 +874,7 @@ var ZoteroContextPane = new function () {
 		// div
 		var div = document.createElement('div');
 		div.className = 'zotero-view-item';
+		div.setAttribute("tabindex", "0");
 		main.append(div);
 		
 		// Info
diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js
index 4ecc9085c7..de35e936c9 100644
--- a/chrome/content/zotero/itemPane.js
+++ b/chrome/content/zotero/itemPane.js
@@ -180,6 +180,16 @@ var ZoteroItemPane = new function() {
 		let sidenav = document.getElementById(
 			isLibraryTab ? 'zotero-view-item-sidenav' : 'zotero-context-pane-sidenav'
 		);
+
+		// Shift-tab from title when reader is opened focuses the last button in tabs toolbar
+		if (event.target.closest(".title") && event.key == "Tab"
+			&& event.shiftKey && Zotero_Tabs.selectedType == "reader") {
+			let focusable = [...document.querySelectorAll("#zotero-tabs-toolbar toolbarbutton:not([disabled]):not([hidden])")];
+			let btn = focusable[focusable.length - 1];
+			btn.focus();
+			stopEvent();
+			return;
+		}
 		// Tab from the scrollable area focuses the pinned pane if it exists
 		if (event.target.classList.contains("zotero-view-item") && event.key == "Tab" && !event.shiftKey && sidenav.pinnedPane) {
 			let pane = sidenav.getPane(sidenav.pinnedPane);
@@ -187,13 +197,6 @@ var ZoteroItemPane = new function() {
 			stopEvent();
 			return;
 		}
-		// Space or Enter on a button or 'keyboard-clickable' triggers a click
-		if ([" ", "Enter"].includes(event.key)
-			&& (event.target.tagName == "toolbarbutton"
-				|| event.target.classList.contains("keyboard-clickable"))) {
-			event.target.click();
-			stopEvent();
-		}
 		// Tab tavigation between entries and buttons within library, related and notes boxes
 		if (event.key == "Tab" && event.target.closest(".box")) {
 			let next = null;
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index c344784565..4b28d2456e 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -244,6 +244,10 @@ var ZoteroPane = new function()
 					ArrowRight: () => null,
 					ArrowLeft: () => null,
 					Tab: () => {
+						if (Zotero_Tabs.selectedIndex > 0) {
+							ZoteroContextPane.focus();
+							return null;
+						}
 						if (collectionsPane.getAttribute("collapsed")) {
 							return document.getElementById('zotero-tb-add');
 						}
@@ -313,6 +317,9 @@ var ZoteroPane = new function()
 					ArrowLeft: () => null,
 					Tab: () => document.getElementById('zotero-tb-collections-search').click(),
 					ShiftTab: () => document.getElementById('zotero-tb-sync')
+					// Enter: () => {
+					// 	document.getElementById('zotero-tb-collection-add').click();
+					// }
 				},
 				'zotero-collections-search': {
 					Tab: () => document.getElementById('zotero-tb-add'),
@@ -349,12 +356,6 @@ var ZoteroPane = new function()
 						}
 						document.getElementById('zotero-tb-collections-search').click();
 						return null;
-					},
-					' ': () => {
-						document.getElementById('zotero-tb-add').open = true;
-					},
-					Enter: () => {
-						document.getElementById('zotero-tb-add').open = true;
 					}
 				},
 				'zotero-tb-lookup': {
@@ -855,18 +856,17 @@ var ZoteroPane = new function()
 	function handleKeyDown(event, from) {
 		if (Zotero_Tabs.selectedIndex > 0) {
 			if (event.key === 'Escape') {
-				if (!document.activeElement.classList.contains('reader')) {
-					let reader = Zotero.Reader.getByTabID(Zotero_Tabs.selectedID);
-					if (reader) {
-						reader.focus();
-						// Keep propagating if current focus is on input or textarea
-						// The Escape event needs to be handled by itemBox, tagBox, etc. to undo edits.
-						if (!["input", "textarea"].includes(document.activeElement.tagName)) {
-							event.preventDefault();
-							event.stopPropagation();
-						}
-					}
+				// If focus is on an opened popup, let Escape just close it
+				// Also, do nothing if focus is inside of tabs toolbar
+				if (document.activeElement.open
+					|| document.querySelector("#zotero-tabs-toolbar").contains(document.activeElement)) {
+					return;
 				}
+				// Escape when a reader tab is opened re-focuses the tab in the tab bar
+				// Timeout to let the focused editable-text reset the value
+				setTimeout(() => {
+					Zotero_Tabs.moveFocus("current");
+				});
 			}
 			else if (event.key === 'Tab' && event.shiftKey) {
 				let node = document.activeElement;
@@ -1014,6 +1014,20 @@ var ZoteroPane = new function()
 			}
 		}
 		
+		let tgt = event.target;
+		if ([" ", "Enter"].includes(event.key)
+			&& (["button", "toolbarbutton"].includes(tgt.tagName)
+				|| tgt.classList.contains("keyboard-clickable"))) {
+			if (event.target.querySelector("menupopup")) {
+				event.target.open = true;
+			}
+			else {
+				event.target.click();
+			}
+			event.preventDefault();
+			event.stopPropagation();
+			return;
+		}
 		try {
 			// Ignore keystrokes outside of Zotero pane
 			if (!(event.originalTarget.ownerDocument instanceof HTMLDocument)) {
diff --git a/chrome/content/zotero/zoteroPane.xhtml b/chrome/content/zotero/zoteroPane.xhtml
index 387e19d214..85dd01e72e 100644
--- a/chrome/content/zotero/zoteroPane.xhtml
+++ b/chrome/content/zotero/zoteroPane.xhtml
@@ -1233,7 +1233,7 @@
 												<html:div class="zotero-view-item-main">
 													<pane-header id="zotero-item-pane-header" />
 													
-													<html:div id="zotero-view-item" class="zotero-view-item">
+													<html:div id="zotero-view-item" class="zotero-view-item" tabindex="0">
 														<item-box id="zotero-editpane-item-box" data-pane="info"/>
 														
 														<abstract-box id="zotero-editpane-abstract" class="zotero-editpane-abstract" data-pane="abstract"/>
diff --git a/scss/elements/_paneHeader.scss b/scss/elements/_paneHeader.scss
index 98b43e9c1e..60f2b6fba8 100644
--- a/scss/elements/_paneHeader.scss
+++ b/scss/elements/_paneHeader.scss
@@ -32,5 +32,8 @@ pane-header {
 	
 	.menu-button toolbarbutton {
 		@include svgicon-menu("go-to", "universal", "20");
+		--radius-focus-border: 5px;
+		--width-focus-border: 2px;
+		@include focus-ring;
 	}
 }