From ed8e3f142bbee84786e0adeb2d7229aa3ac303b0 Mon Sep 17 00:00:00 2001
From: abaevbog <bogdan@zotero.org>
Date: Wed, 24 Jan 2024 03:01:03 -0500
Subject: [PATCH] itempane keyboard navigation

- right/left arrows on section header expand/collapse the sections
- space/enter on section header toggle section collapsed/expanded
- tab from the header goes through the header's buttons and then
tabs into the section if that's opened
- arrow up/down on the header jumps to the previous/next header
- space/enter on clickable elements simulate a click
- if there is a pinned section, tab from the title will focus its header
---
 chrome/content/zotero/contextPane.js          |  2 +
 .../content/zotero/elements/attachmentBox.js  | 43 +++++++++-
 .../zotero/elements/attachmentPreview.js      | 34 ++++++++
 .../content/zotero/elements/attachmentRow.js  |  2 +-
 .../content/zotero/elements/attachmentsBox.js |  2 +-
 .../zotero/elements/collapsibleSection.js     | 84 +++++++++++++++++--
 chrome/content/zotero/elements/itemBox.js     | 11 ++-
 .../elements/librariesCollectionsBox.js       |  4 +-
 chrome/content/zotero/elements/notesBox.js    |  4 +-
 chrome/content/zotero/elements/relatedBox.js  |  4 +-
 chrome/content/zotero/itemPane.js             | 45 ++++++++++
 scss/elements/_attachmentBox.scss             |  4 +-
 scss/elements/_attachmentPreview.scss         |  1 +
 scss/elements/_collapsibleSection.scss        | 11 ++-
 14 files changed, 228 insertions(+), 23 deletions(-)

