From e80d72dad2d3e21df2158143e3f4198a06f93457 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Wed, 6 Nov 2024 18:52:16 +0200 Subject: [PATCH 1/3] Comments formatting --- packages/text-annotator/src/SelectionHandler.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 84d45071..84d1efbd 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -73,9 +73,12 @@ export const SelectionHandler = ( const onSelectionChange = debounce((evt: Event) => { const sel = document.getSelection(); - // This is to handle cases where the selection is "hijacked" by another element - // in a not-annotatable area. A rare case in theory. But rich text editors - // will like Quill do it... + /** + * This is to handle cases where the selection is "hijacked" + * by another element in a not-annotatable area. + * A rare case in theory. + * But rich text editors will like Quill do it. + */ if (isNotAnnotatable(sel.anchorNode)) { currentTarget = undefined; return; @@ -130,7 +133,6 @@ export const SelectionHandler = ( const hasChanged = annotatableRanges.length !== currentTarget.selector.length || annotatableRanges.some((r, i) => r.toString() !== currentTarget.selector[i]?.quote); - if (!hasChanged) return; currentTarget = { @@ -140,8 +142,8 @@ export const SelectionHandler = ( }; /** - * During mouse selection on the desktop, annotation won't usually exist while the selection is being edited. - * But it will be typical during keyboard or mobile handlebars selection! + * During mouse selection on the desktop, the annotation won't usually exist while the selection is being edited. + * But it'll be typical during selection via the keyboard or mobile's handlebars. */ if (store.getAnnotation(currentTarget.annotation)) { store.updateTarget(currentTarget, Origin.LOCAL); @@ -152,7 +154,7 @@ export const SelectionHandler = ( }); /** - * Select events don't carry information about the mouse button + * Select events don't carry information about the mouse button. * Therefore, to prevent right-click selection, we need to listen * to the initial pointerdown event and remember the button */ From 7eb3603c8c2fc9f0823623e0b14c35ad2820576e Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Wed, 6 Nov 2024 18:54:17 +0200 Subject: [PATCH 2/3] Added update target call only when it's newer --- .../text-annotator/src/SelectionHandler.ts | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 84d1efbd..ce31b82b 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -169,20 +169,6 @@ export const SelectionHandler = ( isLeftClick = lastDownEvent.button === 0; }; - // Helper - const upsertCurrentTarget = () => { - const exists = store.getAnnotation(currentTarget.annotation); - if (exists) { - store.updateTarget(currentTarget); - } else { - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - } - } - const onPointerUp = (evt: PointerEvent) => { if (isNotAnnotatable(evt.target as Node) || !isLeftClick) return; @@ -319,6 +305,29 @@ export const SelectionHandler = ( hotkeys(ARROW_KEYS.join(','), { keydown: true, keyup: false }, handleArrowKeyPress); + // Helper + const upsertCurrentTarget = () => { + const existingAnnotation = store.getAnnotation(currentTarget.annotation); + if (!existingAnnotation) { + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + return; + } + + const { target: { updated: existingTargetUpdated } } = existingAnnotation; + const { updated: currentTargetUpdated } = currentTarget; + if ( + !existingAnnotation || + !currentTargetUpdated || + existingTargetUpdated < currentTargetUpdated + ) { + store.updateTarget(currentTarget); + } + }; + container.addEventListener('pointerdown', onPointerDown); document.addEventListener('pointerup', onPointerUp); document.addEventListener('contextmenu', onContextMenu); From e9fbea8869ed95f9506923efa63432803cf46e6f Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko <68850090+oleksandr-danylchenko@users.noreply.github.com> Date: Fri, 8 Nov 2024 12:17:52 +0200 Subject: [PATCH 3/3] Added proper `existingTargetUpdated` existence check --- packages/text-annotator/src/SelectionHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index ce31b82b..510932bd 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -320,7 +320,7 @@ export const SelectionHandler = ( const { target: { updated: existingTargetUpdated } } = existingAnnotation; const { updated: currentTargetUpdated } = currentTarget; if ( - !existingAnnotation || + !existingTargetUpdated || !currentTargetUpdated || existingTargetUpdated < currentTargetUpdated ) {