recogito / text-annotator-js

A JavaScript library for text annotation.
BSD 3-Clause "New" or "Revised" License
22 stars 7 forks source link

Annotation isn't dismissed clicking an empty space, covered by the native selection range ⚠️ #136

Open oleksandr-danylchenko opened 3 months ago

oleksandr-danylchenko commented 3 months ago

Background

While handling the pointerup event, checking the document.getSelection().isCollapsed property can behave in 2 ways:

  1. You made a selection and clicked on it. Then, the isCollapsed is false even though the native highlight disappears! ⚠️
  2. You made a selection and clicked somewhere else. Then, the isCollapsed is expectedly true, matching the native highlight disappearance.

https://github.com/user-attachments/assets/919f9e25-0852-4aec-85fa-983532243060

Issue

In the SelectionHander the annotation selection gets dismissed only when the pointerdown + pointerup combination is recognized as the "click" and there's nothing "hovered" (lines 152-155 & 144-146): https://github.com/recogito/text-annotator-js/blob/e6d3f33339ab1f4cd2ccb6b77624c20f12841ee4/packages/text-annotator/src/SelectionHandler.ts#L149-L157 https://github.com/recogito/text-annotator-js/blob/e6d3f33339ab1f4cd2ccb6b77624c20f12841ee4/packages/text-annotator/src/SelectionHandler.ts#L134-L147

However, as the document.getSelection().isCollapsed is false when you click over the native selection range, the annotation selection sometimes doesn't get dismissed! Instead, the annotation gets selected another time, because the currentTarget is still present!

Alternatively, we expect that the annotation selection will behave like the native selection highlight in all cases. When you click on an empty space - it should be dismissed.

Issue Examples

https://github.com/user-attachments/assets/aee59f76-890b-4571-9614-03ca3bb051e2

https://github.com/user-attachments/assets/2997c3f1-5da3-4b15-935c-b9dad0cfd587

Suggested Changes

The root of the issue is the document.getSelection().isCollapsed that is "lagging behind" on the pointerup event handling.

After I added a 0ms. timeout for the reading, I started getting more adequate value consistently. It's always true either when you click on the selected range or some other content.

https://github.com/user-attachments/assets/35ef211b-06cb-4669-a50f-d6e0764aec68

In that way, the "click" will always get processed consistently and the annotation selection will get dismissed when you didn't click on the highlight explicitly.

rsimon commented 1 month ago

Just adding that this should now also work in the revised-selection-behavior branch.

oleksandr-danylchenko commented 1 month ago

Hello @rsimon 👋🏻 Unfortunately, I noticed that the "lagging behind isCollapsed property" is still a thing for touch devices! When I tap somewhere around the selection - the isCollapsed is still true when the onPointerUp gets handled, even within the setTimeout. image

That creates a weird effect that makes a user tap twice to dismiss the selection. Similar to the one we had in the demos above ;(

oleksandr-danylchenko commented 1 month ago

Incredibly... increasing the setTimeout's timeout from the default 0 to 1 helps! It starts reporting the isCollapsed properly again!