Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
oleksandr-danylchenko opened this issue Nov 4, 2024 · 12 comments

Comments

@oleksandr-danylchenko
Copy link
Contributor

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:

Record_2024-11-04-15-32-39.mp4

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

Record_2024-11-04-15-33-33.mp4

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

iOS_flickering.mp4
@rsimon
Copy link
Member

rsimon commented Nov 4, 2024

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
Copy link
Contributor Author

oleksandr-danylchenko commented Nov 4, 2024

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
Copy link
Member

rsimon commented Nov 4, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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!

slab_jump_up_little.mp4
slab_jump_up_screen.mp4

@oleksandr-danylchenko
Copy link
Contributor Author

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
Copy link
Member

rsimon commented Nov 4, 2024

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
Copy link
Contributor Author

oleksandr-danylchenko commented Nov 4, 2024

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
Copy link
Contributor Author

oleksandr-danylchenko commented Nov 4, 2024

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
Copy link
Member

rsimon commented Nov 4, 2024

Agreed - sounds sensible.

@oleksandr-danylchenko
Copy link
Contributor Author

oleksandr-danylchenko commented Nov 6, 2024

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.

Record_2024-11-06-16-47-59.mp4

That's caused by another reported issue - #169. The store reports about the target update although there are no differences in the selected range!

oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 6, 2024
…ng-testing

# Conflicts:
#	packages/text-annotator-react/src/TextAnnotatorPopup/TextAnnotatorPopup.tsx
#	packages/text-annotator-react/src/hooks/index.ts
#	packages/text-annotator/src/SelectionHandler.ts
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 11, 2024
# Conflicts:
#	packages/text-annotator-react/src/TextAnnotationPopup/TextAnnotationPopup.tsx
#	packages/text-annotator-react/test/App.tsx
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 11, 2024
# Conflicts:
#	packages/text-annotator-react/src/TextAnnotationPopup/TextAnnotationPopup.tsx
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 18, 2024
# Conflicts:
#	package-lock.json
#	packages/text-annotator-react/package.json
#	packages/text-annotator-react/src/TextAnnotationPopup/TextAnnotationPopup.tsx
#	packages/text-annotator-react/src/hooks/index.ts
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 18, 2024
# Conflicts:
#	packages/text-annotator-react/src/hooks/useAnnotationQuoteIdling.ts
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Nov 21, 2024
# Conflicts:
#	package-lock.json
#	packages/text-annotator-react/package.json
#	packages/text-annotator-react/src/TextAnnotationPopup/TextAnnotationPopup.tsx
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Dec 16, 2024
oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants