recogito / text-annotator-js

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

Selecting on touch devices causes highlight flickering when move down rapidly #172

Open oleksandr-danylchenko opened 2 weeks ago

oleksandr-danylchenko commented 2 weeks ago

Issue

I noticed that when the TextAnnotatorPopup is rendered and I move select text down rapidly - the finger can go over the floating element and cause the highlight flickering:

https://github.com/user-attachments/assets/ebdb8383-c4f1-4f49-859e-1ffd7d1818b7

However, it doesn't happen when I remove the popup rendering:

https://github.com/user-attachments/assets/9b208e65-39a2-49e6-a544-23018b4c66ed


In the real app on iOS it looks even more flickery when the selection goes over the headings:

https://github.com/user-attachments/assets/3f60494a-1b35-4467-9339-5a73dd2757f3

rsimon commented 2 weeks ago

Yes, I saw this, too. But haven't investigated further yet.

The selection clearly breaks as it goes from the selection start (on the text) to somewhere on the portal popup. I believe the popup is already marked as not-annotatable? Originally, I had the popup oriented into the upwards direction. In practice, that helps a bit since people will usually want to select downward. (Although there's no guarantee, of course.) But because of the native context menu popup, downwards orientation really is the only option. Not sure what a good way around this would be...

oleksandr-danylchenko commented 2 weeks ago

But haven't investigated further yet

No problem, I'm on it now. As we need to solve the issue for our product too.


Here's the selection change event range breakdown: image

When the jump occurs, the SelectionHandler thinks that you selected down to the end of the annotatable container. However, the range updates back to the "normal" value almost instantly.

rsimon commented 2 weeks ago

Thanks!! Hm, seems like the selection goes to the popup, which itself is not annotatable. Therefore, the clipping will select up to the popup which, because the popup is portalled, is the end of the page?

oleksandr-danylchenko commented 2 weeks ago

up to the popup which, because the popup is portalled, is the end of the page?

Right! It's exactly at the end of the page: image

Therefore, the SelectionHandler just clamps the selection to the end of the annotatable container

oleksandr-danylchenko commented 2 weeks ago

I suppose that's one of the reasons behind hiding the native content menu popup when the selection is active. It doesn't allow the case when the selection goes "over" the floating element.

oleksandr-danylchenko commented 2 weeks ago

It seems to be a common issue in the annotating solutions though 😅 In Slab, they also render the floating popup below the selection and it's mounted before the content in DOM. However, it's still possible to reach it when you select rapidly enough. That will make the selection jump up. Or when you scroll from the top - it will make the whole screen scroll up!

https://github.com/user-attachments/assets/2e732fbd-d9d1-43bb-a888-32dd9503972e

https://github.com/user-attachments/assets/97e8362f-f038-4833-94f4-b8bb40f21b65

oleksandr-danylchenko commented 2 weeks ago

I also tried putting the user-select: none across the TextAnnotatorPopup, however, it doesn't seem to be effective and the selection range can still go over it.

rsimon commented 2 weeks ago

Putting the portal at the top rather than the end sounds. But yikes for the scroll behavior... That seems actually worse UX than the jumping selection I believe.

I also tried putting the user-select: none

Ouch. The selection still happens even if the popup has user-select: none, even with the various browser prefixes? I was thinking of this as a last resort, too. Keep it unselectable until the selection is idle for a while or so...

oleksandr-danylchenko commented 2 weeks ago

The selection still happens even if the popup has user-select: none

That's correct. I tried to apply the user-select: none in various way, but none of them seem to prevent the selection over the popup ;( I started with the following CSS, but it didn't yield any results 🤷🏻‍♂️

.r6o-popup {
  ...
  user-select: none;
}
.r6o-popup * {
  user-select: none;
}

Then I tried the following, more aggressive user-select: none setting + the CSS above: image

However, I still see the flickering when I select down rapidly 🤷🏻‍♂️

oleksandr-danylchenko commented 2 weeks ago

I suppose that's one of the reasons behind hiding the native content menu popup when the selection is active. It doesn't allow the case when the selection goes "over" the floating element.

Maybe we can refer to such a strategy too? As, technically speaking, we can know when the selection is "active" and when it "pauses" on pointerup, keyup, or contextmenu. For that, we can make another flag in the SelectionHandler that will be exposed to the popup component. The latter will be rendered only on the user's idling.

The "idling" concept would also be beneficial to the useAnnouncePopupNavigation that announces where the user should navigate with the keyboard once the popup appears. Currently, it uses the timeout-based approach for listening to the store. But listening to the actual selection "pause" might be more robust.

rsimon commented 2 weeks ago

Agreed - sounds sensible.

oleksandr-danylchenko commented 2 weeks ago

I tried adding the following simple hook for tracking the idling on a single annotation:

const useAnnotationTargetIdling = (
  annotationId: string | undefined,
  options: { timeout: number } = { timeout: 500 }
) => {
  const store = useAnnotationStore();

  const [isIdling, setIdling] = useState(true);

  useEffect(() => {
    if (!annotationId) return;

    let idlingTimeout: ReturnType<typeof setTimeout>;

    const scheduleSetIdling = () => {
      setIdling(false);

      clearTimeout(idlingTimeout);
      idlingTimeout = setTimeout(() => setIdling(true), options.timeout);
    };

    store.observe(
      scheduleSetIdling,
      {
        annotations: annotationId,
        ignore: Ignore.BODY_ONLY,
        origin: Origin.LOCAL
      }
    );
    return () => {
      clearTimeout(idlingTimeout);
      store.unobserve(scheduleSetIdling);
    };
  }, [annotationId]);

  return isIdling;
};

It works pretty nice for the cases when I do a continuous selection and then release the finger. It accounts for the time needed for the selector to adjust the range to the whitespace. But... it causes flicking in a case when I halted the selection, the idling got registered and then I released the finger.

https://github.com/user-attachments/assets/dd58de09-dda4-48fd-a09b-7dbb28f10193

That's caused by another reported issue - https://github.com/recogito/text-annotator-js/issues/169. The store reports about the target update although there are no differences in the selected range!