Skip to content

Commit

Permalink
Fixes:
Browse files Browse the repository at this point in the history 
- Image flickering
- Changes keyboard shortcuts once again
- Scrolls into view highlight/underline annotations created with Ctrl-Alt-1/2
- Fix ghost focus outline
  • Loading branch information
@mrtcode
mrtcode committed Sep 25, 2024
1 parent abe6f23 commit 3bb5463
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 44 deletions.
15 changes: 12 additions & 3 deletions src/common/components/common/preview.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions src/common/components/view-popup/find-popup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ function FindPopup({ params, onChange, onFindNext, onFindPrevious, onAddAnnotati
else if (code === 'Ctrl-Alt-Digit1') {
preventInputRef.current = true;
if (params.result?.annotation) {
onAddAnnotation({ ...params.result.annotation, type: 'highlight' });
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' });
onAddAnnotation({ ...params.result.annotation, type: 'underline' }, true);
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions src/common/reader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,13 @@ class Reader {
this._onChangeSidebarWidth(width);
}}
onResizeSplitView={this.setSplitViewSize.bind(this)}
onAddAnnotation={(annotation) => {
this._annotationManager.addAnnotation(annotation);
this.setSelectedAnnotations([]);
onAddAnnotation={(annotation, select) => {
annotation = this._annotationManager.addAnnotation(annotation);
if (select) {
this.setSelectedAnnotations([annotation.id]);
} else {
this.setSelectedAnnotations([]);
}
}}
onUpdateAnnotations={(annotations) => {
this._annotationManager.updateAnnotations(annotations);
Expand Down Expand Up @@ -1169,7 +1173,8 @@ class Reader {
// Don't navigate to annotation or focus comment if opening a context menu
if (!triggeringEvent || triggeringEvent.button !== 2) {
if (triggeredFromView) {
if (['note', 'highlight', 'underline'].includes(annotation.type) && !annotation.comment) {
if (['note', 'highlight', 'underline'].includes(annotation.type)
&& !annotation.comment && (!triggeringEvent || !('key' in triggeringEvent))) {
this._enableAnnotationDeletionFromComment = true;
setTimeout(() => {
let content;
Expand Down
103 changes: 68 additions & 35 deletions src/pdf/pdf-view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ import {
import {
debounceUntilScrollFinishes,
getCodeCombination,
getKeyCombination
} from '../common/lib/utilities';
import {
getKeyCombination,
getAffectedAnnotations,
isFirefox,
isMac,
isLinux,
isWin,
isFirefox,
isSafari,
throttle
} from '../common/lib/utilities';
Expand Down Expand Up @@ -677,8 +677,26 @@ class PDFView {

setAnnotations(annotations) {
let affected = getAffectedAnnotations(this._annotations, annotations, true);
this._annotations = annotations;
let { created, updated, deleted } = affected;
this._annotations = annotations;
if (this._focusedObject?.type === 'annotation') {
if (updated.find(x => x.id === this._focusedObject.object.id)) {
this._focusedObject.object = updated.find(x => x.id === this._focusedObject.object.id);
}
else if (deleted.find(x => x.id === this._focusedObject.object.id)) {
this._focusedObject = null;
}
}

if (this._lastFocusedObject?.type === 'annotation') {
if (updated.find(x => x.id === this._lastFocusedObject.object.id)) {
this._lastFocusedObject.object = updated.find(x => x.id === this._lastFocusedObject.object.id);
}
else if (deleted.find(x => x.id === this._lastFocusedObject.object.id)) {
this._lastFocusedObject = null;
}
}

let all = [...created, ...updated, ...deleted];
let pageIndexes = getPageIndexesFromAnnotations(all);
this._render(pageIndexes);
Expand Down Expand Up @@ -2559,7 +2577,7 @@ class PDFView {

if (
['note', 'text', 'image', 'ink'].includes(type)
&& ['Shift-ArrowLeft', 'Shift-ArrowRight', 'Shift-ArrowUp', 'Shift-ArrowDown'].includes(key)
&& ['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(key)
) {
let rect;
if (annotation.type === 'ink') {
Expand All @@ -2570,16 +2588,16 @@ class PDFView {
}
let dx = 0;
let dy = 0;
if (key === 'Shift-ArrowLeft' && rect[0] >= STEP + PADDING) {
if (key === 'ArrowLeft' && rect[0] >= STEP + PADDING) {
dx = -STEP;
}
else if (key === 'Shift-ArrowRight' && rect[2] <= viewBox[2] - STEP - PADDING) {
else if (key === 'ArrowRight' && rect[2] <= viewBox[2] - STEP - PADDING) {
dx = STEP;
}
else if (key === 'Shift-ArrowDown' && rect[1] >= STEP + PADDING) {
else if (key === 'ArrowDown' && rect[1] >= STEP + PADDING) {
dy = -STEP;
}
else if (key === 'Shift-ArrowUp' && rect[3] <= viewBox[3] - STEP - PADDING) {
else if (key === 'ArrowUp' && rect[3] <= viewBox[3] - STEP - PADDING) {
dy = STEP;
}
if (dx || dy) {
Expand Down Expand Up @@ -2628,19 +2646,34 @@ class PDFView {
event.preventDefault();
}
else if (['highlight', 'underline'].includes(type)
&& ['Cmd-Shift-ArrowLeft', 'Cmd-Shift-ArrowRight', 'Cmd-Shift-ArrowUp', 'Cmd-Shift-ArrowDown'].includes(key)) {
&& (
isMac() && ['Cmd-Shift-ArrowLeft', 'Cmd-Shift-ArrowRight', 'Cmd-Shift-ArrowUp', 'Cmd-Shift-ArrowDown'].includes(key)
|| (isWin() || isLinux()) && ['Alt-Shift-ArrowLeft', 'Alt-Shift-ArrowRight', 'Alt-Shift-ArrowUp', 'Alt-Shift-ArrowDown'].includes(key)
)) {
let selectionRanges = getSelectionRangesByPosition(this._pdfPages, annotation.position);
selectionRanges = getReversedSelectionRanges(selectionRanges);
if (key === 'Cmd-Shift-ArrowLeft') {
if (
isMac() && key === 'Cmd-Shift-ArrowLeft'
|| (isWin() || isLinux()) && key === 'Alt-Shift-ArrowLeft'
) {
selectionRanges = getModifiedSelectionRanges(this._pdfPages, selectionRanges, 'left');
}
else if (key === 'Cmd-Shift-ArrowRight') {
else if (
isMac() && key === 'Cmd-Shift-ArrowRight'
|| (isWin() || isLinux()) && key === 'Alt-Shift-ArrowRight'
) {
selectionRanges = getModifiedSelectionRanges(this._pdfPages, selectionRanges, 'right');
}
else if (key === 'Cmd-Shift-ArrowUp') {
else if (
isMac() && key === 'Cmd-Shift-ArrowUp'
|| (isWin() || isLinux()) && key === 'Alt-Shift-ArrowUp'
) {
selectionRanges = getModifiedSelectionRanges(this._pdfPages, selectionRanges, 'up');
}
else if (key === 'Cmd-Shift-ArrowDown') {
else if (
isMac() && key === 'Cmd-Shift-ArrowDown'
|| (isWin() || isLinux()) && key === 'Cmd-Shift-ArrowDown'
) {
selectionRanges = getModifiedSelectionRanges(this._pdfPages, selectionRanges, 'down');
}
if (!(selectionRanges.length === 1
Expand All @@ -2653,35 +2686,35 @@ class PDFView {
event.preventDefault();
}
else if (
['highlight', 'underline', 'text', 'image', 'ink'].includes(type)
&& ['Alt-Shift-ArrowLeft',
'Alt-Shift-ArrowRight',
'Alt-Shift-ArrowUp',
'Alt-Shift-ArrowDown'].includes(key)
['text', 'image', 'ink'].includes(type)
&& (
isMac() && ['Shift-ArrowLeft', 'Shift-ArrowRight', 'Shift-ArrowUp', 'Shift-ArrowDown'].includes(key)
|| (isWin() || isLinux()) && ['Shift-ArrowLeft', 'Shift-ArrowRight', 'Shift-ArrowUp', 'Shift-ArrowDown'].includes(key)
)
) {
if (type === 'ink') {
let rect = getPositionBoundingRect(position);
let r1 = rect.slice();
let ratio = (rect[2] - rect[0]) / (rect[3] - rect[1]);
let [, y, x] = rect;
let [, y] = rect;

if (key === 'Alt-Shift-ArrowLeft') {
if (key === 'Shift-ArrowLeft') {
rect[2] -= STEP;
rect[1] += STEP / ratio;
modified = true;
}
else if (key === 'Alt-Shift-ArrowRight') {
else if (key === 'Shift-ArrowRight') {
rect[2] += STEP;
rect[1] -= STEP / ratio;
modified = true;
}
else if (key === 'Alt-Shift-ArrowDown') {
else if (key === 'Shift-ArrowDown') {
modified = true;
y -= STEP;
rect[2] += STEP * ratio;
rect[1] = y;
}
else if (key === 'Alt-Shift-ArrowUp') {
else if (key === 'Shift-ArrowUp') {
y += STEP;
rect[2] -= STEP * ratio;
rect[1] = y;
Expand All @@ -2697,22 +2730,22 @@ class PDFView {
let rect = position.rects[0].slice();

let [, y, x] = rect;
if (key === 'Alt-Shift-ArrowLeft') {
if (key === 'Shift-ArrowLeft') {
x -= STEP;
rect[2] = x < rect[0] + MIN_IMAGE_ANNOTATION_SIZE && rect[0] + MIN_IMAGE_ANNOTATION_SIZE || x;
modified = true;
}
else if (key === 'Alt-Shift-ArrowRight') {
else if (key === 'Shift-ArrowRight') {
x += STEP;
rect[2] = x < viewBox[2] && x || viewBox[2];
modified = true;
}
else if (key === 'Alt-Shift-ArrowDown') {
else if (key === 'Shift-ArrowDown') {
y -= STEP;
rect[1] = y > viewBox[1] && y || viewBox[1];
modified = true;
}
else if (key === 'Alt-Shift-ArrowUp') {
else if (key === 'Shift-ArrowUp') {
y += STEP;
rect[1] = y > rect[3] - MIN_IMAGE_ANNOTATION_SIZE && rect[3] - MIN_IMAGE_ANNOTATION_SIZE || y;
modified = true;
Expand All @@ -2726,12 +2759,12 @@ class PDFView {
let rect = position.rects[0].slice();
const MIN_TEXT_ANNOTATION_WIDTH = 10;
let x = rect[2];
if (key === 'Alt-Shift-ArrowLeft') {
if (key === 'Shift-ArrowLeft') {
x -= STEP;
rect[2] = x < rect[0] + MIN_TEXT_ANNOTATION_WIDTH && rect[0] + MIN_TEXT_ANNOTATION_WIDTH || x;
modified = true;
}
else if (key === 'Alt-Shift-ArrowRight') {
else if (key === 'Shift-ArrowRight') {
x += STEP;
rect[2] = x;
modified = true;
Expand Down Expand Up @@ -2777,10 +2810,10 @@ class PDFView {
&& !this._selectionRanges[0].collapsed
&& !this._readOnly
) {

let annotation = this._getAnnotationFromSelectionRanges(this._selectionRanges, 'highlight');
annotation.sortIndex = getSortIndex(this._pdfPages, annotation.position);
this._onAddAnnotation(annotation);
this._onAddAnnotation(annotation, true);
this.navigateToPosition(annotation.position);
this._setSelectionRanges();
}
else if (
Expand All @@ -2791,7 +2824,8 @@ class PDFView {
) {
let annotation = this._getAnnotationFromSelectionRanges(this._selectionRanges, 'underline');
annotation.sortIndex = getSortIndex(this._pdfPages, annotation.position);
this._onAddAnnotation(annotation);
this._onAddAnnotation(annotation, true);
this.navigateToPosition(annotation.position);
this._setSelectionRanges();
}
else if (code === 'Ctrl-Alt-Digit3' && !this._readOnly) {
Expand Down Expand Up @@ -2944,7 +2978,6 @@ class PDFView {
}

if (this._focusedObject && !this._selectedAnnotationIDs.length) {
let { key } = event;
if (key === 'ArrowLeft') {
this._focusNext('left');
event.preventDefault();
Expand Down

0 comments on commit 3bb5463

Please sign in to comment.