From 2e64b8215f4c9e048774c37fd900bcc9833a42f2 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Tue, 15 Oct 2024 15:46:43 +0300 Subject: [PATCH 1/5] Increased the `collapsed` check timeout on `pointerup` --- .../src/TextAnnotatorPopup/TextAnnotatorPopup.tsx | 2 +- packages/text-annotator/src/SelectionHandler.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx index 3a3272e0..0055dd0e 100644 --- a/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx +++ b/packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx @@ -154,4 +154,4 @@ export const TextAnnotatorPopup = (props: TextAnnotationPopupProps) => { ) : null; -} \ No newline at end of file +} diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index b61d6dc4..8b182a3b 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -217,7 +217,7 @@ export const SelectionHandler = ( const timeDifference = evt.timeStamp - lastDownEvent.timeStamp; /** - * We must check the `isCollapsed` within the 0-timeout + * We must check the `isCollapsed` within the 1-timeout * to handle the annotation dismissal after a click properly. * * Otherwise, the `isCollapsed` will return an obsolete `false` value, @@ -237,7 +237,7 @@ export const SelectionHandler = ( upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); } - }); + }, 1); } const onContextMenu = (evt: PointerEvent) => { From b3ae1741abec1e9625f25524058756bca91467c3 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Tue, 15 Oct 2024 16:51:19 +0300 Subject: [PATCH 2/5] Increased timeout to 5ms --- packages/text-annotator/src/SelectionHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 8b182a3b..1743b4c5 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -217,7 +217,7 @@ export const SelectionHandler = ( const timeDifference = evt.timeStamp - lastDownEvent.timeStamp; /** - * We must check the `isCollapsed` within the 1-timeout + * We must check the `isCollapsed` within the 5ms. timeout * to handle the annotation dismissal after a click properly. * * Otherwise, the `isCollapsed` will return an obsolete `false` value, @@ -237,7 +237,7 @@ export const SelectionHandler = ( upsertCurrentTarget(); selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); } - }, 1); + }, 5); } const onContextMenu = (evt: PointerEvent) => { From 03927528a30427e33ce0834a91079ee8e4b16ef3 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Tue, 15 Oct 2024 19:05:43 +0300 Subject: [PATCH 3/5] Added polling for the collapsed selection state --- package-lock.json | 7 +++ packages/text-annotator/package.json | 3 +- .../text-annotator/src/SelectionHandler.ts | 57 ++++++++++++------- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index ae2cdf55..2a84e6a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3580,6 +3580,12 @@ "pathe": "^1.1.2" } }, + "node_modules/poll": { + "version": "3.2.2", + "resolved": "https://registry.npmjs.org/poll/-/poll-3.2.2.tgz", + "integrity": "sha512-qckJRcLqqsX72Uu/Sa/GbzWUXB/zZcyMNccwdGFQnYoewnXUdWyMWkHUmaiBAvm950ujJJ15OiFFy+gtBm1K+A==", + "license": "MIT" + }, "node_modules/postcss": { "version": "8.4.47", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.47.tgz", @@ -4577,6 +4583,7 @@ "colord": "^2.9.3", "dequal": "^2.0.3", "hotkeys-js": "^3.13.7", + "poll": "^3.2.2", "rbush": "^4.0.1", "uuid": "^10.0.0" }, diff --git a/packages/text-annotator/package.json b/packages/text-annotator/package.json index fed72f8c..013b7309 100644 --- a/packages/text-annotator/package.json +++ b/packages/text-annotator/package.json @@ -41,7 +41,8 @@ "colord": "^2.9.3", "dequal": "^2.0.3", "hotkeys-js": "^3.13.7", + "poll": "^3.2.2", "rbush": "^4.0.1", "uuid": "^10.0.0" } -} \ No newline at end of file +} diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 1743b4c5..8686f51c 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -2,6 +2,7 @@ import { Origin } from '@annotorious/core'; import type { Filter, Selection, User } from '@annotorious/core'; import { v4 as uuidv4 } from 'uuid'; import hotkeys from 'hotkeys-js'; +import { poll } from 'poll'; import type { TextAnnotatorState } from './state'; import type { TextAnnotation, TextAnnotationTarget } from './model'; import { @@ -188,7 +189,7 @@ export const SelectionHandler = ( } } - const onPointerUp = (evt: PointerEvent) => { + const onPointerUp = async (evt: PointerEvent) => { if (isContextMenuOpen) return; const annotatable = !(evt.target as Node).parentElement?.closest(NOT_ANNOTATABLE_SELECTOR); @@ -214,30 +215,48 @@ export const SelectionHandler = ( } }; + const timeDifference = evt.timeStamp - lastDownEvent.timeStamp; + if (timeDifference < CLICK_TIMEOUT) { + await pollSelectionCollapsed(); - /** - * We must check the `isCollapsed` within the 5ms. timeout - * to handle the annotation dismissal after a click properly. - * - * Otherwise, the `isCollapsed` will return an obsolete `false` value, - * click won't be processed, and the annotation will get falsely re-selected. - * - * @see https://github.com/recogito/text-annotator-js/issues/136 - */ - setTimeout(() => { const sel = document.getSelection(); - - // Just a click, not a selection - if (sel?.isCollapsed && timeDifference < CLICK_TIMEOUT) { + if (sel?.isCollapsed) { currentTarget = undefined; clickSelect(); - } else if (currentTarget && currentTarget.selector.length > 0) { - selection.clear(); - upsertCurrentTarget(); - selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); + return; } - }, 5); + } + + if (currentTarget && currentTarget.selector.length > 0) { + selection.clear(); + upsertCurrentTarget(); + selection.userSelect(currentTarget.annotation, clonePointerEvent(evt)); + } + } + + /** + * We must check the `isCollapsed` after an unspecified timeout + * to handle the annotation dismissal after a click properly. + * + * Otherwise, the `isCollapsed` will return an obsolete `false` value, + * click won't be processed, and the annotation will get falsely re-selected. + * + * @see https://github.com/recogito/text-annotator-js/issues/136#issue-2465915707 + * @see https://github.com/recogito/text-annotator-js/issues/136#issuecomment-2413773804 + */ + const pollSelectionCollapsed = async () => { + const sel = document.getSelection(); + + let stopPolling = false; + let isCollapsed = sel?.isCollapsed; + const shouldStopPolling = () => isCollapsed || stopPolling; + + const pollingDelayMs = 2; + const stopPollingInMs = 50; + setTimeout(() => stopPolling = true, stopPollingInMs); + + await poll(() => isCollapsed = sel?.isCollapsed, pollingDelayMs, shouldStopPolling); } const onContextMenu = (evt: PointerEvent) => { From 71c342330a71091db097d8db98f59fafe82ac5d6 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Tue, 15 Oct 2024 19:06:50 +0300 Subject: [PATCH 4/5] Returned poll --- 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 8686f51c..ef1f9dd1 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -256,7 +256,7 @@ export const SelectionHandler = ( const stopPollingInMs = 50; setTimeout(() => stopPolling = true, stopPollingInMs); - await poll(() => isCollapsed = sel?.isCollapsed, pollingDelayMs, shouldStopPolling); + return poll(() => isCollapsed = sel?.isCollapsed, pollingDelayMs, shouldStopPolling); } const onContextMenu = (evt: PointerEvent) => { From 5d3a8a07864cc3b1d5056c7f0a12dc5ce4cc7ad1 Mon Sep 17 00:00:00 2001 From: Oleksandr Danylchenko Date: Wed, 16 Oct 2024 23:25:51 +0300 Subject: [PATCH 5/5] Decreased polling delay to 1ms --- packages/text-annotator/src/SelectionHandler.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/text-annotator/src/SelectionHandler.ts b/packages/text-annotator/src/SelectionHandler.ts index 836c4ab0..3fe2f813 100644 --- a/packages/text-annotator/src/SelectionHandler.ts +++ b/packages/text-annotator/src/SelectionHandler.ts @@ -55,7 +55,7 @@ export const SelectionHandler = ( const onSelectStart = (evt: Event) => { isContextMenuOpen = false; - + if (isLeftClick === false) return; @@ -250,7 +250,7 @@ export const SelectionHandler = ( let isCollapsed = sel?.isCollapsed; const shouldStopPolling = () => isCollapsed || stopPolling; - const pollingDelayMs = 2; + const pollingDelayMs = 1; const stopPollingInMs = 50; setTimeout(() => stopPolling = true, stopPollingInMs); @@ -264,12 +264,12 @@ export const SelectionHandler = ( if (sel?.isCollapsed) return; - // When selecting the initial word, Chrome Android fires `contextmenu` + // 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)); @@ -301,7 +301,7 @@ export const SelectionHandler = ( selection.userSelect(currentTarget.annotation, cloneKeyboardEvent(evt)); } - + document.removeEventListener('selectionchange', onSelected); // Sigh... this needs a delay to work. But doesn't seem reliable. @@ -309,7 +309,7 @@ export const SelectionHandler = ( // Listen to the change event that follows document.addEventListener('selectionchange', onSelected); - + // Start selection! onSelectStart(evt); }