From 47066df735617b950436b9c6ab34c21af2db563c Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 10:18:43 +0200 Subject: [PATCH 01/23] Selection behavior change: annotation gets created on pointer up, not immediately --- .../text-annotator/src/SelectionHandler.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 164c4ed7..91b0ba49 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -95,10 +95,8 @@ export const SelectionHandler = ( } else if (sel.isCollapsed && timeDifference < CLICK_TIMEOUT) { - /* - Firefox doesn't fire the 'selectstart' when user clicks - over the text, which collapses the selection - */ + // Firefox doesn't fire the 'selectstart' when user clicks + // over the text, which collapses the selection onSelectStart(lastDownEvent || evt); } @@ -142,9 +140,11 @@ export const SelectionHandler = ( updated: new Date() }; + // On destkop, the annotation won't usually exist while the selection is + // being edited. But it will typcially be the case on mobile! if (store.getAnnotation(currentTarget.annotation)) { store.updateTarget(currentTarget, Origin.LOCAL); - } else { + } /* else { // Proper lifecycle management: clear selection first... selection.clear(); @@ -158,6 +158,7 @@ export const SelectionHandler = ( // ...then make the new annotation the current selection selection.userSelect(currentTarget.annotation, lastDownEvent); } + */ }); /** @@ -218,8 +219,19 @@ export const SelectionHandler = ( if (sel?.isCollapsed && timeDifference < CLICK_TIMEOUT) { currentTarget = undefined; clickSelect(); - } else if (currentTarget && store.getAnnotation(currentTarget.annotation)) { - selection.userSelect(currentTarget.annotation, evt); + } else if (currentTarget) { + // Proper lifecycle management: clear selection first... + selection.clear(); + + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + + // ...then make the new annotation the current selection + selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); } }); } From f786661f5ab7a1228f7281bd0effdfdb8973c167 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 10:45:56 +0200 Subject: [PATCH 02/23] Removed pointerup/keydown event requirement from popup --- .../src/TextAnnotatorPopup/TextAnnotatorPopup.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index d001400e..237fb785 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -73,12 +73,13 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const { getFloatingProps } = useInteractions([dismiss, role]); const selectedKey = selected.map(a => a.annotation.id).join('-'); + useEffect(() => { // Ignore all selection changes except those accompanied by a user event. - if (selected.length > 0 && event) { - setOpen(event.type === 'pointerup' || event.type === 'keydown'); + if (selected.length > 0) { // && event) { + setOpen(true); // event.type === 'pointerup' || event.type === 'keydown'); } - }, [selectedKey, event]); + }, [selectedKey /*, event */]); useEffect(() => { // Close the popup if the selection is cleared From b96f04a192e3a68f1fc6e4c6c8e87a618027c9d5 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 10:48:18 +0200 Subject: [PATCH 03/23] Formatting --- .../src/TextAnnotatorPopup/TextAnnotatorPopup.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index 237fb785..b87443f1 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -1,4 +1,6 @@ -import React, { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; +import { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; +import { useAnnotator, useSelection } from '@annotorious/react'; +import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; import { autoUpdate, flip, @@ -13,9 +15,6 @@ import { useRole } from '@floating-ui/react'; -import { useAnnotator, useSelection } from '@annotorious/react'; -import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; - import './TextAnnotatorPopup.css'; interface TextAnnotationPopupProps { @@ -162,4 +161,4 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { ) : null; -}; +} From c14ed9909d09741d83a325c64d8bf705c5941bcf Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 11:06:59 +0200 Subject: [PATCH 04/23] Keyboard selection --- .../text-annotator/src/SelectionHandler.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 91b0ba49..b03caadb 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -236,6 +236,23 @@ export const SelectionHandler = ( }); } + const onKeyup = (evt: KeyboardEvent) => { + if (evt.key === 'Shift' && currentTarget) { + // Proper lifecycle management: clear selection first... + selection.clear(); + + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + + // ...then make the new annotation the current selection + selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + } + } + hotkeys(SELECTION_KEYS.join(','), { element: container, keydown: true, keyup: false }, (evt) => { if (!evt.repeat) lastDownEvent = cloneKeyboardEvent(evt); @@ -267,6 +284,7 @@ export const SelectionHandler = ( document.addEventListener('pointerup', onPointerUp); if (annotatingEnabled) { + container.addEventListener('keyup', onKeyup); container.addEventListener('selectstart', onSelectStart); document.addEventListener('selectionchange', onSelectionChange); } @@ -275,6 +293,7 @@ export const SelectionHandler = ( container.removeEventListener('pointerdown', onPointerDown); document.removeEventListener('pointerup', onPointerUp); + container.removeEventListener('keyup', onKeyup); container.removeEventListener('selectstart', onSelectStart); document.removeEventListener('selectionchange', onSelectionChange); From 9e3f045c81c0238775798b5aabf420ee8c5d8eb2 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 11:13:29 +0200 Subject: [PATCH 05/23] Addresses #139 in the revised selection model --- packages/text-annotator/src/SelectionHandler.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index b03caadb..dbe05df8 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -117,6 +117,8 @@ export const SelectionHandler = ( store.deleteAnnotation(currentTarget.annotation); } + currentTarget = undefined; + return; } @@ -144,21 +146,7 @@ export const SelectionHandler = ( // being edited. But it will typcially be the case on mobile! if (store.getAnnotation(currentTarget.annotation)) { store.updateTarget(currentTarget, Origin.LOCAL); - } /* else { - // Proper lifecycle management: clear selection first... - selection.clear(); - - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - - // ...then make the new annotation the current selection - selection.userSelect(currentTarget.annotation, lastDownEvent); } - */ }); /** From 9461812c76d82bb834ac9a751002b0cfc8df8a9c Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 11:54:44 +0200 Subject: [PATCH 06/23] Bugfix --- .../text-annotator/src/SelectionHandler.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index dbe05df8..ee67406d 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -117,8 +117,6 @@ export const SelectionHandler = ( store.deleteAnnotation(currentTarget.annotation); } - currentTarget = undefined; - return; } @@ -201,7 +199,7 @@ export const SelectionHandler = ( * @see https://github.com/recogito/text-annotator-js/issues/136 */ setTimeout(() => { - const sel = document.getSelection() + const sel = document.getSelection(); // Just a click, not a selection if (sel?.isCollapsed && timeDifference < CLICK_TIMEOUT) { @@ -226,18 +224,22 @@ export const SelectionHandler = ( const onKeyup = (evt: KeyboardEvent) => { if (evt.key === 'Shift' && currentTarget) { - // Proper lifecycle management: clear selection first... - selection.clear(); - - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - - // ...then make the new annotation the current selection - selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + const sel = document.getSelection(); + + if (!sel.isCollapsed) { + // Proper lifecycle management: clear selection first... + selection.clear(); + + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + + // ...then make the new annotation the current selection + selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + } } } From e26b657fc115f8243c9bb75d1ca5cacf8550df0f Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 12:09:55 +0200 Subject: [PATCH 07/23] Upserting target instead of inserting on Shift up --- .../text-annotator/src/SelectionHandler.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index ee67406d..22b7bda6 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -230,12 +230,18 @@ export const SelectionHandler = ( // Proper lifecycle management: clear selection first... selection.clear(); - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); + const exists = store.getAnnotation(currentTarget.annotation); + if (exists) { + // ...then add annotation to store... + store.updateTarget(currentTarget); + } else { + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + } // ...then make the new annotation the current selection selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); From 08c8b71381a894c0140a8bf54000c63036432692 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 14:21:23 +0200 Subject: [PATCH 08/23] CTRL/CMD+A support --- .../text-annotator/src/SelectionHandler.ts | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 22b7bda6..b69f4591 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -19,10 +19,11 @@ const CLICK_TIMEOUT = 300; const ARROW_KEYS = ['up', 'down', 'left', 'right']; +const SELECT_ALL = ['ctrl+a', '⌘+a']; + const SELECTION_KEYS = [ ...ARROW_KEYS.map(key => `shift+${key}`), - 'ctrl+a', - '⌘+a' + ...SELECT_ALL ]; export const SelectionHandler = ( @@ -59,7 +60,6 @@ export const SelectionHandler = ( * Note that Chrome/iOS will sometimes return the root doc as target! */ const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); - currentTarget = annotatable ? { annotation: uuidv4(), selector: [], @@ -184,7 +184,7 @@ export const SelectionHandler = ( selection.userSelect(hovered.id, evt); } else if (!selection.isEmpty()) { selection.clear(); - } + } }; const timeDifference = evt.timeStamp - lastDownEvent.timeStamp; @@ -249,11 +249,30 @@ export const SelectionHandler = ( } } - hotkeys(SELECTION_KEYS.join(','), { element: container, keydown: true, keyup: false }, (evt) => { + const onSelectAll = (evt: KeyboardEvent) => { + onSelectStart(evt); + + // Proper lifecycle management: clear selection first... + selection.clear(); + + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + } + + hotkeys(SELECTION_KEYS.join(','), { element: container, keydown: true, keyup: false }, evt => { if (!evt.repeat) lastDownEvent = cloneKeyboardEvent(evt); }); + hotkeys(SELECT_ALL.join(','), { keydown: true, keyup: false}, evt => { + lastDownEvent = cloneKeyboardEvent(evt); + onSelectAll(evt); + }); + /** * Free caret movement through the text resets the annotation selection. * From 56c6670fa010aa10b358e9b56db95b4a438e88dc Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 14:27:03 +0200 Subject: [PATCH 09/23] Listening to event for mobile support --- packages/text-annotator/src/SelectionHandler.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index b69f4591..7781cefc 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -297,6 +297,7 @@ export const SelectionHandler = ( container.addEventListener('pointerdown', onPointerDown); document.addEventListener('pointerup', onPointerUp); + document.addEventListener('contextmenu', onPointerUp); if (annotatingEnabled) { container.addEventListener('keyup', onKeyup); @@ -307,6 +308,7 @@ export const SelectionHandler = ( const destroy = () => { container.removeEventListener('pointerdown', onPointerDown); document.removeEventListener('pointerup', onPointerUp); + document.removeEventListener('contextmenu', onPointerUp); container.removeEventListener('keyup', onKeyup); container.removeEventListener('selectstart', onSelectStart); From 233254f1c5010252f66e25b9096d94665c527995 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Mon, 30 Sep 2024 15:38:45 +0200 Subject: [PATCH 10/23] Work in progress --- .../TextAnnotatorPopup/TextAnnotatorPopup.tsx | 126 ++++++------------ packages/text-annotator-react/test/App.tsx | 4 +- .../text-annotator/src/SelectionHandler.ts | 46 +++++-- 3 files changed, 81 insertions(+), 95 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index b87443f1..2ca3dcb4 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -1,13 +1,11 @@ -import { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; +import { ReactNode, useCallback, useEffect, useState, PointerEvent } from 'react'; import { useAnnotator, useSelection } from '@annotorious/react'; -import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; +import { type TextAnnotation, type TextAnnotator } from '@recogito/text-annotator'; import { autoUpdate, - flip, - FloatingFocusManager, - FloatingPortal, inline, offset, + flip, shift, useDismiss, useFloating, @@ -15,18 +13,16 @@ import { useRole } from '@floating-ui/react'; -import './TextAnnotatorPopup.css'; - interface TextAnnotationPopupProps { popup(props: TextAnnotationPopupContentProps): ReactNode; } -export interface TextAnnotationPopupContentProps { +interface TextAnnotationPopupContentProps { annotation: TextAnnotation; - + editable?: boolean; event?: PointerEvent; @@ -37,27 +33,21 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const r = useAnnotator(); - const { selected, event } = useSelection(); + const { selected } = useSelection(); + const annotation = selected[0]?.annotation; const [isOpen, setOpen] = useState(selected?.length > 0); - const handleClose = () => { - r?.cancelSelected(); - }; - const { refs, floatingStyles, update, context } = useFloating({ placement: 'top', open: isOpen, - onOpenChange: (open, _event, reason) => { + /* onOpenChange: (open, _event, reason) => { setOpen(open); - - if (!open) { - if (reason === 'escape-key' || reason === 'focus-out') { - r?.cancelSelected(); - } + if (!open && reason === 'escape-key') { + r?.cancelSelected(); } - }, + }, */ middleware: [ offset(10), inline(), @@ -68,41 +58,33 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { }); const dismiss = useDismiss(context); - const role = useRole(context, { role: 'dialog' }); + + const role = useRole(context, { role: 'tooltip' }); + const { getFloatingProps } = useInteractions([dismiss, role]); const selectedKey = selected.map(a => a.annotation.id).join('-'); useEffect(() => { - // Ignore all selection changes except those accompanied by a user event. - if (selected.length > 0) { // && event) { - setOpen(true); // event.type === 'pointerup' || event.type === 'keydown'); - } - }, [selectedKey /*, event */]); + // Ignore all selection changes except those accompanied by a pointer event. + setOpen(selected.length > 0); + }, [selectedKey]); useEffect(() => { - // Close the popup if the selection is cleared - if (selected.length === 0 && isOpen) { - setOpen(false); - } - }, [isOpen, selectedKey]); + if (!isOpen || !annotation) return; - useEffect(() => { - if (isOpen && annotation) { - const { - target: { - selector: [{ range }] - } - } = annotation; - - refs.setPositionReference({ - getBoundingClientRect: range.getBoundingClientRect.bind(range), - getClientRects: range.getClientRects.bind(range) - }); - } else { - // Don't leave the reference depending on the previously selected annotation - refs.setPositionReference(null); - } + if (!annotation.target.selector || annotation.target.selector.length === 0) return; + + const { + target: { + selector: [{ range }] + } + } = annotation; + + refs.setPositionReference({ + getBoundingClientRect: () => range.getBoundingClientRect(), + getClientRects:() => range.getClientRects() + }); }, [isOpen, annotation, refs]); // Prevent text-annotator from handling the irrelevant events triggered from the popup @@ -122,43 +104,21 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { return () => { mutationObserver.disconnect(); window.document.removeEventListener('scroll', update, true); - }; + } }, [update]); return isOpen && selected.length > 0 ? ( - - -
- {props.popup({ - annotation: selected[0].annotation, - editable: selected[0].editable, - event - })} - - {/* It lets keyboard/sr users to know that the dialog closes when they focus out of it */} - -
-
-
+
+ {props.popup({ + annotation: selected[0].annotation, + editable: selected[0].editable + })} +
) : null; -} +} \ No newline at end of file diff --git a/packages/text-annotator-react/test/App.tsx b/packages/text-annotator-react/test/App.tsx index d638e588..a9064d57 100644 --- a/packages/text-annotator-react/test/App.tsx +++ b/packages/text-annotator-react/test/App.tsx @@ -1,9 +1,9 @@ import React, { FC, useCallback, useEffect } from 'react'; import { AnnotationBody, Annotorious, useAnnotationStore, useAnnotator, useSelection } from '@annotorious/react'; -import { TextAnnotator, TextAnnotatorPopup, type TextAnnotationPopupContentProps } from '../src'; +import { TextAnnotator, TextAnnotatorPopup } from '../src'; import { W3CTextFormat, type TextAnnotation, type TextAnnotator as RecogitoTextAnnotator } from '@recogito/text-annotator'; -const TestPopup: FC = (props) => { +const TestPopup = (props) => { const { annotation } = props; diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 7781cefc..d701fc4e 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -92,13 +92,11 @@ export const SelectionHandler = ( // Chrome/iOS does not reliably fire the 'selectstart' event! onSelectStart(lastDownEvent || evt); - } else if (sel.isCollapsed && timeDifference < CLICK_TIMEOUT) { // Firefox doesn't fire the 'selectstart' when user clicks // over the text, which collapses the selection onSelectStart(lastDownEvent || evt); - } } @@ -209,12 +207,18 @@ export const SelectionHandler = ( // Proper lifecycle management: clear selection first... selection.clear(); - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); + const exists = store.getAnnotation(currentTarget.annotation); + if (exists) { + // ...then add annotation to store... + store.updateTarget(currentTarget); + } else { + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + } // ...then make the new annotation the current selection selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); @@ -222,6 +226,28 @@ export const SelectionHandler = ( }); } + const onContextMenu = (evt: PointerEvent) => { + const sel = document.getSelection(); + if (sel?.isCollapsed) return; + + // selection.clear(); + + const exists = store.getAnnotation(currentTarget.annotation); + if (exists) { + // ...then add annotation to store... + store.updateTarget(currentTarget); + } else { + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + } + + selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); + } + const onKeyup = (evt: KeyboardEvent) => { if (evt.key === 'Shift' && currentTarget) { const sel = document.getSelection(); @@ -297,7 +323,7 @@ export const SelectionHandler = ( container.addEventListener('pointerdown', onPointerDown); document.addEventListener('pointerup', onPointerUp); - document.addEventListener('contextmenu', onPointerUp); + document.addEventListener('contextmenu', onContextMenu); if (annotatingEnabled) { container.addEventListener('keyup', onKeyup); @@ -308,7 +334,7 @@ export const SelectionHandler = ( const destroy = () => { container.removeEventListener('pointerdown', onPointerDown); document.removeEventListener('pointerup', onPointerUp); - document.removeEventListener('contextmenu', onPointerUp); + document.removeEventListener('contextmenu', onContextMenu); container.removeEventListener('keyup', onKeyup); container.removeEventListener('selectstart', onSelectStart); From 4c5b09deaec28c4f917d19ea70942cce89852fd9 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Wed, 2 Oct 2024 14:19:37 +0200 Subject: [PATCH 11/23] Minor changes --- .../TextAnnotatorPopup/TextAnnotatorPopup.css | 6 +- .../TextAnnotatorPopup/TextAnnotatorPopup.tsx | 106 +++++++++++------- .../text-annotator/src/SelectionHandler.ts | 5 +- 3 files changed, 70 insertions(+), 47 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.css b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.css index 21ad1759..eec17ded 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.css +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.css @@ -3,7 +3,7 @@ * or the screen reader users as the popup behavior hint * Inspired by https://gist.github.com/ffoodd/000b59f431e3e64e4ce1a24d5bb36034 */ -.popup-close-message { +.r6o-popup-sr-only { border: 0 !important; clip: rect(1px, 1px, 1px, 1px); -webkit-clip-path: inset(50%); @@ -17,8 +17,8 @@ white-space: nowrap; } -.popup-close-message:focus, -.popup-close-message:active { +.r6o-popup-sr-only:focus, +.r6o-popup-sr-only:active { clip: auto; -webkit-clip-path: none; clip-path: none; diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index 2ca3dcb4..a824c9df 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -1,11 +1,13 @@ -import { ReactNode, useCallback, useEffect, useState, PointerEvent } from 'react'; +import { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; import { useAnnotator, useSelection } from '@annotorious/react'; -import { type TextAnnotation, type TextAnnotator } from '@recogito/text-annotator'; +import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; import { autoUpdate, + flip, + FloatingFocusManager, + FloatingPortal, inline, offset, - flip, shift, useDismiss, useFloating, @@ -13,16 +15,20 @@ import { useRole } from '@floating-ui/react'; +import './TextAnnotatorPopup.css'; + interface TextAnnotationPopupProps { + ariaCloseWarning?: string; + popup(props: TextAnnotationPopupContentProps): ReactNode; } -interface TextAnnotationPopupContentProps { +export interface TextAnnotationPopupContentProps { annotation: TextAnnotation; - + editable?: boolean; event?: PointerEvent; @@ -33,7 +39,7 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const r = useAnnotator(); - const { selected } = useSelection(); + const { selected, event } = useSelection(); const annotation = selected[0]?.annotation; @@ -42,12 +48,14 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const { refs, floatingStyles, update, context } = useFloating({ placement: 'top', open: isOpen, - /* onOpenChange: (open, _event, reason) => { + onOpenChange: (open, _event, reason) => { setOpen(open); - if (!open && reason === 'escape-key') { - r?.cancelSelected(); + + if (!open) { + if (reason === 'escape-key' || reason === 'focus-out') + r?.cancelSelected(); } - }, */ + }, middleware: [ offset(10), inline(), @@ -59,32 +67,29 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const dismiss = useDismiss(context); - const role = useRole(context, { role: 'tooltip' }); - - const { getFloatingProps } = useInteractions([dismiss, role]); + const role = useRole(context, { role: 'dialog' }); - const selectedKey = selected.map(a => a.annotation.id).join('-'); + const { getFloatingProps } = useInteractions([dismiss, role]); useEffect(() => { - // Ignore all selection changes except those accompanied by a pointer event. setOpen(selected.length > 0); - }, [selectedKey]); + }, [selected.map(a => a.annotation.id).join('-')]); useEffect(() => { - if (!isOpen || !annotation) return; - - if (!annotation.target.selector || annotation.target.selector.length === 0) return; - - const { - target: { - selector: [{ range }] - } - } = annotation; - - refs.setPositionReference({ - getBoundingClientRect: () => range.getBoundingClientRect(), - getClientRects:() => range.getClientRects() - }); + if (isOpen && annotation) { + const { + target: { + selector: [{ range }] + } + } = annotation; + + refs.setPositionReference({ + getBoundingClientRect: () => range.getBoundingClientRect(), + getClientRects: () => range.getClientRects() + }); + } else { + refs.setPositionReference(null); + } }, [isOpen, annotation, refs]); // Prevent text-annotator from handling the irrelevant events triggered from the popup @@ -104,21 +109,38 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { return () => { mutationObserver.disconnect(); window.document.removeEventListener('scroll', update, true); - } + }; }, [update]); return isOpen && selected.length > 0 ? ( -
- {props.popup({ - annotation: selected[0].annotation, - editable: selected[0].editable - })} -
+ + +
+ {props.popup({ + annotation: selected[0].annotation, + editable: selected[0].editable, + event + })} + + + {props.ariaCloseWarning || 'This dialog will close when you leave it.'} + +
+
+
) : null; } \ No newline at end of file diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index d701fc4e..8a89629b 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -228,15 +228,16 @@ export const SelectionHandler = ( const onContextMenu = (evt: PointerEvent) => { const sel = document.getSelection(); - if (sel?.isCollapsed) return; - // selection.clear(); + if (sel?.isCollapsed) return; const exists = store.getAnnotation(currentTarget.annotation); if (exists) { // ...then add annotation to store... store.updateTarget(currentTarget); } else { + selection.clear(); + // ...then add annotation to store... store.addAnnotation({ id: currentTarget.annotation, From 85e351a288417d523f60fc6d83f1f9d55bf63eae Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Wed, 2 Oct 2024 16:14:41 +0200 Subject: [PATCH 12/23] Mobile fixes (Android in particular) --- .../TextAnnotatorPopup/TextAnnotatorPopup.tsx | 19 +++--- .../src/TextAnnotatorPopup/isMobile.ts | 17 +++++ .../text-annotator/src/SelectionHandler.ts | 68 ++++++------------- 3 files changed, 50 insertions(+), 54 deletions(-) create mode 100644 packages/text-annotator-react/src/TextAnnotatorPopup/isMobile.ts diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index a824c9df..2c42fd65 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -1,6 +1,7 @@ import { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; import { useAnnotator, useSelection } from '@annotorious/react'; import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; +import { isMobile } from './isMobile'; import { autoUpdate, flip, @@ -46,14 +47,12 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { const [isOpen, setOpen] = useState(selected?.length > 0); const { refs, floatingStyles, update, context } = useFloating({ - placement: 'top', + placement: isMobile() ? 'bottom' : 'top', open: isOpen, onOpenChange: (open, _event, reason) => { - setOpen(open); - - if (!open) { - if (reason === 'escape-key' || reason === 'focus-out') - r?.cancelSelected(); + if (!open && (reason === 'escape-key' || reason === 'focus-out')) { + setOpen(open); + r?.cancelSelected(); } }, middleware: [ @@ -77,6 +76,9 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { useEffect(() => { if (isOpen && annotation) { + // Extra precaution - shouldn't normally happen + if (!annotation.target.selector || annotation.target.selector.length < 1) return; + const { target: { selector: [{ range }] @@ -120,8 +122,9 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { closeOnFocusOut={true} returnFocus={false} initialFocus={ - // Don't shift focus to the floating element when selected via keyboard - event?.type === 'keydown' ? -1 : 0 + // Don't shift focus to the floating element if selected via keyboard + // or on iPad/Android. + (event?.type === 'keydown' || isMobile()) ? -1 : 0 }>
{ + // @ts-ignore + var userAgent: string = navigator.userAgent || navigator.vendor || window.opera; + + if (/android/i.test(userAgent)) + return true; + + // @ts-ignore + // Note: as of recently, this NO LONGER DETECTS FIREFOX ON iOS! + // This means FF/iOS will behave like on the desktop, and loose + // selection handlebars after the popup opens. + if (/iPad|iPhone/.test(userAgent) && !window.MSStream) + return true; + + return false; +} \ No newline at end of file diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 8a89629b..76ac8270 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -162,6 +162,20 @@ 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) => { const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable || !isLeftClick) return; @@ -204,23 +218,8 @@ export const SelectionHandler = ( currentTarget = undefined; clickSelect(); } else if (currentTarget) { - // Proper lifecycle management: clear selection first... selection.clear(); - - const exists = store.getAnnotation(currentTarget.annotation); - if (exists) { - // ...then add annotation to store... - store.updateTarget(currentTarget); - } else { - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - } - - // ...then make the new annotation the current selection + upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); } }); @@ -231,20 +230,12 @@ export const SelectionHandler = ( if (sel?.isCollapsed) return; - const exists = store.getAnnotation(currentTarget.annotation); - if (exists) { - // ...then add annotation to store... - store.updateTarget(currentTarget); - } else { - selection.clear(); - - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - } + // When selecting the initial word, Chrome Android fires `contextmenu` + // before selectionChanged. + if (!currentTarget || currentTarget.selector.length === 0) + onSelectionChange(evt); + + upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); } @@ -254,23 +245,8 @@ export const SelectionHandler = ( const sel = document.getSelection(); if (!sel.isCollapsed) { - // Proper lifecycle management: clear selection first... selection.clear(); - - const exists = store.getAnnotation(currentTarget.annotation); - if (exists) { - // ...then add annotation to store... - store.updateTarget(currentTarget); - } else { - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); - } - - // ...then make the new annotation the current selection + upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); } } From c8000a115cded0393946b47fd87ca75d6860cd5d Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Wed, 2 Oct 2024 16:35:41 +0200 Subject: [PATCH 13/23] Device-specific 'select all' key --- packages/text-annotator/src/SelectionHandler.ts | 7 ++++--- packages/text-annotator/src/utils/device.ts | 2 ++ packages/text-annotator/src/utils/index.ts | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 packages/text-annotator/src/utils/device.ts diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 76ac8270..8c419c7a 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -10,6 +10,7 @@ import { debounce, splitAnnotatableRanges, rangeToSelector, + isMac, isWhitespaceOrEmpty, trimRangeToContainer, NOT_ANNOTATABLE_SELECTOR @@ -19,11 +20,11 @@ const CLICK_TIMEOUT = 300; const ARROW_KEYS = ['up', 'down', 'left', 'right']; -const SELECT_ALL = ['ctrl+a', '⌘+a']; +const SELECT_ALL = isMac ? '⌘+a' : 'ctrl+a'; const SELECTION_KEYS = [ ...ARROW_KEYS.map(key => `shift+${key}`), - ...SELECT_ALL + SELECT_ALL ]; export const SelectionHandler = ( @@ -271,7 +272,7 @@ export const SelectionHandler = ( lastDownEvent = cloneKeyboardEvent(evt); }); - hotkeys(SELECT_ALL.join(','), { keydown: true, keyup: false}, evt => { + hotkeys(SELECT_ALL, { keydown: true, keyup: false}, evt => { lastDownEvent = cloneKeyboardEvent(evt); onSelectAll(evt); }); diff --git a/packages/text-annotator/src/utils/device.ts b/packages/text-annotator/src/utils/device.ts new file mode 100644 index 00000000..a935d82a --- /dev/null +++ b/packages/text-annotator/src/utils/device.ts @@ -0,0 +1,2 @@ +// @ts-ignore +export const isMac = /mac/i.test(navigator.userAgentData ? navigator.userAgentData.platform : navigator.platform); diff --git a/packages/text-annotator/src/utils/index.ts b/packages/text-annotator/src/utils/index.ts index f1284390..fe84a63b 100644 --- a/packages/text-annotator/src/utils/index.ts +++ b/packages/text-annotator/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './cancelSingleClickEvents'; +export * from './device'; export * from './programmaticallyFocusable'; export * from './debounce'; export * from './getAnnotatableFragment'; From 4c7c67ef46f0e5141f98fef483cf49faa3aee683 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 07:43:23 +0200 Subject: [PATCH 14/23] Minor touch tweak --- packages/text-annotator/src/SelectionHandler.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 8c419c7a..579b6ac5 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -50,6 +50,8 @@ export const SelectionHandler = ( let lastDownEvent: Selection['event'] | undefined; + let isContextMenuOpen = false; + const onSelectStart = (evt: Event) => { if (isLeftClick === false) return; @@ -152,6 +154,8 @@ export const SelectionHandler = ( * to the initial pointerdown event and remember the button */ const onPointerDown = (evt: PointerEvent) => { + if (isContextMenuOpen) return; + const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable) return; @@ -178,6 +182,8 @@ export const SelectionHandler = ( } const onPointerUp = (evt: PointerEvent) => { + if (isContextMenuOpen) return; + const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable || !isLeftClick) return; @@ -227,6 +233,8 @@ export const SelectionHandler = ( } const onContextMenu = (evt: PointerEvent) => { + isContextMenuOpen = true; + const sel = document.getSelection(); if (sel?.isCollapsed) return; From 5d0af354faec05844ccb3729aed7340c4cbc9821 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 08:44:21 +0200 Subject: [PATCH 15/23] Bugfix/reverted broken change --- .../src/TextAnnotatorPopup/TextAnnotatorPopup.tsx | 5 ++--- packages/text-annotator/src/SelectionHandler.ts | 11 ++--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index 2c42fd65..1e941080 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -122,9 +122,8 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { closeOnFocusOut={true} returnFocus={false} initialFocus={ - // Don't shift focus to the floating element if selected via keyboard - // or on iPad/Android. - (event?.type === 'keydown' || isMobile()) ? -1 : 0 + // Don't shift focus to the floating element if selected via keyboard or on mobile. + (event?.type === 'keydown' || event?.type === 'contextmenu' || isMobile()) ? -1 : 0 }>
{ if (isLeftClick === false) return; @@ -154,8 +152,6 @@ export const SelectionHandler = ( * to the initial pointerdown event and remember the button */ const onPointerDown = (evt: PointerEvent) => { - if (isContextMenuOpen) return; - const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable) return; @@ -182,8 +178,6 @@ export const SelectionHandler = ( } const onPointerUp = (evt: PointerEvent) => { - if (isContextMenuOpen) return; - const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable || !isLeftClick) return; @@ -233,16 +227,15 @@ export const SelectionHandler = ( } const onContextMenu = (evt: PointerEvent) => { - isContextMenuOpen = true; - const sel = document.getSelection(); if (sel?.isCollapsed) return; // When selecting the initial word, Chrome Android fires `contextmenu` // before selectionChanged. - if (!currentTarget || currentTarget.selector.length === 0) + if (!currentTarget || currentTarget.selector.length === 0) { onSelectionChange(evt); + } upsertCurrentTarget(); From fb3d70bedba11660d64b479460094e951e530d38 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 09:17:09 +0200 Subject: [PATCH 16/23] Minor fix --- packages/text-annotator/src/SelectionHandler.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 948f9c61..440f2e34 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -50,7 +50,11 @@ export const SelectionHandler = ( let lastDownEvent: Selection['event'] | undefined; + let isContextMenuOpen = false; + const onSelectStart = (evt: Event) => { + isContextMenuOpen = false; + if (isLeftClick === false) return; @@ -152,6 +156,8 @@ export const SelectionHandler = ( * to the initial pointerdown event and remember the button */ const onPointerDown = (evt: PointerEvent) => { + if (isContextMenuOpen) return; + const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable) return; @@ -178,6 +184,8 @@ export const SelectionHandler = ( } const onPointerUp = (evt: PointerEvent) => { + if (isContextMenuOpen) return; + const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); if (!annotatable || !isLeftClick) return; @@ -227,6 +235,8 @@ export const SelectionHandler = ( } const onContextMenu = (evt: PointerEvent) => { + isContextMenuOpen = true; + const sel = document.getSelection(); if (sel?.isCollapsed) return; From b2e62330d01cc0ccd4c91f905990d0579c33159c Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 09:39:34 +0200 Subject: [PATCH 17/23] Minor bugfix --- .../src/TextAnnotatorPopup/TextAnnotatorPopup.tsx | 12 +++++++----- packages/text-annotator/src/SelectionHandler.ts | 5 +++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index 1e941080..c5a476b1 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -1,4 +1,4 @@ -import { PointerEvent, ReactNode, useCallback, useEffect, useState } from 'react'; +import { PointerEvent, ReactNode, useCallback, useEffect, useMemo, useState } from 'react'; import { useAnnotator, useSelection } from '@annotorious/react'; import type { TextAnnotation, TextAnnotator } from '@recogito/text-annotator'; import { isMobile } from './isMobile'; @@ -114,6 +114,11 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { }; }, [update]); + // Don't shift focus to the floating element if selected via keyboard or on mobile. + const initialFocus = useMemo(() => { + return (event?.type === 'keyup' || event?.type === 'contextmenu' || isMobile()) ? -1 : 0; + }, [event]); + return isOpen && selected.length > 0 ? ( { modal={false} closeOnFocusOut={true} returnFocus={false} - initialFocus={ - // Don't shift focus to the floating element if selected via keyboard or on mobile. - (event?.type === 'keydown' || event?.type === 'contextmenu' || isMobile()) ? -1 : 0 - }> + initialFocus={initialFocus}>
0) { selection.clear(); upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); From eb942a3de943da501cd89c59bc1fe5d355a7f286 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 10:00:11 +0200 Subject: [PATCH 18/23] 'Select All' workaround --- .../text-annotator/src/SelectionHandler.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 0a5b15d6..da663440 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -271,12 +271,19 @@ export const SelectionHandler = ( // Proper lifecycle management: clear selection first... selection.clear(); - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); + setTimeout(() => { + // ...then add annotation to store... + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); + + selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + + // Sigh.. not sure there's a reliable timeout. Alternative would be + // to listen to the selectionchange event once? + }, 250); } hotkeys(SELECTION_KEYS.join(','), { element: container, keydown: true, keyup: false }, evt => { From 9cf64f85160f13634a448273d68480b3d79d4ae7 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 10:13:42 +0200 Subject: [PATCH 19/23] Select-all fix --- .../text-annotator/src/SelectionHandler.ts | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index da663440..b2a98309 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -266,24 +266,30 @@ export const SelectionHandler = ( } const onSelectAll = (evt: KeyboardEvent) => { - onSelectStart(evt); - // Proper lifecycle management: clear selection first... - selection.clear(); + const onSelected = () => setTimeout(() => { + if (currentTarget?.selector.length > 0) { + selection.clear(); - setTimeout(() => { - // ...then add annotation to store... - store.addAnnotation({ - id: currentTarget.annotation, - bodies: [], - target: currentTarget - }); + store.addAnnotation({ + id: currentTarget.annotation, + bodies: [], + target: currentTarget + }); - selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); + } + + document.removeEventListener('selectionchange', onSelected); - // Sigh.. not sure there's a reliable timeout. Alternative would be - // to listen to the selectionchange event once? - }, 250); + // Sigh... this needs a delay to work. But doesn't seem reliable. + }, 100); + + // Listen to the change event that follows + document.addEventListener('selectionchange', onSelected); + + // Start selection! + onSelectStart(evt); } hotkeys(SELECTION_KEYS.join(','), { element: container, keydown: true, keyup: false }, evt => { From 5adbeebbbd497f617b1355e9b76cc1d025bbcab7 Mon Sep 17 00:00:00 2001 From: Rainer Simon <470971+rsimon@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:14:14 +0200 Subject: [PATCH 20/23] Update packages/text-annotator/src/SelectionHandler.ts Co-authored-by: Oleksandr Danylchenko <68850090+oleksandr-danylchenko@users.noreply.github.com> --- 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 b2a98309..fd585270 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -206,7 +206,7 @@ export const SelectionHandler = ( } } else if (!selection.isEmpty()) { selection.clear(); - } + } }; const timeDifference = evt.timeStamp - lastDownEvent.timeStamp; From 28908cf0bca3f354ee8f344a918bf975b2a2e7ea Mon Sep 17 00:00:00 2001 From: Rainer Simon <470971+rsimon@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:14:41 +0200 Subject: [PATCH 21/23] Update packages/text-annotator/src/SelectionHandler.ts Co-authored-by: Oleksandr Danylchenko <68850090+oleksandr-danylchenko@users.noreply.github.com> --- packages/text-annotator/src/SelectionHandler.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index fd585270..801e0f1c 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -143,8 +143,10 @@ export const SelectionHandler = ( updated: new Date() }; - // On destkop, the annotation won't usually exist while the selection is - // being edited. But it will typcially be the case on mobile! + /** + * 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! + */ if (store.getAnnotation(currentTarget.annotation)) { store.updateTarget(currentTarget, Origin.LOCAL); } From 8d113dc93167df8d48521f331649202bf6cf32a1 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 13:20:38 +0200 Subject: [PATCH 22/23] Minor fix --- packages/text-annotator-react/test/App.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/text-annotator-react/test/App.tsx b/packages/text-annotator-react/test/App.tsx index a9064d57..74849e2c 100644 --- a/packages/text-annotator-react/test/App.tsx +++ b/packages/text-annotator-react/test/App.tsx @@ -1,9 +1,9 @@ import React, { FC, useCallback, useEffect } from 'react'; -import { AnnotationBody, Annotorious, useAnnotationStore, useAnnotator, useSelection } from '@annotorious/react'; -import { TextAnnotator, TextAnnotatorPopup } from '../src'; +import { AnnotationBody, Annotorious, useAnnotationStore, useAnnotator } from '@annotorious/react'; +import { TextAnnotationPopupContentProps, TextAnnotator, TextAnnotatorPopup } from '../src'; import { W3CTextFormat, type TextAnnotation, type TextAnnotator as RecogitoTextAnnotator } from '@recogito/text-annotator'; -const TestPopup = (props) => { +const TestPopup: FC = (props) => { const { annotation } = props; From 6758d7d3187edc374e37a8639a7ff20df1c9f421 Mon Sep 17 00:00:00 2001 From: Rainer Simon Date: Thu, 3 Oct 2024 14:54:24 +0200 Subject: [PATCH 23/23] Reverted to button for screenreader close hint --- .../TextAnnotatorPopup/TextAnnotatorPopup.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index c5a476b1..59170bc4 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -94,12 +94,6 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { } }, [isOpen, annotation, refs]); - // Prevent text-annotator from handling the irrelevant events triggered from the popup - const getStopEventsPropagationProps = useCallback( - () => ({ onPointerUp: (event: PointerEvent) => event.stopPropagation() }), - [] - ); - useEffect(() => { const config: MutationObserverInit = { attributes: true, childList: true, subtree: true }; @@ -114,11 +108,19 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { }; }, [update]); + // Prevent text-annotator from handling the irrelevant events triggered from the popup + const getStopEventsPropagationProps = useCallback( + () => ({ onPointerUp: (event: PointerEvent) => event.stopPropagation() }), + [] + ); + // Don't shift focus to the floating element if selected via keyboard or on mobile. const initialFocus = useMemo(() => { return (event?.type === 'keyup' || event?.type === 'contextmenu' || isMobile()) ? -1 : 0; }, [event]); + const onClose = () => r?.cancelSelected(); + return isOpen && selected.length > 0 ? ( { event })} - - {props.ariaCloseWarning || 'This dialog will close when you leave it.'} - +