Keyboard accessibility improvements

src/common/annotation-manager.js

@@ -15,6 +15,7 @@ class AnnotationManager {
 		this._readOnly = options.readOnly;
 		this._authorName = options.authorName;
 		this._annotations = options.annotations;
+		this._tools = options.tools;
 		this._onChangeFilter = options.onChangeFilter;
 		this._onSave = options.onSave;
 		this._onDelete = options.onDelete;
@@ -59,14 +60,15 @@ class AnnotationManager {
 			return null;
 		}
 		// Mandatory properties
-		let { color, sortIndex } = annotation;
-		if (!color) {
-			throw new Error(`Missing 'color' property`);
-		}
-		if (!sortIndex) {
+		if (!annotation.sortIndex) {
 			throw new Error(`Missing 'sortIndex' property`);
 		}
 
+		// Use the current default color from the toolbar, if missing
+		if (!annotation.color) {
+			annotation.color = this._tools[annotation.type].color;
+		}
+
 		// Optional properties
 		annotation.pageLabel = annotation.pageLabel || '';
 		annotation.text = annotation.text || '';
@@ -123,7 +125,7 @@ class AnnotationManager {
 			}
 			// All properties in the existing annotation position are preserved except nextPageRects,
 			// which isn't preserved only when a new rects property is given
-			let deleteNextPageRects = annotation.rects && !annotation.position?.nextPageRects;
+			let deleteNextPageRects = annotation.position?.rects && !annotation.position?.nextPageRects;
 			annotation = {
 				...existingAnnotation,
 				...annotation,

src/common/components/common/preview.js

@@ -1,4 +1,4 @@
-import React, { useContext } from 'react';
+import React, { useContext, useRef } from 'react';
 import { FormattedMessage, useIntl } from 'react-intl';
 import cx from 'classnames';
 import Editor from './editor';
@@ -136,6 +136,13 @@ export function PopupPreview(props) {
 export function SidebarPreview(props) {
 	const intl = useIntl();
 	const { platform } = useContext(ReaderContext);
+	const lastImageRef = useRef();
+
+	// Store and render the last image to avoid flickering when annotation manager removes
+	// old image, but the new one isn't generated yet
+	if (props.annotation.image) {
+		lastImageRef.current = props.annotation.image;
+	}
 
 	function handlePageLabelClick(event) {
 		event.stopPropagation();
@@ -276,6 +283,8 @@ export function SidebarPreview(props) {
 	let expandedState = {};
 	expandedState['expanded' + props.state] = true;
 
+	let image = annotation.image || lastImageRef.current;
+
 	return (
 		<div
 			onContextMenu={handleContextMenu}
@@ -337,10 +346,10 @@ export function SidebarPreview(props) {
 					>{props.readOnly ? <IconLock/> : <IconOptions/>}</button>
 				</div>
 			</header>
-			{annotation.image && (
+			{image && (
 				<img
 					className="image"
-					src={annotation.image}
+					src={image}
 					onClick={e => handleSectionClick(e, 'image')}
 					draggable={true}
 					onDragStart={handleDragStart}

src/common/components/reader-ui.js

@@ -43,14 +43,6 @@ function View(props) {
 				data-proxy={`#${name}-view > iframe`}
 				style={{ position: 'absolute' }}
 			/>
-			{state[name + 'ViewFindState'].popupOpen &&
-				<FindPopup
-					params={state[name + 'ViewFindState']}
-					onChange={handleFindStateChange}
-					onFindNext={handleFindNext}
-					onFindPrevious={handleFindPrevious}
-				/>
-			}
 			{state[name + 'ViewSelectionPopup'] && !state.readOnly &&
 				<SelectionPopup
 					params={state[name + 'ViewSelectionPopup']}
@@ -81,6 +73,15 @@ function View(props) {
 					onNavigate={props.onNavigate}
 				/>
 			}
+			{state[name + 'ViewFindState'].popupOpen &&
+				<FindPopup
+					params={state[name + 'ViewFindState']}
+					onChange={handleFindStateChange}
+					onFindNext={handleFindNext}
+					onFindPrevious={handleFindPrevious}
+					onAddAnnotation={props.onAddAnnotation}
+				/>
+			}
 		</div>
 	);
 }

src/common/components/sidebar/annotations-view.js

@@ -193,15 +193,15 @@ const AnnotationsView = memo(React.forwardRef((props, ref) => {
 	}, []);
 
 	// Allow navigating to next/previous annotation if inner annotation element like
-	// more button, empty comment or tags are focused
+	// more button, or tags are focused, but not comment/text
 	function handleKeyDown(event) {
 		let node = event.target;
 		// Don't do anything if annotation element is focused, because focus-manager will do the navigation
 		if (node.classList.contains('annotation')) {
 			return;
 		}
 		let annotationNode = node.closest('.annotation');
-		if (!node.classList.contains('content') || !node.innerText) {
+		if (!node.classList.contains('content')) {
 			if (pressedPreviousKey(event)) {
 				annotationNode.previousElementSibling?.focus();
 				event.preventDefault();

src/common/components/sidebar/search-box.js

@@ -23,7 +23,7 @@ function SearchBox({ query, placeholder, onInput }) {
 	function handleKeyDown(event) {
 		if (event.key === 'Escape') {
 			if (event.target.value) {
-				handleClear();
+				handleClear(event);
 				event.stopPropagation();
 			}
 		}

src/common/components/sidebar/thumbnails-view.js

@@ -224,7 +224,7 @@ function ThumbnailsView(props) {
 				onMouseDown={handleMouseDown}
 				ref={containerRef}
 				tabIndex={-1}
-				role='listbox'
+				role="listbox"
 				aria-label={intl.formatMessage({ id: "pdfReader.thumbnails" })}
 				aria-activedescendant={`thumbnail_${selected[selected.length-1]}`}
 				aria-multiselectable="true"

src/common/components/view-popup/find-popup.js

@@ -7,15 +7,20 @@ import { DEBOUNCE_FIND_POPUP_INPUT } from '../../defines';
 import IconChevronUp from '../../../../res/icons/20/chevron-up.svg';
 import IconChevronDown from '../../../../res/icons/20/chevron-down.svg';
 import IconClose from '../../../../res/icons/20/x.svg';
+import { getCodeCombination, getKeyCombination } from '../../lib/utilities';
 
-function FindPopup({ params, onChange, onFindNext, onFindPrevious }) {
+function FindPopup({ params, onChange, onFindNext, onFindPrevious, onAddAnnotation }) {
 	const intl = useIntl();
 	const inputRef = useRef();
+	const preventInputRef = useRef(false);
 	const [query, setQuery] = useState(params.query);
 
 	const debounceInputChange = useCallback(debounce(value => {
+		if (!inputRef.current) {
+			return;
+		}
 		let query = inputRef.current.value;
-		if (!(query.length === 1 && RegExp(/^\p{Script=Latin}/, 'u').test(query))) {
+		if (query !== params.query && !(query.length === 1 && RegExp(/^\p{Script=Latin}/, 'u').test(query))) {
 			onChange({ ...params, query, active: true, result: null });
 		}
 	}, DEBOUNCE_FIND_POPUP_INPUT), [onChange]);
@@ -31,25 +36,51 @@ function FindPopup({ params, onChange, onFindNext, onFindPrevious }) {
 	}, [params.query]);
 
 	function handleInputChange(event) {
+		if (preventInputRef.current) {
+			preventInputRef.current = false;
+			return;
+		}
 		let value = event.target.value;
 		setQuery(value);
 		debounceInputChange();
 	}
 
 	function handleInputKeyDown(event) {
-		if (event.key === 'Enter') {
+		let key = getKeyCombination(event);
+		let code = getCodeCombination(event);
+		if (key === 'Enter') {
 			if (params.active) {
-				if (event.shiftKey) {
-					onFindPrevious();
-				}
-				else {
-					onFindNext();
-				}
+				onFindNext();
 			}
 			else {
 				onChange({ ...params, active: true });
 			}
 		}
+		else if (key === 'Shift-Enter') {
+			if (params.active) {
+				onFindPrevious();
+			}
+			else {
+				onChange({ ...params, active: true });
+			}
+		}
+		else if (key === 'Escape') {
+			onChange({ ...params, popupOpen: false, active: false, result: null });
+			event.preventDefault();
+			event.stopPropagation();
+		}
+		else if (code === 'Ctrl-Alt-Digit1') {
+			preventInputRef.current = true;
+			if (params.result?.annotation) {
+				onAddAnnotation({ ...params.result.annotation, type: 'highlight' }, true);
+			}
+		}
+		else if (code === 'Ctrl-Alt-Digit2') {
+			preventInputRef.current = true;
+			if (params.result?.annotation) {
+				onAddAnnotation({ ...params.result.annotation, type: 'underline' }, true);
+			}
+		}
 	}
 
 	function handleCloseClick() {

src/common/components/view-popup/overlay-popup/preview-popup.js

@@ -30,7 +30,7 @@ function PreviewPopup(props) {
 			padding={10}
 			onRender={handleRender}
 		>
-			<div dir="ltr" ref={innerRef} className="inner">
+			<div dir="ltr" ref={innerRef} className="inner" tabIndex="-1" data-tabstop={1}>
 				<img height={props.params.height} width={props.params.width} src={props.params.image}/>
 			</div>
 		</ViewPopup>

src/common/defines.js

@@ -28,3 +28,6 @@ export const MIN_IMAGE_ANNOTATION_SIZE = 10; // pt
 export const DEBOUNCE_STATE_CHANGE = 300; // ms
 export const DEBOUNCE_STATS_CHANGE = 100; // ms
 export const DEBOUNCE_FIND_POPUP_INPUT = 500; // ms
+
+export const FIND_RESULT_COLOR_ALL = '#EDD3ED';
+export const FIND_RESULT_COLOR_CURRENT = '#D4E0D1';

src/common/focus-manager.js

@@ -81,7 +81,7 @@ export class FocusManager {
 
 	_handlePointerDown(event) {
 		if ('closest' in event.target) {
-			if (!event.target.closest('input, textarea, [contenteditable="true"], .annotation, .thumbnails-view, .outline-view, .error-bar, .reference-row')) {
+			if (!event.target.closest('input, textarea, [contenteditable="true"], .annotation, .thumbnails-view, .outline-view, .error-bar, .reference-row, .preview-popup')) {
 				// Note: Doing event.preventDefault() also prevents :active class on Firefox
 				event.preventDefault();
 			}
@@ -105,11 +105,11 @@ export class FocusManager {
 		if ((e.target.closest('.outline-view') || e.target.closest('input[type="range"]')) && ['ArrowLeft', 'ArrowRight'].includes(e.key)) {
 			return;
 		}
-		if (pressedNextKey(e) && !e.target.closest('[contenteditable], input[type="text"]')) {
+		if (pressedNextKey(e) && !e.target.closest('[contenteditable], input[type="text"], .preview-popup')) {
 			e.preventDefault();
 			this.tabToItem();
 		}
-		else if (pressedPreviousKey(e) && !e.target.closest('[contenteditable], input[type="text"]')) {
+		else if (pressedPreviousKey(e) && !e.target.closest('[contenteditable], input[type="text"], .preview-popup')) {
 			e.preventDefault();
 			this.tabToItem(true);
 		}
@@ -168,6 +168,19 @@ export class FocusManager {
 
 		group = groups[groupIndex];
 
+		// If jumping into the sidebar annotations view, focus the last selected annotation,
+		// but don't trigger navigation in the view
+		if (group.classList.contains('annotations')
+			&& this._reader._lastSelectedAnnotationID
+			// Make sure there are at least two annotations, otherwise it won't be possible to navigate to annotation
+			&& this._reader._state.annotations.length >= 2
+			// Make sure the annotation still exists
+			&& this._reader._state.annotations.find(x => x.id === this._reader._lastSelectedAnnotationID)) {
+			this._reader._updateState({ selectedAnnotationIDs: [this._reader._lastSelectedAnnotationID] });
+			// It also needs to be focused, otherwise pressing TAB will shift the focus to an unexpected location
+			setTimeout(() => group.querySelector(`[data-sidebar-annotation-id="${this._reader._lastSelectedAnnotationID}"]`)?.focus(), 100);
+			return;
+		}
 
 		let focusableParent = item.parentNode.closest('[tabindex="-1"]');