diff --git a/chrome/content/zotero/contextPane.js b/chrome/content/zotero/contextPane.js
index 1ad34a7a5e..4d833cf3c4 100644
--- a/chrome/content/zotero/contextPane.js
+++ b/chrome/content/zotero/contextPane.js
@@ -80,10 +80,12 @@ var ZoteroContextPane = new function () {
 		window.addEventListener('resize', _update);
 		Zotero.Reader.onChangeSidebarWidth = _updatePaneWidth;
 		Zotero.Reader.onToggleSidebar = _updatePaneWidth;
+		_contextPaneInner.addEventListener("keypress", ZoteroItemPane.handleKeypress);
 	};
 
 	this.destroy = function () {
 		window.removeEventListener('resize', _update);
+		_contextPaneInner.removeEventListener("keypress", ZoteroItemPane.handleKeypress);
 		Zotero.Notifier.unregisterObserver(this._notifierID);
 		Zotero.Reader.onChangeSidebarWidth = () => {};
 		Zotero.Reader.onToggleSidebar = () => {};
diff --git a/chrome/content/zotero/elements/attachmentBox.js b/chrome/content/zotero/elements/attachmentBox.js
index dc3f95c601..a07260c26d 100644
--- a/chrome/content/zotero/elements/attachmentBox.js
+++ b/chrome/content/zotero/elements/attachmentBox.js
@@ -32,8 +32,8 @@
 		content = MozXULElement.parseXULToFragment(`
 			<collapsible-section data-l10n-id="section-attachment-info" data-pane="attachment-info">
 				<html:div class="body">
-					<attachment-preview id="attachment-preview"/>
-					<label id="url" crop="end"
+					<attachment-preview id="attachment-preview" tabindex="0"/>
+					<label id="url" crop="end" tabindex="0"
 						ondragstart="let dt = event.dataTransfer; dt.setData('text/x-moz-url', this.value); dt.setData('text/uri-list', this.value); dt.setData('text/plain', this.value);"/>
 					<html:div class="metadata-table">
 						<html:div id="fileNameRow" class="meta-row">
@@ -56,7 +56,7 @@
 							<html:div class="meta-label"><html:label id="index-status-label" class="key" data-l10n-id="attachment-info-index"/></html:div>
 							<html:div class="meta-data">
 								<html:label id="index-status"/>
-								<toolbarbutton id="reindex" oncommand="this.hidden = true; setTimeout(function () { ZoteroPane_Local.reindexItem(); }, 50)"/>
+								<toolbarbutton id="reindex" tabindex="0" oncommand="this.hidden = true; setTimeout(function () { ZoteroPane_Local.reindexItem(); }, 50)"/>
 							</html:div>
 						</html:div>
 					</html:div>
@@ -228,6 +228,34 @@
 					this._preview.render();
 				}
 			});
+
+			// Work around the reindex toolbarbutton not wanting to properly receive focus on tab.
+			// Make <image> focusable. On focus of the image, bounce the focus to the toolbarbutton.
+			// Temporarily remove tabindex from the <image> so that the focus can move past the
+			// reindex button
+			let reindexButton = this._id("indexStatusRow").querySelector(".meta-data toolbarbutton");
+			if (reindexButton) {
+				reindexButton.addEventListener("focusin", function (e) {
+					if (e.target.tagName == "image") {
+						reindexButton.focus();
+						reindexButton.querySelector("image").removeAttribute("tabindex");
+					}
+				});
+				reindexButton.addEventListener("blur", function (_) {
+					setTimeout(() => {
+						if (document.activeElement !== reindexButton) {
+							reindexButton.querySelector("image").setAttribute("tabindex", "0");
+						}
+					});
+				});
+			}
+			// Prevents the button from getting stuck in active state
+			reindexButton.addEventListener("keydown", (e) => {
+				if (e.key == " ") {
+					e.preventDefault();
+					reindexButton.click();
+				}
+			});
 		}
 
 		destroy() {
@@ -281,7 +309,7 @@
 							ZoteroPane_Local.loadURI(this.value, event);
 						}
 					};
-					urlField.className = 'zotero-text-link';
+					urlField.className = 'zotero-text-link keyboard-clickable';
 				}
 				else {
 					urlField.className = '';
@@ -381,6 +409,13 @@
 				indexStatusRow.hidden = true;
 			}
 			
+			// Make the image of the reindex toolbarbutton focusable because for some reason the
+			// actual toolbarbutton does not receive focus on tab
+			let reindexButton = indexStatusRow.querySelector("toolbarbutton");
+			if (document.activeElement !== reindexButton) {
+				reindexButton.querySelector("image").setAttribute("tabindex", "0");
+			}
+
 			this.initAttachmentNoteEditor();
 			
 			if (this.displayButton) {
diff --git a/chrome/content/zotero/elements/attachmentPreview.js b/chrome/content/zotero/elements/attachmentPreview.js
index ec0e044863..049e7d8c31 100644
--- a/chrome/content/zotero/elements/attachmentPreview.js
+++ b/chrome/content/zotero/elements/attachmentPreview.js
@@ -181,6 +181,8 @@
 			this.addEventListener("mouseenter", this.updateGoto);
 			this.addEventListener("dragstart", this._handleDragStart);
 			this.addEventListener("dragend", this._handleDragEnd);
+			this.addEventListener("focusin", this._handleFocusIn);
+			this.addEventListener("keypress", this._handleKeypress);
 			this.setAttribute("data-preview-type", "unknown");
 		}
 
@@ -192,6 +194,8 @@
 			this.removeEventListener("mouseenter", this.updateGoto);
 			this.removeEventListener("dragstart", this._handleDragStart);
 			this.removeEventListener("dragend", this._handleDragEnd);
+			this.removeEventListener("focusin", this._handleFocusIn);
+			this.removeEventListener("keypress", this._handleKeypress);
 		}
 
 		async render() {
@@ -279,6 +283,36 @@
 			this._id("next").disabled = !this._reader?.canGoto("next");
 		}
 
+		_handleFocusIn() {
+			this.focus();
+		}
+
+		_handleKeypress(e) {
+			let stopEvent = false;
+			// Space or enter open attachment
+			if ([" ", "Enter"].includes(e.key)) {
+				this.openAttachment(e);
+				stopEvent = true;
+			}
+			// Hacky way to preventing the focus from going into the actual reader where it can
+			// get stuck. On tab from the preview, try to find the next element and focus it.
+			else if (e.key == "Tab" && !e.shiftKey) {
+				let toFocus = this.nextElementSibling.querySelector('[tabindex="0"]');
+				if (!toFocus && this.nextElementSibling.getAttribute("tabindex") == "0") {
+					toFocus = this.nextElementSibling;
+				}
+				if (toFocus) {
+					toFocus.focus();
+					stopEvent = true;
+				}
+			}
+
+			if (stopEvent) {
+				e.stopPropagation();
+				e.preventDefault();
+			}
+		}
+
 		async _renderReader() {
 			this.setPreviewStatus("loading");
 			// This only need to be awaited during first load
diff --git a/chrome/content/zotero/elements/attachmentRow.js b/chrome/content/zotero/elements/attachmentRow.js
index d82e939343..e303d23ffc 100644
--- a/chrome/content/zotero/elements/attachmentRow.js
+++ b/chrome/content/zotero/elements/attachmentRow.js
@@ -31,7 +31,7 @@ import { getCSSItemTypeIcon } from 'components/icons';
 	class AttachmentRow extends XULElementBase {
 		content = MozXULElement.parseXULToFragment(`
 			<html:div class="head">
-				<html:div class="clicky-item attachment-btn">
+				<html:div class="clicky-item attachment-btn keyboard-clickable" tabindex="0">
 					<html:span class="icon"/>
 					<html:div class="label"/>
 				</html:div>
diff --git a/chrome/content/zotero/elements/attachmentsBox.js b/chrome/content/zotero/elements/attachmentsBox.js
index 3ac01fbb6b..dbb7120290 100644
--- a/chrome/content/zotero/elements/attachmentsBox.js
+++ b/chrome/content/zotero/elements/attachmentsBox.js
@@ -30,7 +30,7 @@
 		content = MozXULElement.parseXULToFragment(`
 			<collapsible-section data-l10n-id="section-attachments" data-pane="attachments" extra-buttons="add">
 				<html:div class="body">
-					<attachment-preview/>
+					<attachment-preview tabindex="0"/>
 					<html:div class="attachments-container"></html:div>
 				</html:div>
 			</collapsible-section>
diff --git a/chrome/content/zotero/elements/collapsibleSection.js b/chrome/content/zotero/elements/collapsibleSection.js
index fab3911d8e..9efc0b5550 100644
--- a/chrome/content/zotero/elements/collapsibleSection.js
+++ b/chrome/content/zotero/elements/collapsibleSection.js
@@ -66,8 +66,10 @@
 				cancelable: false
 			});
 			
-			if (!newOpen && this.ownerDocument?.activeElement && this.contains(this.ownerDocument?.activeElement)) {
-				this.ownerDocument.activeElement.blur();
+			// Blur the focus if it's within the body (not the header) of the section on collapse
+			let focused = this.ownerDocument?.activeElement;
+			if (!newOpen && focused && this.lastChild.contains(focused)) {
+				focused.blur();
 			}
 			
 			this._saveOpenState();
@@ -114,11 +116,12 @@
 				throw new Error('data-pane is required');
 			}
 			
-			this.tabIndex = 0;
 			
 			this._head = document.createElement('div');
 			this._head.role = 'button';
 			this._head.className = 'head';
+			this._head.setAttribute("tabindex", "0");
+			this._head.addEventListener('mousedown', this._mouseDown);
 			this._head.addEventListener('click', this._handleClick);
 			this._head.addEventListener('keydown', this._handleKeyDown);
 			this._head.addEventListener('contextmenu', this._handleContextMenu);
@@ -136,6 +139,7 @@
 			
 			let twisty = document.createXULElement('toolbarbutton');
 			twisty.className = 'twisty';
+			twisty.setAttribute("tabindex", "0");
 			this._head.append(twisty);
 			
 			this._buildExtraButtons();
@@ -237,6 +241,7 @@
 				if (!buttonType) continue;
 				let button = document.createXULElement('toolbarbutton');
 				button.classList.add(buttonType, 'section-custom-button');
+				button.setAttribute("tabindex", "0");
 				button.addEventListener('command', (event) => {
 					this.dispatchEvent(new CustomEvent(buttonType, {
 						...event,
@@ -251,6 +256,7 @@
 		
 		destroy() {
 			this._head.removeEventListener('click', this._handleClick);
+			this._head.removeEventListener('mousedown', this._mouseDown);
 			this._head.removeEventListener('keydown', this._handleKeyDown);
 			this._head.removeEventListener('contextmenu', this._handleContextMenu);
 			
@@ -284,12 +290,78 @@
 			if (event.target.closest('.section-custom-button, menupopup')) return;
 			this.open = !this.open;
 		};
+
+		// Prevent moving focus to the header on click
+		_mouseDown = (event) => {
+			event.preventDefault();
+		};
 		
 		_handleKeyDown = (event) => {
-			if (event.target.closest('.section-custom-button')) return;
-			if (event.key === 'Enter' || event.key === ' ') {
-				this.open = !this.open;
+			let tgt = event.target;
+			let stopEvent = () => {
 				event.preventDefault();
+				event.stopPropagation();
+			};
+
+			// Tab/Shift-Tab from section header through header buttons
+			if (event.key === "Tab") {
+				let nextBtn;
+				if (tgt.classList.contains("head") && event.shiftKey) {
+					return;
+				}
+				if (tgt.classList.contains("head")) {
+					nextBtn = this._head.querySelector("toolbarbutton");
+				}
+				else {
+					nextBtn = event.shiftKey ? tgt.previousElementSibling : tgt.nextElementSibling;
+				}
+				
+				if (nextBtn?.tagName == "popupset") {
+					nextBtn = this._head;
+				}
+				if (nextBtn) {
+					nextBtn.focus();
+					stopEvent();
+				}
+			}
+			if (event.target.tagName === "toolbarbutton") {
+				// No actions on right/left on header buttons
+				if (["ArrowRight", "ArrowLeft"].includes(event.key)) {
+					stopEvent();
+					return;
+				}
+				// Let itemPane.js listener handle space or Enter clicks
+				if ([" ", "Enter"].includes(event.key)) {
+					return;
+				}
+			}
+			// Space/Enter toggle section open/closed.
+			// ArrowLeft/ArrowRight on actual header will close/open
+			if (["ArrowLeft", "ArrowRight", " ", "Enter"].includes(event.key)) {
+				stopEvent();
+				this.open = ([" ", "Enter"].includes(event.key)) ? !this.open : (event.key == "ArrowRight");
+				event.target.focus();
+			}
+			if (["ArrowUp", "ArrowDown"].includes(event.key)) {
+				let up = event.key == "ArrowUp";
+				// Arrow up from a button focuses the header
+				if (up && this._head !== tgt) {
+					this._head.focus();
+					stopEvent();
+					return;
+				}
+				// ArrowUp focuses the header of the previous section, ArrowDown - of the next one
+				let box = this.parentNode;
+				let nextBox;
+				nextBox = up ? box.previousElementSibling : box.nextElementSibling;
+				while (nextBox && nextBox.hidden) {
+					nextBox = up ? nextBox.previousElementSibling : nextBox.nextElementSibling;
+				}
+				let nextSection = nextBox?.querySelector("collapsible-section");
+				if (nextSection) {
+					nextSection._head.focus();
+					stopEvent();
+				}
 			}
 		};
 		
diff --git a/chrome/content/zotero/elements/itemBox.js b/chrome/content/zotero/elements/itemBox.js
index 6b9b3bec68..1aebc0b9ae 100644
--- a/chrome/content/zotero/elements/itemBox.js
+++ b/chrome/content/zotero/elements/itemBox.js
@@ -1914,16 +1914,15 @@
 							return;
 						}
 						event.preventDefault();
+						event.stopPropagation();
 						this._focusNextField(this._lastTabIndex, true);
 					}
 					else {
-						event.preventDefault();
-						// If on the last field, return focus to item tree
-						if (this._lastTabIndex == this._tabIndexMaxFields) {
-							document.getElementById('item-tree-main-default')?.focus();
-							return;
+						let focused = this._focusNextField(++this._lastTabIndex);
+						if (focused) {
+							event.preventDefault();
+							event.stopPropagation();
 						}
-						this._focusNextField(++this._lastTabIndex);
 					}
 			}
 		}
diff --git a/chrome/content/zotero/elements/librariesCollectionsBox.js b/chrome/content/zotero/elements/librariesCollectionsBox.js
index 7fc1423b8c..c0e3019f85 100644
--- a/chrome/content/zotero/elements/librariesCollectionsBox.js
+++ b/chrome/content/zotero/elements/librariesCollectionsBox.js
@@ -116,7 +116,8 @@ import { getCSSIcon } from 'components/icons';
 			row.classList.toggle('context', isContext);
 			
 			let box = document.createElement('div');
-			box.classList.add('box');
+			box.className = 'box keyboard_clickable';
+			box.setAttribute("tabindex", "0");
 			
 			let iconName;
 			if (obj instanceof Zotero.Group) {
@@ -141,6 +142,7 @@ import { getCSSIcon } from 'components/icons';
 			if (this._mode == 'edit' && obj instanceof Zotero.Collection && !isContext) {
 				let remove = document.createXULElement('toolbarbutton');
 				remove.className = 'zotero-clicky zotero-clicky-minus';
+				remove.setAttribute("tabindex", "0");
 				remove.addEventListener('command', () => {
 					if (Services.prompt.confirm(
 						window,
diff --git a/chrome/content/zotero/elements/notesBox.js b/chrome/content/zotero/elements/notesBox.js
index bc5905b0a2..32be4dcdc0 100644
--- a/chrome/content/zotero/elements/notesBox.js
+++ b/chrome/content/zotero/elements/notesBox.js
@@ -122,7 +122,8 @@ import { getCSSItemTypeIcon } from 'components/icons';
 
 				let box = document.createElement('div');
 				box.addEventListener('click', () => this._handleShowItem(id));
-				box.className = 'box';
+				box.className = 'box keyboard-clickable';
+				box.setAttribute("tabindex", 0);
 				box.append(icon, label);
 
 				row.append(box);
@@ -131,6 +132,7 @@ import { getCSSItemTypeIcon } from 'components/icons';
 					let remove = document.createXULElement("toolbarbutton");
 					remove.addEventListener('command', () => this._handleRemove(id));
 					remove.className = 'zotero-clicky zotero-clicky-minus';
+					remove.setAttribute("tabindex", "0");
 					row.append(remove);
 				}
 
diff --git a/chrome/content/zotero/elements/relatedBox.js b/chrome/content/zotero/elements/relatedBox.js
index d75b9c26a0..9f43760dd0 100644
--- a/chrome/content/zotero/elements/relatedBox.js
+++ b/chrome/content/zotero/elements/relatedBox.js
@@ -132,7 +132,8 @@ import { getCSSItemTypeIcon } from 'components/icons';
 
 					let box = document.createElement('div');
 					box.addEventListener('click', () => this._handleShowItem(id));
-					box.className = 'box';
+					box.setAttribute("tabindex", "0");
+					box.className = 'box keyboard-clickable';
 					box.appendChild(icon);
 					box.appendChild(label);
 					row.append(box);
@@ -141,6 +142,7 @@ import { getCSSItemTypeIcon } from 'components/icons';
 						let remove = document.createXULElement("toolbarbutton");
 						remove.addEventListener('command', () => this._handleRemove(id));
 						remove.className = 'zotero-clicky zotero-clicky-minus';
+						remove.setAttribute("tabindex", "0");
 						row.append(remove);
 					}
 
diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js
index b924f4d0d2..8b713ed5e1 100644
--- a/chrome/content/zotero/itemPane.js
+++ b/chrome/content/zotero/itemPane.js
@@ -54,6 +54,8 @@ var ZoteroItemPane = new function() {
 		_deck = document.getElementById('zotero-item-pane-content');
 		
 		this._unregisterID = Zotero.Notifier.registerObserver(this, ['item'], 'itemPane');
+
+		_container.addEventListener("keypress", this.handleKeypress);
 	};
 	
 	
@@ -166,6 +168,49 @@ var ZoteroItemPane = new function() {
 		
 		document.getElementById('zotero-item-pane-content').selectedIndex = 2;
 	};
+
+	// Keyboard navigation within the itemPane. Also handles contextPane keyboard nav
+	this.handleKeypress = function (event) {
+		let stopEvent = () => {
+			event.preventDefault();
+			event.stopPropagation();
+		};
+		let isLibraryTab = Zotero_Tabs.selectedIndex == 0;
+		let sidenav = document.getElementById(
+			isLibraryTab ? 'zotero-view-item-sidenav' : 'zotero-context-pane-sidenav'
+		);
+		// 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);
+			pane.firstChild._head.focus();
+			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;
+			if (event.key == "Tab" && !event.shiftKey) {
+				next = event.target.nextElementSibling;
+			}
+			if (event.key == "Tab" && event.shiftKey) {
+				next = event.target.parentNode.previousElementSibling?.lastChild;
+			}
+			// Force the element to be visible before focusing
+			if (next) {
+				next.style.visibility = "visible";
+				next.focus();
+				next.style.removeProperty("visibility");
+				stopEvent();
+			}
+		}
+	};
 	
 	
 	/**
diff --git a/scss/elements/_attachmentBox.scss b/scss/elements/_attachmentBox.scss
index 03961efab9..a5373de329 100644
--- a/scss/elements/_attachmentBox.scss
+++ b/scss/elements/_attachmentBox.scss
@@ -36,8 +36,10 @@ attachment-box {
 			width: 22px;
 			padding: 1px;
 			margin-left: 4px;
-			color: var(--fill-secondary);
 			@include svgicon-menu("sync", "universal", "20");
+			&:not([disabled='true']) {
+				color: var(--fill-secondary);
+			}
 		}
 	}
 
diff --git a/scss/elements/_attachmentPreview.scss b/scss/elements/_attachmentPreview.scss
index 32f464c620..1779a58cef 100644
--- a/scss/elements/_attachmentPreview.scss
+++ b/scss/elements/_attachmentPreview.scss
@@ -14,6 +14,7 @@ attachment-preview {
 	--preview-width: 400;
 	--preview-height: calc(min(var(--preview-width) / var(--width-height-ratio), var(--max-height)));
 	max-height: var(--max-height);
+	@include focus-ring;
 
 	&[hidden] {
 		display: none;
diff --git a/scss/elements/_collapsibleSection.scss b/scss/elements/_collapsibleSection.scss
index 06cc074d66..5e1f16118a 100644
--- a/scss/elements/_collapsibleSection.scss
+++ b/scss/elements/_collapsibleSection.scss
@@ -3,12 +3,16 @@ collapsible-section {
 	flex-direction: column;
 	gap: 2px;
 	padding-block: 4px;
-	
+
+	--width-focus-border: 2px;
+	--radius-focus-border: 5px;
+
 	:not(:last-child) > & {
 		border-bottom: 1px solid var(--fill-quinary);
 	}
 
 	& > .head {
+		@include focus-ring;
 		@include comfortable {
 			padding-block: 2px;
 		}
@@ -102,4 +106,9 @@ collapsible-section {
 	&.disable-transitions * {
 		transition: none !important;
 	}
+
+	toolbarbutton, .box, .keyboard-clickable {
+		@include focus-ring;
+	}
+	
 }