From b63e4e85ad3355745ecc85e50d7825032fbbfde2 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Tue, 12 Nov 2024 12:12:25 -0500 Subject: [PATCH 01/24] EPUB/Snapshot: Keyboard accessibility improvements for annotations --- src/common/lib/utilities.js | 12 ++- src/common/types.ts | 3 +- src/dom/common/dom-view.tsx | 121 ++++++++++++++++++++++++++---- src/dom/common/lib/find/index.ts | 21 ++++-- src/dom/epub/epub-view.ts | 15 +++- src/dom/epub/find.ts | 15 ++-- src/dom/snapshot/snapshot-view.ts | 15 +++- 7 files changed, 166 insertions(+), 36 deletions(-) diff --git a/src/common/lib/utilities.js b/src/common/lib/utilities.js index 09d418de..2850048a 100644 --- a/src/common/lib/utilities.js +++ b/src/common/lib/utilities.js @@ -55,7 +55,11 @@ export function normalizeKey(key, code) { return key; } -// Key combination taking into account layout and modifier keys +/** + * Key combination taking into account layout and modifier keys + * @param {KeyboardEvent} event + * @returns {string} + */ export function getKeyCombination(event) { let modifiers = []; if (event.metaKey && isMac()) { @@ -86,7 +90,11 @@ export function getKeyCombination(event) { return modifiers.join('-'); } -// Physical key combination +/** + * Physical key combination + * @param {KeyboardEvent} event + * @returns {string} + */ export function getCodeCombination(event) { let modifiers = []; if (event.metaKey && isMac()) { diff --git a/src/common/types.ts b/src/common/types.ts index 1172fed7..7aa4c658 100644 --- a/src/common/types.ts +++ b/src/common/types.ts @@ -160,7 +160,8 @@ export type FindState = { total: number, index: number, // Mobile app lists all results in a popup - snippets: string[] + snippets: string[], + annotation?: NewAnnotation } | null; }; diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 6579ff3f..30070e1a 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -14,6 +14,7 @@ import { Platform, SelectionPopupParams, Tool, + ToolType, ViewStats, WADMAnnotation, } from "../../common/types"; @@ -37,9 +38,15 @@ import { import { getSelectionRanges } from "./lib/selection"; import { FindProcessor } from "./lib/find"; import { SELECTION_COLOR } from "../../common/defines"; -import { debounceUntilScrollFinishes, isMac, isSafari } from "../../common/lib/utilities"; import { - closestElement, + debounceUntilScrollFinishes, + getCodeCombination, + getKeyCombination, + isMac, + isSafari +} from "../../common/lib/utilities"; +import { + closestElement, getContainingBlock, isElement } from "./lib/nodes"; import { debounce } from "../../common/lib/debounce"; @@ -302,7 +309,25 @@ abstract class DOMView { if (!selection || selection.isCollapsed) { return null; } - let range = makeRangeSpanning(...getSelectionRanges(selection)); + let range: Range; + if (type === 'highlight' || type === 'underline') { + range = makeRangeSpanning(...getSelectionRanges(selection)); + } + else if (type === 'note') { + let element = closestElement(selection.getRangeAt(0).commonAncestorContainer); + if (!element) { + return null; + } + let blockElement = getContainingBlock(element); + if (!blockElement) { + return null; + } + range = this._iframeDocument.createRange(); + range.selectNode(blockElement); + } + else { + return null; + } return this._getAnnotationFromRange(range, type, color); } @@ -689,13 +714,11 @@ abstract class DOMView { else { let pos = supportsCaretPositionFromPoint() && caretPositionFromPoint(this._iframeDocument, event.clientX, event.clientY); - let node = pos ? pos.offsetNode : target; - // Expand to the closest block element - while (node.parentNode - && (!isElement(node) || this._iframeWindow.getComputedStyle(node).display.includes('inline'))) { - node = node.parentNode; - } - range.selectNode(node); + let element = closestElement(pos ? pos.offsetNode : target); + if (!element) return null; + let blockElement = getContainingBlock(element); + if (!blockElement) return null; + range.selectNode(blockElement); } let rect = range.getBoundingClientRect(); if (rect.right <= 0 || rect.left >= this._iframeWindow.innerWidth @@ -722,12 +745,12 @@ abstract class DOMView { protected abstract _handleInternalLinkClick(link: HTMLAnchorElement): void; protected _handleKeyDown(event: KeyboardEvent) { - let { key } = event; - let shift = event.shiftKey; - // To figure out if wheel events are pinch-to-zoom this._isCtrlKeyDown = event.key === 'Control'; + let key = getKeyCombination(event); + let code = getCodeCombination(event); + // Focusable elements in PDF view are annotations and overlays (links, citations, figures). // Once TAB is pressed, arrows can be used to navigate between them let focusableElements: HTMLElement[] = []; @@ -755,7 +778,7 @@ abstract class DOMView { // pass it to this._onKeyDown(event) below return; } - else if (shift && key === 'Tab') { + else if (key === 'Shift-Tab') { if (focusedElement) { focusedElement.blur(); } @@ -808,6 +831,75 @@ abstract class DOMView { } } + if (this._selectedAnnotationIDs.length === 1 + && (key.endsWith('Shift-ArrowLeft') + || key.endsWith('Shift-ArrowRight'))) { + let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); + let granularity = event.altKey ? 'word' : 'character'; + + let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; + let selection = this._iframeDocument.getSelection()!; + + let oldRange = this.toDisplayedRange(annotation.position)!; + selection.removeAllRanges(); + selection.addRange(oldRange); + if (resizeStart) { + selection.collapseToStart(); + } + else { + selection.collapseToEnd(); + } + selection.modify( + 'move', + key.endsWith('ArrowRight') ? 'right' : 'left', + granularity + ); + let newRange = selection.getRangeAt(0); + if (resizeStart) { + newRange.setEnd(oldRange.endContainer, oldRange.endOffset); + } + else { + newRange.setStart(oldRange.startContainer, oldRange.startOffset); + } + + if (newRange.collapsed) { + return; + } + + annotation.position = this.toSelector(newRange)!; + this._options.onUpdateAnnotations([annotation]); + selection.removeAllRanges(); + + event.preventDefault(); + return; + } + + if (code === 'Ctrl-Alt-Digit1' || code === 'Ctrl-Alt-Digit2' || code === 'Ctrl-Alt-Digit3') { + let type: AnnotationType; + switch (code) { + case 'Ctrl-Alt-Digit1': + type = 'highlight'; + break; + case 'Ctrl-Alt-Digit2': + type = 'underline'; + break; + case 'Ctrl-Alt-Digit3': + type = 'note'; + break; + } + let annotation = this._getAnnotationFromTextSelection(type, this._options.tools[type].color); + if (annotation) { + this._options.onAddAnnotation(annotation, true); + this._navigateToSelector(annotation.position, { + block: 'center', + behavior: 'smooth' + }); + this._iframeWindow.getSelection()?.removeAllRanges(); + } + event.preventDefault(); + return; + } + // Pass keydown even to the main window where common keyboard // shortcuts are handled i.e. Delete, Cmd-Minus, Cmd-f, etc. this._options.onKeyDown(event); @@ -1468,6 +1560,7 @@ export type DOMViewOptions = { primary?: boolean; mobile?: boolean; container: Element; + tools: Record; tool: Tool; platform: Platform; selectedAnnotationIDs: string[]; diff --git a/src/dom/common/lib/find/index.ts b/src/dom/common/lib/find/index.ts index 233bc2c5..346453f1 100644 --- a/src/dom/common/lib/find/index.ts +++ b/src/dom/common/lib/find/index.ts @@ -17,7 +17,7 @@ class DefaultFindProcessor implements FindProcessor { private _initialPos: number | null = null; - private readonly _onSetFindState?: (state?: FindState) => void; + private readonly _onSetFindState?: (result: ResultArg) => void; private readonly _annotationKeyPrefix?: string; @@ -27,7 +27,7 @@ class DefaultFindProcessor implements FindProcessor { constructor(options: { findState: FindState, - onSetFindState?: (state?: FindState) => void, + onSetFindState?: (result: ResultArg) => void, annotationKeyPrefix?: string, }) { this.findState = options.findState; @@ -254,12 +254,10 @@ class DefaultFindProcessor implements FindProcessor { if (this._cancelled) return; if (this._onSetFindState) { this._onSetFindState({ - ...this.findState, - result: { - total: this._buf.length, - index: this._pos === null ? 0 : this._pos, - snippets: this.getSnippets(), - } + total: this._buf.length, + index: this._pos === null ? 0 : this._pos, + snippets: this.getSnippets(), + range: this.current?.range }); } } @@ -289,6 +287,13 @@ function normalize(s: string) { export type FindAnnotation = Omit & { range: PersistentRange }; +export type ResultArg = { + total: number; + index: number; + snippets: string[]; + range?: PersistentRange; +}; + export type SearchContext = { text: string; charDataRanges: CharDataRange[]; diff --git a/src/dom/epub/epub-view.ts b/src/dom/epub/epub-view.ts index 3a299b9b..5210f8d2 100644 --- a/src/dom/epub/epub-view.ts +++ b/src/dom/epub/epub-view.ts @@ -1028,7 +1028,20 @@ class EPUBView extends DOMView { this._find = new EPUBFindProcessor({ view: this, findState: { ...state }, - onSetFindState: this._options.onSetFindState, + onSetFindState: (result) => { + this._options.onSetFindState({ + ...state, + result: { + total: result.total, + index: result.index, + snippets: result.snippets, + annotation: ( + result.range + && this._getAnnotationFromRange(result.range.toRange(), 'highlight') + ) ?? undefined + } + }); + }, }); let startRange = (this.flow.startRange && new PersistentRange(this.flow.startRange)) ?? undefined; let onFirstResult = () => this.findNext(); diff --git a/src/dom/epub/find.ts b/src/dom/epub/find.ts index 1928adfc..96ddbc84 100644 --- a/src/dom/epub/find.ts +++ b/src/dom/epub/find.ts @@ -1,7 +1,7 @@ import DefaultFindProcessor, { FindAnnotation, FindProcessor, - FindResult + FindResult, ResultArg } from "../common/lib/find"; import EPUBView from "./epub-view"; import SectionRenderer from "./section-renderer"; @@ -23,12 +23,12 @@ export class EPUBFindProcessor implements FindProcessor { private _cancelled = false; - private readonly _onSetFindState?: (state?: FindState) => void; + private readonly _onSetFindState?: (result: ResultArg) => void; constructor(options: { view: EPUBView, findState: FindState, - onSetFindState?: (state?: FindState) => void, + onSetFindState?: (result: ResultArg) => void, }) { this.view = options.view; this.findState = options.findState; @@ -182,12 +182,9 @@ export class EPUBFindProcessor implements FindProcessor { snippets.push(...processor.getSnippets()); } this._onSetFindState({ - ...this.findState, - result: { - total: this._totalResults, - index, - snippets, - } + total: this._totalResults, + index, + snippets, }); } } diff --git a/src/dom/snapshot/snapshot-view.ts b/src/dom/snapshot/snapshot-view.ts index e3f6f242..4dd38910 100644 --- a/src/dom/snapshot/snapshot-view.ts +++ b/src/dom/snapshot/snapshot-view.ts @@ -402,7 +402,20 @@ class SnapshotView extends DOMView { console.log('Initiating new search', state); this._find = new DefaultFindProcessor({ findState: { ...state }, - onSetFindState: this._options.onSetFindState, + onSetFindState: (result) => { + this._options.onSetFindState({ + ...state, + result: { + total: result.total, + index: result.index, + snippets: result.snippets, + annotation: ( + result.range + && this._getAnnotationFromRange(result.range.toRange(), 'highlight') + ) ?? undefined + } + }); + }, }); await this._find.run( this._searchContext, From 6c2f7807a60a24f64a1bf218ad42bda017be3c10 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Wed, 13 Nov 2024 15:47:48 -0500 Subject: [PATCH 02/24] Fix EPUB find annotation shortcuts --- src/dom/epub/find.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dom/epub/find.ts b/src/dom/epub/find.ts index 96ddbc84..c7cc1253 100644 --- a/src/dom/epub/find.ts +++ b/src/dom/epub/find.ts @@ -168,13 +168,17 @@ export class EPUBFindProcessor implements FindProcessor { let index = 0; let foundSelected = false; let snippets = []; + let range: PersistentRange | undefined; for (let processor of this._processors) { if (!processor) { continue; } if (this._selectedProcessor == processor) { - index += processor.position ?? 0; + let position = processor.position ?? 0; + index += position; foundSelected = true; + // TODO: Expose this in a nicer way + range = processor.getAnnotations()[position]?.range; } else if (!foundSelected) { index += processor.getResults().length; @@ -185,6 +189,7 @@ export class EPUBFindProcessor implements FindProcessor { total: this._totalResults, index, snippets, + range, }); } } From 726b27d3c524dbc3b626d33258bc3f70a1a5c198 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 14 Nov 2024 12:24:16 -0500 Subject: [PATCH 03/24] EPUB/Snapshot: Enable annotation focus keyboard navigation --- .../components/overlay/annotation-overlay.tsx | 6 +- src/dom/common/dom-view.tsx | 68 +++++++++++++++---- src/dom/epub/epub-view.ts | 7 +- src/dom/epub/flow.ts | 3 + 4 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/dom/common/components/overlay/annotation-overlay.tsx b/src/dom/common/components/overlay/annotation-overlay.tsx index de387795..cfdeccc9 100644 --- a/src/dom/common/components/overlay/annotation-overlay.tsx +++ b/src/dom/common/components/overlay/annotation-overlay.tsx @@ -376,7 +376,7 @@ let HighlightOrUnderline: React.FC = (props) => { // the whole outer containing the underline/highlight (potentially small) and the interactive s // (big) so that we get all the highlighted text to render in the drag image. return <> - + {rectGroup} {foreignObjects} {resizer} @@ -448,6 +448,7 @@ const Note: React.FC = (props) => { opacity={annotation.id ? '100%' : '50%'} selected={selected} large={true} + tabIndex={-1} onPointerDown={onPointerDown && (event => onPointerDown!(annotation, event))} onPointerUp={onPointerUp && (event => onPointerUp!(annotation, event))} onContextMenu={onContextMenu && (event => onContextMenu!(annotation, event))} @@ -778,6 +779,8 @@ let CommentIcon = React.forwardRef((props, ref) width={size} height={size} className="needs-pointer-events" + tabIndex={props.tabIndex} + data-annotation-id={props.annotation?.id} >
void; onPointerUp?: (event: React.PointerEvent) => void; onContextMenu?: (event: React.MouseEvent) => void; diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 30070e1a..304bb3e8 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -46,8 +46,8 @@ import { isSafari } from "../../common/lib/utilities"; import { - closestElement, getContainingBlock, - isElement + closestElement, + getContainingBlock } from "./lib/nodes"; import { debounce } from "../../common/lib/debounce"; import { @@ -142,6 +142,8 @@ abstract class DOMView { protected _outline!: OutlineItem[]; + protected _lastKeyboardFocusedAnnotationID: string | null = null; + scale = 1; protected constructor(options: DOMViewOptions) { @@ -753,22 +755,47 @@ abstract class DOMView { // Focusable elements in PDF view are annotations and overlays (links, citations, figures). // Once TAB is pressed, arrows can be used to navigate between them - let focusableElements: HTMLElement[] = []; - let focusedElementIndex = -1; - let focusedElement: HTMLElement | null = this._iframeDocument.activeElement as HTMLElement | null; - if (focusedElement?.getAttribute('tabindex') != '-1') { - focusedElement = null; - } - for (let element of this._iframeDocument.querySelectorAll('[tabindex="-1"]')) { - focusableElements.push(element as HTMLElement); - if (element === focusedElement) { - focusedElementIndex = focusableElements.length - 1; + let focusedElement = this._iframeDocument.activeElement as HTMLElement | SVGElement | null; + if (focusedElement === this._annotationShadowRoot.host) { + focusedElement = this._annotationShadowRoot.activeElement as HTMLElement | SVGElement | null; + } + let focusableElements = [ + ...this._iframeDocument.querySelectorAll('a, area'), + ...this._annotationShadowRoot.querySelectorAll('[tabindex="-1"]') + ] as (HTMLElement | SVGElement)[]; + focusableElements.sort((a, b) => { + let rangeA; + if (a.getRootNode() === this._annotationShadowRoot && a.hasAttribute('data-annotation-id')) { + rangeA = this.toDisplayedRange(this._annotationsByID.get(a.getAttribute('data-annotation-id')!)!.position); + } + if (!rangeA) { + rangeA = this._iframeDocument.createRange(); + rangeA.selectNode(a); + } + let rangeB; + if (b.getRootNode() === this._annotationShadowRoot && b.hasAttribute('data-annotation-id')) { + rangeB = this.toDisplayedRange(this._annotationsByID.get(b.getAttribute('data-annotation-id')!)!.position); + } + if (!rangeB) { + rangeB = this._iframeDocument.createRange(); + rangeB.selectNode(b); } + return rangeA.compareBoundaryPoints(Range.START_TO_START, rangeB); + }); + let focusedElementIndex = focusedElement ? focusableElements.indexOf(focusedElement) : -1; + if (focusedElementIndex === -1) { + focusedElement = null; } if (key === 'Escape' && !this._resizingAnnotationID) { if (this._selectedAnnotationIDs.length) { this._options.onSelectAnnotations([], event); + if (this._lastKeyboardFocusedAnnotationID) { + (this._annotationRenderRootEl.querySelector( + `[tabindex="-1"][data-annotation-id="${this._lastKeyboardFocusedAnnotationID}"]` + ) as HTMLElement | SVGElement | null) + ?.focus(); + } } else if (focusedElement) { focusedElement.blur(); @@ -807,17 +834,22 @@ abstract class DOMView { if (focusedElement) { if (!window.rtl && key === 'ArrowRight' || window.rtl && key === 'ArrowLeft' || key === 'ArrowDown') { - focusableElements[focusedElementIndex + 1]?.focus(); + focusableElements[(focusedElementIndex + 1) % focusableElements.length]?.focus(); event.preventDefault(); return; } else if (!window.rtl && key === 'ArrowLeft' || window.rtl && key === 'ArrowRight' || key === 'ArrowUp') { - focusableElements[focusedElementIndex - 1]?.focus(); + focusableElements[(focusedElementIndex - 1 + focusableElements.length) % focusableElements.length]?.focus(); event.preventDefault(); return; } else if (['Enter', 'Space'].includes(key)) { - if (focusedElement.classList.contains('highlight')) { + if (focusedElement.matches('a, area')) { + (focusedElement as HTMLElement).click(); + event.preventDefault(); + return; + } + else if (focusedElement.hasAttribute('data-annotation-id')) { let annotationID = focusedElement.getAttribute('data-annotation-id')!; let annotation = this._annotationsByID.get(annotationID); if (annotation) { @@ -825,6 +857,9 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length == 1) { this._openAnnotationPopup(annotation); } + this._lastKeyboardFocusedAnnotationID = annotationID; + focusedElement.blur(); + event.preventDefault(); return; } } @@ -1033,6 +1068,8 @@ abstract class DOMView { this._openAnnotationPopup(this._annotationsByID.get(id)!); } } + this._iframeDocument.body.focus(); + this._lastKeyboardFocusedAnnotationID = null; } this._handledPointerIDs.add(event.pointerId); }; @@ -1058,6 +1095,7 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length == 1) { this._openAnnotationPopup(this._annotationsByID.get(nextID)!); } + this._lastKeyboardFocusedAnnotationID = null; }; private _getAnnotationsAtPoint(clientX: number, clientY: number): string[] { diff --git a/src/dom/epub/epub-view.ts b/src/dom/epub/epub-view.ts index 5210f8d2..80966848 100644 --- a/src/dom/epub/epub-view.ts +++ b/src/dom/epub/epub-view.ts @@ -781,6 +781,11 @@ class EPUBView extends DOMView { protected override _handleKeyDown(event: KeyboardEvent) { let { key } = event; + super._handleKeyDown(event); + if (event.defaultPrevented) { + return; + } + if (!event.shiftKey) { if (key == 'ArrowLeft') { this.flow.navigateLeft(); @@ -793,8 +798,6 @@ class EPUBView extends DOMView { return; } } - - super._handleKeyDown(event); } protected override _updateViewState() { diff --git a/src/dom/epub/flow.ts b/src/dom/epub/flow.ts index e0c364a4..0b249193 100644 --- a/src/dom/epub/flow.ts +++ b/src/dom/epub/flow.ts @@ -738,6 +738,9 @@ export class PaginatedFlow extends AbstractFlow { } private _handleKeyDown = (event: KeyboardEvent) => { + if (event.defaultPrevented) { + return; + } let { key, shiftKey } = event; // Left/right arrows are handled in EPUBView if (!shiftKey) { From 78b1f79c105598e7ac441c1b2cd76e98d2410775 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 14 Nov 2024 12:39:46 -0500 Subject: [PATCH 04/24] EPUB/Snapshot: Allow moving note annotations with keyboard --- src/dom/common/dom-view.tsx | 95 +++++++++++++++++++++++++------------ src/dom/common/lib/nodes.ts | 13 +++-- 2 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 304bb3e8..1865afac 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -47,7 +47,7 @@ import { } from "../../common/lib/utilities"; import { closestElement, - getContainingBlock + getContainingBlock, isBlock } from "./lib/nodes"; import { debounce } from "../../common/lib/debounce"; import { @@ -869,41 +869,76 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length === 1 && (key.endsWith('Shift-ArrowLeft') || key.endsWith('Shift-ArrowRight'))) { - let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); - let granularity = event.altKey ? 'word' : 'character'; - let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; - let selection = this._iframeDocument.getSelection()!; - let oldRange = this.toDisplayedRange(annotation.position)!; - selection.removeAllRanges(); - selection.addRange(oldRange); - if (resizeStart) { - selection.collapseToStart(); - } - else { - selection.collapseToEnd(); - } - selection.modify( - 'move', - key.endsWith('ArrowRight') ? 'right' : 'left', - granularity - ); - let newRange = selection.getRangeAt(0); - if (resizeStart) { - newRange.setEnd(oldRange.endContainer, oldRange.endOffset); + if (annotation.type === 'note') { + let walker = this._iframeDocument.createTreeWalker( + this._iframeDocument.body, + NodeFilter.SHOW_ELEMENT, + node => (isBlock(node as Element) && !node.contains(oldRange.startContainer) + ? NodeFilter.FILTER_ACCEPT + : NodeFilter.FILTER_SKIP), + ); + walker.currentNode = oldRange.startContainer; + + let range = this._iframeDocument.createRange(); + if (key.endsWith('ArrowRight')) { + walker.nextNode(); + } + else { + walker.previousNode(); + } + range.selectNode(walker.currentNode); + let selector = this.toSelector(range); + if (selector) { + annotation.position = selector; + this._navigateToSelector(selector, { block: 'center', behavior: 'smooth' }); + } + else { + console.warn('Invalid selector after resize'); + } + this._options.onUpdateAnnotations([annotation]); } else { - newRange.setStart(oldRange.startContainer, oldRange.startOffset); - } + let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); + let granularity = event.altKey ? 'word' : 'character'; + let selection = this._iframeDocument.getSelection()!; + + selection.removeAllRanges(); + selection.addRange(oldRange); + if (resizeStart) { + selection.collapseToStart(); + } + else { + selection.collapseToEnd(); + } + selection.modify( + 'move', + key.endsWith('ArrowRight') ? 'right' : 'left', + granularity + ); + let newRange = selection.getRangeAt(0); + if (resizeStart) { + newRange.setEnd(oldRange.endContainer, oldRange.endOffset); + } + else { + newRange.setStart(oldRange.startContainer, oldRange.startOffset); + } - if (newRange.collapsed) { - return; - } + if (newRange.collapsed) { + return; + } - annotation.position = this.toSelector(newRange)!; - this._options.onUpdateAnnotations([annotation]); - selection.removeAllRanges(); + let selector = this.toSelector(newRange); + if (selector) { + annotation.position = selector; + } + else { + console.warn('Invalid selector after resize'); + } + this._options.onUpdateAnnotations([annotation]); + selection.removeAllRanges(); + } event.preventDefault(); return; diff --git a/src/dom/common/lib/nodes.ts b/src/dom/common/lib/nodes.ts index da4eeb09..9fafb484 100644 --- a/src/dom/common/lib/nodes.ts +++ b/src/dom/common/lib/nodes.ts @@ -54,16 +54,15 @@ export function* closestAll(element: Element, selector: string): Generator Date: Thu, 14 Nov 2024 14:48:16 -0500 Subject: [PATCH 05/24] Only focus elements that are already in view And don't scroll when focusing. --- src/dom/common/dom-view.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 1865afac..d729cb1a 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -763,6 +763,9 @@ abstract class DOMView { ...this._iframeDocument.querySelectorAll('a, area'), ...this._annotationShadowRoot.querySelectorAll('[tabindex="-1"]') ] as (HTMLElement | SVGElement)[]; + focusableElements = focusableElements.filter( + el => isPageRectVisible(getBoundingPageRect(el), this._iframeWindow, 0) + ); focusableElements.sort((a, b) => { let rangeA; if (a.getRootNode() === this._annotationShadowRoot && a.hasAttribute('data-annotation-id')) { @@ -819,7 +822,7 @@ abstract class DOMView { if (!focusedElement) { // In PDF view the first visible object (annotation, overlay) is focused if (focusableElements.length) { - focusableElements[0].focus(); + focusableElements[0].focus({ preventScroll: true }); } else { this._options.onTabOut(); @@ -834,12 +837,14 @@ abstract class DOMView { if (focusedElement) { if (!window.rtl && key === 'ArrowRight' || window.rtl && key === 'ArrowLeft' || key === 'ArrowDown') { - focusableElements[(focusedElementIndex + 1) % focusableElements.length]?.focus(); + focusableElements[(focusedElementIndex + 1) % focusableElements.length] + ?.focus({ preventScroll: true }); event.preventDefault(); return; } else if (!window.rtl && key === 'ArrowLeft' || window.rtl && key === 'ArrowRight' || key === 'ArrowUp') { - focusableElements[(focusedElementIndex - 1 + focusableElements.length) % focusableElements.length]?.focus(); + focusableElements[(focusedElementIndex - 1 + focusableElements.length) % focusableElements.length] + ?.focus({ preventScroll: true }); event.preventDefault(); return; } From 4ce5ecf5bcc7cbf0827db8680b527983acaf2dcc Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 14 Nov 2024 15:07:03 -0500 Subject: [PATCH 06/24] Calculate focus state lazily --- src/dom/common/dom-view.tsx | 139 +++++++++++++++++++++++------------- 1 file changed, 88 insertions(+), 51 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index d729cb1a..76f6e9f1 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -366,6 +366,78 @@ abstract class DOMView { protected _tryUseToolDebounced = debounce(this._tryUseTool.bind(this), 500); + protected _getFocusState() { + let getFocusedElement = () => { + let focusedElement = this._iframeDocument.activeElement as HTMLElement | SVGElement | null; + if (focusedElement === this._annotationShadowRoot.host) { + focusedElement = this._annotationShadowRoot.activeElement as HTMLElement | SVGElement | null; + if (!focusedElement?.matches('[tabindex="-1"]')) { + focusedElement = null; + } + } + else if (!focusedElement?.matches('a, area')) { + focusedElement = null; + } + return focusedElement; + }; + + let getFocusedElementIndex = () => { + return obj.focusedElement ? obj.focusableElements.indexOf(obj.focusedElement) : -1; + }; + + let getFocusableElements = () => { + let focusableElements = [ + ...this._iframeDocument.querySelectorAll('a, area'), + ...this._annotationShadowRoot.querySelectorAll('[tabindex="-1"]') + ] as (HTMLElement | SVGElement)[]; + focusableElements = focusableElements.filter( + el => isPageRectVisible(getBoundingPageRect(el), this._iframeWindow, 0) + ); + focusableElements.sort((a, b) => { + let rangeA; + if (a.getRootNode() === this._annotationShadowRoot && a.hasAttribute('data-annotation-id')) { + rangeA = this.toDisplayedRange(this._annotationsByID.get(a.getAttribute('data-annotation-id')!)!.position); + } + if (!rangeA) { + rangeA = this._iframeDocument.createRange(); + rangeA.selectNode(a); + } + let rangeB; + if (b.getRootNode() === this._annotationShadowRoot && b.hasAttribute('data-annotation-id')) { + rangeB = this.toDisplayedRange(this._annotationsByID.get(b.getAttribute('data-annotation-id')!)!.position); + } + if (!rangeB) { + rangeB = this._iframeDocument.createRange(); + rangeB.selectNode(b); + } + return rangeA.compareBoundaryPoints(Range.START_TO_START, rangeB); + }); + return focusableElements; + }; + + let obj = { + get focusedElement() { + let value = getFocusedElement(); + Object.defineProperty(this, 'focusedElement', { value }); + return value; + }, + + get focusedElementIndex() { + let value = getFocusedElementIndex(); + Object.defineProperty(this, 'focusedElementIndex', { value }); + return value; + }, + + get focusableElements() { + let value = getFocusableElements(); + Object.defineProperty(this, 'focusableElements', { value }); + return value; + }, + }; + + return obj; + } + protected _handleViewUpdate() { this._updateViewState(); this._updateViewStats(); @@ -753,42 +825,7 @@ abstract class DOMView { let key = getKeyCombination(event); let code = getCodeCombination(event); - // Focusable elements in PDF view are annotations and overlays (links, citations, figures). - // Once TAB is pressed, arrows can be used to navigate between them - let focusedElement = this._iframeDocument.activeElement as HTMLElement | SVGElement | null; - if (focusedElement === this._annotationShadowRoot.host) { - focusedElement = this._annotationShadowRoot.activeElement as HTMLElement | SVGElement | null; - } - let focusableElements = [ - ...this._iframeDocument.querySelectorAll('a, area'), - ...this._annotationShadowRoot.querySelectorAll('[tabindex="-1"]') - ] as (HTMLElement | SVGElement)[]; - focusableElements = focusableElements.filter( - el => isPageRectVisible(getBoundingPageRect(el), this._iframeWindow, 0) - ); - focusableElements.sort((a, b) => { - let rangeA; - if (a.getRootNode() === this._annotationShadowRoot && a.hasAttribute('data-annotation-id')) { - rangeA = this.toDisplayedRange(this._annotationsByID.get(a.getAttribute('data-annotation-id')!)!.position); - } - if (!rangeA) { - rangeA = this._iframeDocument.createRange(); - rangeA.selectNode(a); - } - let rangeB; - if (b.getRootNode() === this._annotationShadowRoot && b.hasAttribute('data-annotation-id')) { - rangeB = this.toDisplayedRange(this._annotationsByID.get(b.getAttribute('data-annotation-id')!)!.position); - } - if (!rangeB) { - rangeB = this._iframeDocument.createRange(); - rangeB.selectNode(b); - } - return rangeA.compareBoundaryPoints(Range.START_TO_START, rangeB); - }); - let focusedElementIndex = focusedElement ? focusableElements.indexOf(focusedElement) : -1; - if (focusedElementIndex === -1) { - focusedElement = null; - } + let f = this._getFocusState(); if (key === 'Escape' && !this._resizingAnnotationID) { if (this._selectedAnnotationIDs.length) { @@ -800,8 +837,8 @@ abstract class DOMView { ?.focus(); } } - else if (focusedElement) { - focusedElement.blur(); + else if (f.focusedElement) { + f.focusedElement.blur(); } this._iframeWindow.getSelection()?.removeAllRanges(); // The keyboard shortcut was handled here, therefore no need to @@ -809,8 +846,8 @@ abstract class DOMView { return; } else if (key === 'Shift-Tab') { - if (focusedElement) { - focusedElement.blur(); + if (f.focusedElement) { + f.focusedElement.blur(); } else { this._options.onTabOut(true); @@ -819,10 +856,10 @@ abstract class DOMView { return; } else if (key === 'Tab') { - if (!focusedElement) { + if (!f.focusedElement) { // In PDF view the first visible object (annotation, overlay) is focused - if (focusableElements.length) { - focusableElements[0].focus({ preventScroll: true }); + if (f.focusableElements.length) { + f.focusableElements[0].focus({ preventScroll: true }); } else { this._options.onTabOut(); @@ -835,27 +872,27 @@ abstract class DOMView { return; } - if (focusedElement) { + if (f.focusedElement) { if (!window.rtl && key === 'ArrowRight' || window.rtl && key === 'ArrowLeft' || key === 'ArrowDown') { - focusableElements[(focusedElementIndex + 1) % focusableElements.length] + f.focusableElements[(f.focusedElementIndex + 1) % f.focusableElements.length] ?.focus({ preventScroll: true }); event.preventDefault(); return; } else if (!window.rtl && key === 'ArrowLeft' || window.rtl && key === 'ArrowRight' || key === 'ArrowUp') { - focusableElements[(focusedElementIndex - 1 + focusableElements.length) % focusableElements.length] + f.focusableElements[(f.focusedElementIndex - 1 + f.focusableElements.length) % f.focusableElements.length] ?.focus({ preventScroll: true }); event.preventDefault(); return; } else if (['Enter', 'Space'].includes(key)) { - if (focusedElement.matches('a, area')) { - (focusedElement as HTMLElement).click(); + if (f.focusedElement.matches('a, area')) { + (f.focusedElement as HTMLElement).click(); event.preventDefault(); return; } - else if (focusedElement.hasAttribute('data-annotation-id')) { - let annotationID = focusedElement.getAttribute('data-annotation-id')!; + else if (f.focusedElement.hasAttribute('data-annotation-id')) { + let annotationID = f.focusedElement.getAttribute('data-annotation-id')!; let annotation = this._annotationsByID.get(annotationID); if (annotation) { this._options.onSelectAnnotations([annotationID], event); @@ -863,7 +900,7 @@ abstract class DOMView { this._openAnnotationPopup(annotation); } this._lastKeyboardFocusedAnnotationID = annotationID; - focusedElement.blur(); + f.focusedElement.blur(); event.preventDefault(); return; } From f57a3a32ebf109cb7e970a2bfc5d1dc959a7caa6 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 10:31:09 -0500 Subject: [PATCH 07/24] Fix dummy selection remaining in some cases --- src/dom/common/dom-view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 76f6e9f1..b9171e35 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -966,6 +966,7 @@ abstract class DOMView { else { newRange.setStart(oldRange.startContainer, oldRange.startOffset); } + selection.removeAllRanges(); if (newRange.collapsed) { return; @@ -979,7 +980,6 @@ abstract class DOMView { console.warn('Invalid selector after resize'); } this._options.onUpdateAnnotations([annotation]); - selection.removeAllRanges(); } event.preventDefault(); From 88e6c4473f27ef668e3139dda39dc4f9d6eb4326 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 10:33:00 -0500 Subject: [PATCH 08/24] Prevent scroll on Escape --- src/dom/common/dom-view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index b9171e35..6c9a2bc6 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -834,7 +834,7 @@ abstract class DOMView { (this._annotationRenderRootEl.querySelector( `[tabindex="-1"][data-annotation-id="${this._lastKeyboardFocusedAnnotationID}"]` ) as HTMLElement | SVGElement | null) - ?.focus(); + ?.focus({ preventScroll: true }); } } else if (f.focusedElement) { From 5a97ae9bf0c13fff8983ec895f7199881b91a514 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 10:44:02 -0500 Subject: [PATCH 09/24] Create note in center of screen by default --- src/dom/common/dom-view.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 6c9a2bc6..a2c6eacd 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -1000,6 +1000,20 @@ abstract class DOMView { break; } let annotation = this._getAnnotationFromTextSelection(type, this._options.tools[type].color); + if (!annotation && type === 'note') { + let pos = caretPositionFromPoint( + this._iframeDocument, + this._iframeWindow.innerWidth / 2, + this._iframeWindow.innerHeight / 2 + ); + let elem = pos && closestElement(pos.offsetNode); + let block = elem && getContainingBlock(elem); + if (block) { + let range = this._iframeDocument.createRange(); + range.selectNode(block); + annotation = this._getAnnotationFromRange(range, type, this._options.tools[type].color); + } + } if (annotation) { this._options.onAddAnnotation(annotation, true); this._navigateToSelector(annotation.position, { From f9847780f3ca5fc4bbef88b7ba36b8c1e1766ea8 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 10:48:46 -0500 Subject: [PATCH 10/24] Update annotation text when moved/resized via keyboard --- src/dom/common/dom-view.tsx | 52 ++++++++++++++----------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index a2c6eacd..eb66d542 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -438,6 +438,20 @@ abstract class DOMView { return obj; } + protected _updateAnnotationRange(annotation: WADMAnnotation, range: Range): WADMAnnotation { + let newAnnotation = this._getAnnotationFromRange(range, annotation.type); + if (!newAnnotation) { + throw new Error('Invalid updated range'); + } + return { + ...annotation, + position: newAnnotation.position, + pageLabel: newAnnotation.pageLabel, + sortIndex: newAnnotation.sortIndex, + text: newAnnotation.text, + }; + } + protected _handleViewUpdate() { this._updateViewState(); this._updateViewStats(); @@ -923,23 +937,16 @@ abstract class DOMView { ); walker.currentNode = oldRange.startContainer; - let range = this._iframeDocument.createRange(); + let newRange = this._iframeDocument.createRange(); if (key.endsWith('ArrowRight')) { walker.nextNode(); } else { walker.previousNode(); } - range.selectNode(walker.currentNode); - let selector = this.toSelector(range); - if (selector) { - annotation.position = selector; - this._navigateToSelector(selector, { block: 'center', behavior: 'smooth' }); - } - else { - console.warn('Invalid selector after resize'); - } - this._options.onUpdateAnnotations([annotation]); + newRange.selectNode(walker.currentNode); + this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); + this._navigateToSelector(annotation.position, { block: 'center', behavior: 'smooth' }); } else { let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); @@ -972,14 +979,7 @@ abstract class DOMView { return; } - let selector = this.toSelector(newRange); - if (selector) { - annotation.position = selector; - } - else { - console.warn('Invalid selector after resize'); - } - this._options.onUpdateAnnotations([annotation]); + this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); } event.preventDefault(); @@ -1253,19 +1253,7 @@ abstract class DOMView { return; } let annotation = this._annotationsByID.get(id)!; - let updatedAnnotation = this._getAnnotationFromRange(range, annotation.type); - if (!updatedAnnotation) { - throw new Error('Invalid resized range'); - } - - annotation = { - ...annotation, - position: updatedAnnotation.position, - pageLabel: updatedAnnotation.pageLabel, - sortIndex: updatedAnnotation.sortIndex, - text: updatedAnnotation.text, - }; - this._options.onUpdateAnnotations([annotation]); + this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, range)]); // If the resize ends over a link, that somehow counts as a click in Fx // (even though the mousedown wasn't over the link - weird). Prevent that. From c708e7fe2a4aeb03d59ce07c01574e04e67f3b50 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 10:53:16 -0500 Subject: [PATCH 11/24] Disable annotation creation shortcuts when annotation is selected --- src/dom/common/dom-view.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index eb66d542..2d2037e0 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -986,7 +986,8 @@ abstract class DOMView { return; } - if (code === 'Ctrl-Alt-Digit1' || code === 'Ctrl-Alt-Digit2' || code === 'Ctrl-Alt-Digit3') { + if (!this._selectedAnnotationIDs.length + && (code === 'Ctrl-Alt-Digit1' || code === 'Ctrl-Alt-Digit2' || code === 'Ctrl-Alt-Digit3')) { let type: AnnotationType; switch (code) { case 'Ctrl-Alt-Digit1': From 00a60909bbdaae1fad3cc683bf40be89267fd446 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 11:03:30 -0500 Subject: [PATCH 12/24] Fix another case where selection could remain after resize --- src/dom/common/dom-view.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 2d2037e0..06608009 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -925,6 +925,8 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length === 1 && (key.endsWith('Shift-ArrowLeft') || key.endsWith('Shift-ArrowRight'))) { + event.preventDefault(); + let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; let oldRange = this.toDisplayedRange(annotation.position)!; if (annotation.type === 'note') { @@ -982,7 +984,6 @@ abstract class DOMView { this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); } - event.preventDefault(); return; } From 972a384ab4a1836a6070bc37f2e348daa7a9b8b6 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 11:04:42 -0500 Subject: [PATCH 13/24] fixup --- src/dom/common/dom-view.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 06608009..258ba199 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -925,8 +925,6 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length === 1 && (key.endsWith('Shift-ArrowLeft') || key.endsWith('Shift-ArrowRight'))) { - event.preventDefault(); - let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; let oldRange = this.toDisplayedRange(annotation.position)!; if (annotation.type === 'note') { @@ -977,13 +975,12 @@ abstract class DOMView { } selection.removeAllRanges(); - if (newRange.collapsed) { - return; + if (!newRange.collapsed) { + this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); } - - this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); } + event.preventDefault(); return; } From 4f5f33bbd8e852d7a5e87dd959a9b4c20dc321e6 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 11:37:28 -0500 Subject: [PATCH 14/24] Support up/down resizing --- src/dom/common/dom-view.tsx | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 258ba199..ed675c28 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -922,9 +922,7 @@ abstract class DOMView { } } - if (this._selectedAnnotationIDs.length === 1 - && (key.endsWith('Shift-ArrowLeft') - || key.endsWith('Shift-ArrowRight'))) { + if (this._selectedAnnotationIDs.length === 1 && key.includes('Shift-Arrow')) { let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; let oldRange = this.toDisplayedRange(annotation.position)!; if (annotation.type === 'note') { @@ -938,7 +936,8 @@ abstract class DOMView { walker.currentNode = oldRange.startContainer; let newRange = this._iframeDocument.createRange(); - if (key.endsWith('ArrowRight')) { + if (key.endsWith('Arrow' + (window.rtl ? 'Left' : 'Right')) + || key.endsWith('ArrowDown')) { walker.nextNode(); } else { @@ -950,7 +949,17 @@ abstract class DOMView { } else { let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); - let granularity = event.altKey ? 'word' : 'character'; + let granularity; + // Up/down set via granularity, not direction + if (event.key === 'ArrowUp' || event.key === 'ArrowDown') { + granularity = 'line'; + } + else if (event.altKey) { + granularity = 'word'; + } + else { + granularity = 'character'; + } let selection = this._iframeDocument.getSelection()!; selection.removeAllRanges(); @@ -963,7 +972,7 @@ abstract class DOMView { } selection.modify( 'move', - key.endsWith('ArrowRight') ? 'right' : 'left', + event.key === 'ArrowRight' || event.key === 'ArrowDown' ? 'right' : 'left', granularity ); let newRange = selection.getRangeAt(0); From a9cce1b220e1a44ed031fa158a3b71e609a127a9 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 13:24:04 -0500 Subject: [PATCH 15/24] Skip history when bringing annotation into view --- src/dom/common/dom-view.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index ed675c28..2d7f19ab 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -944,8 +944,13 @@ abstract class DOMView { walker.previousNode(); } newRange.selectNode(walker.currentNode); - this._options.onUpdateAnnotations([this._updateAnnotationRange(annotation, newRange)]); - this._navigateToSelector(annotation.position, { block: 'center', behavior: 'smooth' }); + annotation = this._updateAnnotationRange(annotation, newRange); + this._options.onUpdateAnnotations([annotation]); + this._navigateToSelector(annotation.position, { + block: 'center', + behavior: 'smooth', + skipHistory: true, + }); } else { let resizeStart = key.startsWith('Cmd-') || key.startsWith('Ctrl-'); @@ -1026,7 +1031,8 @@ abstract class DOMView { this._options.onAddAnnotation(annotation, true); this._navigateToSelector(annotation.position, { block: 'center', - behavior: 'smooth' + behavior: 'smooth', + skipHistory: true, }); this._iframeWindow.getSelection()?.removeAllRanges(); } From 1079dba67df7eeccc99550187c28bf105470e8ce Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 15 Nov 2024 14:24:48 -0500 Subject: [PATCH 16/24] Only navigate if needed --- src/dom/common/dom-view.tsx | 2 ++ src/dom/snapshot/snapshot-view.ts | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 2d7f19ab..cfe9a92a 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -950,6 +950,7 @@ abstract class DOMView { block: 'center', behavior: 'smooth', skipHistory: true, + ifNeeded: true, }); } else { @@ -1033,6 +1034,7 @@ abstract class DOMView { block: 'center', behavior: 'smooth', skipHistory: true, + ifNeeded: true, }); this._iframeWindow.getSelection()?.removeAllRanges(); } diff --git a/src/dom/snapshot/snapshot-view.ts b/src/dom/snapshot/snapshot-view.ts index 4dd38910..900f83e7 100644 --- a/src/dom/snapshot/snapshot-view.ts +++ b/src/dom/snapshot/snapshot-view.ts @@ -31,6 +31,7 @@ import DefaultFindProcessor, { createSearchContext } from "../common/lib/find"; import injectCSS from './stylesheets/inject.scss'; import darkReaderJS from '!!raw-loader!darkreader/darkreader'; import { DynamicThemeFix } from "darkreader"; +import { isPageRectVisible } from "../common/lib/rect"; class SnapshotView extends DOMView { protected _find: DefaultFindProcessor | null = null; @@ -318,6 +319,10 @@ class SnapshotView extends DOMView { // Non-element nodes and ranges don't have scrollIntoView(), // so scroll using a temporary element, removed synchronously let rect = getBoundingPageRect(range); + if (options.ifNeeded && isPageRectVisible(rect, this._iframeWindow, 0)) { + return; + } + let tempElem = this._iframeDocument.createElement('div'); tempElem.style.position = 'absolute'; tempElem.style.visibility = 'hidden'; From b7dd68408227bb0d6c32f84a37430a22774f41e7 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 21 Nov 2024 16:14:35 -0500 Subject: [PATCH 17/24] Hide annotation popup on keyboard resize --- src/dom/common/dom-view.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index cfe9a92a..8de0f544 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -995,6 +995,7 @@ abstract class DOMView { } } + this._options.onSetAnnotationPopup(null); event.preventDefault(); return; } From 130ac895466ad75e390ff8764688fa3038b614a7 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 21 Nov 2024 16:14:48 -0500 Subject: [PATCH 18/24] Show selected annotation popup on Enter --- src/dom/common/dom-view.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 8de0f544..949a3cdf 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -921,6 +921,9 @@ abstract class DOMView { } } } + else if (this._selectedAnnotationIDs.length === 1 && key === 'Enter') { + this._openAnnotationPopup(this._annotationsByID.get(this._selectedAnnotationIDs[0])!); + } if (this._selectedAnnotationIDs.length === 1 && key.includes('Shift-Arrow')) { let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; From 22f5f4f18b325cac136ac08b1a0400958b22244a Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 21 Nov 2024 16:21:52 -0500 Subject: [PATCH 19/24] Prevent click from focusing annotations --- src/dom/common/components/overlay/annotation-overlay.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/dom/common/components/overlay/annotation-overlay.tsx b/src/dom/common/components/overlay/annotation-overlay.tsx index cfdeccc9..8ac570c7 100644 --- a/src/dom/common/components/overlay/annotation-overlay.tsx +++ b/src/dom/common/components/overlay/annotation-overlay.tsx @@ -376,7 +376,13 @@ let HighlightOrUnderline: React.FC = (props) => { // the whole outer containing the underline/highlight (potentially small) and the interactive s // (big) so that we get all the highlighted text to render in the drag image. return <> - + e.preventDefault()} + data-annotation-id={annotation.id} + fill={annotation.color} + ref={outerGroupRef} + > {rectGroup} {foreignObjects} {resizer} @@ -780,6 +786,7 @@ let CommentIcon = React.forwardRef((props, ref) height={size} className="needs-pointer-events" tabIndex={props.tabIndex} + onPointerDown={e => e.preventDefault()} data-annotation-id={props.annotation?.id} >
Date: Thu, 21 Nov 2024 16:28:14 -0500 Subject: [PATCH 20/24] Tab out when annotation is selected --- src/dom/common/dom-view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 949a3cdf..3932517b 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -870,7 +870,7 @@ abstract class DOMView { return; } else if (key === 'Tab') { - if (!f.focusedElement) { + if (!f.focusedElement && this._iframeDocument.getSelection()!.isCollapsed && !this._selectedAnnotationIDs.length) { // In PDF view the first visible object (annotation, overlay) is focused if (f.focusableElements.length) { f.focusableElements[0].focus({ preventScroll: true }); From 476a6b6e4b713d9a8598ee2c099b8d4143481ba4 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 21 Nov 2024 16:47:10 -0500 Subject: [PATCH 21/24] Open note annotation popup after creation --- src/dom/common/components/overlay/annotation-overlay.tsx | 1 + src/dom/common/dom-view.tsx | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dom/common/components/overlay/annotation-overlay.tsx b/src/dom/common/components/overlay/annotation-overlay.tsx index 8ac570c7..9196bd66 100644 --- a/src/dom/common/components/overlay/annotation-overlay.tsx +++ b/src/dom/common/components/overlay/annotation-overlay.tsx @@ -772,6 +772,7 @@ let CommentIcon = React.forwardRef((props, ref) width={size} height={size} viewBox="0 0 24 24" + data-annotation-id={props.annotation?.id} ref={ref} > diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 3932517b..a829f696 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -1041,6 +1041,10 @@ abstract class DOMView { ifNeeded: true, }); this._iframeWindow.getSelection()?.removeAllRanges(); + if (type === 'note') { + this._renderAnnotations(true); + this._openAnnotationPopup(); + } } event.preventDefault(); return; @@ -1716,7 +1720,7 @@ export type DOMViewOptions = { onChangeViewState: (state: State, primary?: boolean) => void; onChangeViewStats: (stats: ViewStats) => void; onSetDataTransferAnnotations: (dataTransfer: DataTransfer, annotation: NewAnnotation | NewAnnotation[], fromText?: boolean) => void; - onAddAnnotation: (annotation: NewAnnotation, select?: boolean) => void; + onAddAnnotation: (annotation: NewAnnotation, select?: boolean) => WADMAnnotation; onUpdateAnnotations: (annotations: Annotation[]) => void; onOpenLink: (url: string) => void; onSelectAnnotations: (ids: string[], triggeringEvent?: KeyboardEvent | MouseEvent) => void; From a50beb5dcd0a075fa87244b4f52aaef4d163ccba Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 6 Dec 2024 12:01:13 -0500 Subject: [PATCH 22/24] Abort if annotation isn't in view --- src/dom/common/dom-view.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index a829f696..c651ecc9 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -927,12 +927,16 @@ abstract class DOMView { if (this._selectedAnnotationIDs.length === 1 && key.includes('Shift-Arrow')) { let annotation = this._annotationsByID.get(this._selectedAnnotationIDs[0])!; - let oldRange = this.toDisplayedRange(annotation.position)!; + let oldRange = this.toDisplayedRange(annotation.position); + if (!oldRange) { + event.preventDefault(); + return; + } if (annotation.type === 'note') { let walker = this._iframeDocument.createTreeWalker( this._iframeDocument.body, NodeFilter.SHOW_ELEMENT, - node => (isBlock(node as Element) && !node.contains(oldRange.startContainer) + node => (isBlock(node as Element) && !node.contains(oldRange!.startContainer) ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP), ); From fa60ca08ad15028cf2c8aaafc4ffa36d61701ac0 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 6 Dec 2024 12:04:01 -0500 Subject: [PATCH 23/24] Un-break note annotation dragging --- src/dom/common/components/overlay/annotation-overlay.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dom/common/components/overlay/annotation-overlay.tsx b/src/dom/common/components/overlay/annotation-overlay.tsx index 9196bd66..27a3deb4 100644 --- a/src/dom/common/components/overlay/annotation-overlay.tsx +++ b/src/dom/common/components/overlay/annotation-overlay.tsx @@ -787,7 +787,6 @@ let CommentIcon = React.forwardRef((props, ref) height={size} className="needs-pointer-events" tabIndex={props.tabIndex} - onPointerDown={e => e.preventDefault()} data-annotation-id={props.annotation?.id} >
Date: Fri, 6 Dec 2024 12:32:36 -0500 Subject: [PATCH 24/24] EPUB: Potentially prevent invalid annotations --- src/dom/common/dom-view.tsx | 22 ++++++++++++++++++++-- src/dom/epub/epub-view.ts | 5 +++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index c651ecc9..1ea5eb67 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -253,6 +253,12 @@ abstract class DOMView { protected abstract _updateViewStats(): void; + protected _getContainingRoot(node: Node): HTMLElement | null { + return this._iframeDocument.body.contains(node) + ? this._iframeDocument.body + : null; + } + // *** // Utilities - called in appropriate event handlers // *** @@ -933,8 +939,12 @@ abstract class DOMView { return; } if (annotation.type === 'note') { + let root = this._getContainingRoot(oldRange.startContainer); + if (!root) { + throw new Error('Annotation is outside of root?'); + } let walker = this._iframeDocument.createTreeWalker( - this._iframeDocument.body, + root, NodeFilter.SHOW_ELEMENT, node => (isBlock(node as Element) && !node.contains(oldRange!.startContainer) ? NodeFilter.FILTER_ACCEPT @@ -951,7 +961,15 @@ abstract class DOMView { walker.previousNode(); } newRange.selectNode(walker.currentNode); - annotation = this._updateAnnotationRange(annotation, newRange); + try { + annotation = this._updateAnnotationRange(annotation, newRange); + } + catch (e) { + // Reached the end of the section (EPUB) + // TODO: Allow movement between sections + event.preventDefault(); + return; + } this._options.onUpdateAnnotations([annotation]); this._navigateToSelector(annotation.position, { block: 'center', diff --git a/src/dom/epub/epub-view.ts b/src/dom/epub/epub-view.ts index 80966848..6699824d 100644 --- a/src/dom/epub/epub-view.ts +++ b/src/dom/epub/epub-view.ts @@ -520,6 +520,11 @@ class EPUBView extends DOMView { }; } + protected override _getContainingRoot(node: Node) { + return this._sectionRenderers.find(r => r.container.contains(node))?.container + ?? null; + } + private _upsertAnnotation(annotation: NewAnnotation) { let existingAnnotation = this._annotations.find( existingAnnotation => existingAnnotation.text === annotation!.text