Bullet/Editable: Use fastClick for clickHandler #2752

ethan-james · 2025-01-02T22:19:02Z

The Bullet component was using onClick to dispatch its setCursor action, which has a noticeable delay on iOS. Wrapping the click handler in fastClick solves the issue.

In Editable, the onTap handler was ignoring taps that would cause the keyboard to open, and falling back to the onFocus handler after a 300ms delay. It was necessary to intercept those taps and set the cursor with a manual offset to put the caret back at the end of the thought on iOS Safari. There was also some flakiness where selection.set wasn't always opening the keyboard, so now it calls contentRef.current.focus() explicitly.

ethan-james · 2025-01-04T00:12:36Z

src/components/Editable.tsx

@@ -203,6 +203,8 @@ const Editable = ({
      // set offset to null to allow the browser to set the position of the selection
      let offset = null

+      if (isTouch && isSafari() && contentRef.current?.innerHTML.length) offset = contentRef.current.innerHTML.length


The caret was getting positioned at the end of the thought somehow onFocus, so I wanted to preserve that behavior onTap.

ethan-james · 2025-01-04T00:15:19Z

src/components/Editable/useEditMode.ts

@@ -51,6 +51,7 @@ const useEditMode = ({
          selection.clear()
        } else {
          selection.set(contentRef.current, { offset: editingCursorOffset || 0 })
+          if (isTouch && isSafari()) contentRef.current?.focus()


I'm not quite sure why this is necessary, but it was having the following problem on iOS Safari:

Tap a thought - cursor moves, keyboard stays closed

Tap it again - keyboard opens

Close keyboard

Tap it again - keyboard does not open

Thanks, good catch.

I'll create a separate issue to get to the bottom of this.

trevinhofmann

Thank you, @ethan-james!

Switching to fastClick seems to have done the trick, when I test locally:

thoughts.mp4

src/components/Bullet.tsx

Co-authored-by: Trevin Hofmann <[email protected]>

raineorshine

Thanks for the submission! I can confirm that the timing is correct now.

The only problem I'm seeing is that the selection offset is not correct when tapping in the middle of a non-cursor thought in edit mode. This behavior relies on the default browser behavior to set the selection underneath the user's finger.

Steps to Reproduce

- Hello World
- x

Set the cursor on x.
Tap x to open the keyboard if it's not open already.
Tap in between Hello and World.

Current Behavior

a81cfd8

The caret is placed at the end of the thought, i.e. Hello World|

Expected Behavior

main

The caret should be placed in between Hello and World, i.e. Hello| World.

ethan-james · 2025-01-08T18:33:02Z

When I tested it on the main branch, I saw that onFocus would place the caret at the end of the thought, so that's the behavior that I tried to emulate. I'll see if instead I can come up with a reliable way to determine the selection offset from a touchend event.

raineorshine · 2025-01-09T16:23:26Z

Do you know why the onTap handler is ignoring attempts to open the keyboard. Is one of the e.preventDefaults being called?

My sense is that it would be more reliable to fix the root cause of the keyboard not appearing rather than trying to restore the selection and offset after the fact. But let me know what you find out!

ethan-james · 2025-01-09T16:58:43Z

Do you know why the onTap handler is ignoring attempts to open the keyboard. Is one of the e.preventDefaults being called?

My sense is that it would be more reliable to fix the root cause of the keyboard not appearing rather than trying to restore the selection and offset after the fact. But let me know what you find out!

It used to call onTap which would fall back to onFocus in this circumstance (cursor already on thought, keyboard closed). onFocus had a ~300ms delay so we're not using that anymore (by including e.preventDefault and manually managing the selection).

All of the advice on Google for eliminating click/focus delays is 10 years old and doesn't work, but I did just now put together a CodePen (https://codepen.io/ethan-james-the-sasster/pen/ByBYqrL) showing that the delay doesn't occur on simple contenteditable divs. So maybe there is hope for eliminating the delay at the source and going back to using onFocus, which I agree is a better solution.

Last thing to point out, though, is that on the iPad I'm using, the native focus behavior places the caret at the end of the editable text, and there doesn't seem to be a way to tap it into a different location. I've already emulated that behavior, although I agree that letting the browser control it natively is better.

ethan-james · 2025-01-09T17:09:49Z

Also, I wasn't able to find a way to get the offset from a touchend event. If we do decide to pursue manually setting the offset, I think we have to try one of the usual suspects:

Estimating the width of each character and dividing by the touch x/y position
Inserting a hidden element and measuring its size with different combinations of characters

raineorshine · 2025-01-09T17:20:54Z

All of the advice on Google for eliminating click/focus delays is 10 years old and doesn't work, but I did just now put together a CodePen (codepen.io/ethan-james-the-sasster/pen/ByBYqrL) showing that the delay doesn't occur on simple contenteditable divs. So maybe there is hope for eliminating the delay at the source and going back to using onFocus, which I agree is a better solution.

Yes, exactly. The delay is not inherent to contenteditables, but is being created through our own event handling. There is admittedly a lot of complexity with the selection handling, including automatically setting the selection when a thought re-renders, via useEditMode. However, theoretically we shouldn't need a lot of complex functionality for setting the caret when we are already in edit mode. The default browser behavior should set the caret under the user's finger on touchend.

Last thing to point out, though, is that on the iPad I'm using, the native focus behavior places the caret at the end of the editable text, and there doesn't seem to be a way to tap it into a different location. I've already emulated that behavior, although I agree that letting the browser control it natively is better.

Interesting. My last few iPhones have correctly set the caret in the middle of the thought in these cases. I haven't tested on iPad recently, but I would assume with the same version of iOS it would behave the same. What version of iOS are you running? Do you possibly have another device or way to reproduce the default caret behavior? It might be difficult to troubleshoot this otherwise.

Also, I wasn't able to find a way to get the offset from a touchend event. If we do decide to pursue manually setting the offset, I think we have to try one of the usual suspects:

Estimating the width of each character and dividing by the touch x/y position

Inserting a hidden element and measuring its size with different combinations of characters

Yes, that's what I was worried about. It's plausible, but strikes me as high effort + high risk. Plus, inserting a DOM element and measuring it may introduce a delay itself, negating the benefit of the fastClick.

ethan-james · 2025-01-09T17:43:54Z

Here's what I see on an iPhone running 18.1.1. You can see that I can tap to the end of the editable div, or to the beginning, or select all.

ScreenRecording_01-09-2025.09-39-04_1.MP4

ethan-james · 2025-01-09T17:48:38Z

I just tried it with longer text in the editable div, and I see that you can also tap to the end of any given word. 🤦

raineorshine · 2025-01-09T18:12:36Z

I just tried it with longer text in the editable div, and I see that you can also tap to the end of any given word. 🤦

Yes, that too! In that case, I really don't recommend trying to programmatically recreate the default caret offset behavior on tap.

ethan-james · 2025-01-09T18:14:05Z

I just tried it with longer text in the editable div, and I see that you can also tap to the end of any given word. 🤦

Yes, that too! In that case, I really don't recommend trying to programmatically recreate the default caret offset behavior on tap.

Yes, I would like to avoid it as well! I'm looking into why the focus delay exists, so hopefully that will bear fruit.

raineorshine · 2025-01-09T18:24:12Z

Thanks. Let me know if any of the existing selection logic needs clarification. I may not be able to explain all the details that have built up over time, but I'll do my best to elucidate anything that is particularly confusing.

ethan-james · 2025-01-09T19:29:29Z

I found out that react-dnd was using this option: { delayTouchStart: TIMEOUT_LONG_PRESS_THOUGHT } which was causing about a 300ms delay before focus. I removed it and I think drag & drop is still working OK, although there may be specific reasons why that delay was included in the first place.

There still may be about a 100ms delay between touch events & focus.

It looks a bit different from the CodePen

but there could be React or app-related overhead baked into that.

raineorshine · 2025-01-09T19:34:10Z

Good catch! Interesting that that changed the touchend timing. We want the long press and drag-and-drop timing to remain unchanged from the user's perspective, so hopefully this doesn't affect that.

ethan-james · 2025-01-10T17:21:06Z

Long press timing should be unaffected, and I tested drag-and-drop some more. Without any delayTouchStart, drag-and-drop works well but takes control immediately and doesn't allow drawing gestures on top of editable elements.

If I set delayTouchStart to 100ms, then everything seems to feel similar to how it was before. Gestures can be drawn, presumably the window for allowing them is smaller but I can't really tell from playing around with it.

There was always a 100ms delay (even with delayTouchStart set to 0) and I haven't been able to locate the cause of that. It seems remarkably consistent in its timing, so presumably it's intentional on some level. I tried to remove react-dnd entirely to see if the delay went away, and as best I could tell it did not. That was a messy process, so I'm not terribly confident in the result that I got. This delay also affects non-editable elements such as the hamburger menu. The 300ms delay that was plaguing the hamburger menu is also down to 100ms, but it is still affected. To be clear, hamburger menu is still using fastClick so there's currently no delay. I'm just talking about focus/click events.

So as things stand, I think that setting delayTouchStart: 100 is the way to go. In addition to the possible inevitability of that delay, it also restores behavior like drawing gestures on top of thoughts. I haven't committed that change yet, in case you have a different view.

raineorshine · 2025-01-11T14:33:58Z

Thanks for your thoroughness and excellent troubleshooting.

Apologies if this question has been answered already, but why is touchend short circuited? Maybe if the default touchend was allowed then onFocus wouldn't be needed. However I haven't reviewed the intricacies of the different events that are fired in this case so I may be missing something.

While your proposed path forward seems reasonable given the limitations and challenges, I have two concerns:

We still don't actually know the cause of the 100ms delay.
When touchend fires, drag-and-drop should not be relevant anymore. A short tap won't trigger a long press or drag-and-drop. Thus it's not clear why drag-and-drop would be interfering with touchend timing. (Or is it interfering with onFocus timing?)

If you can offer any insight into the above, it would be greatly appreciated. Also please let me know if the time you have invested in this issue has exceeded reasonable expectations for the original scope. I'd like to get to the bottom of this, and I'm willing to allocate more resources to make it happen.

ethan-james · 2025-01-11T17:18:16Z

If I understand the question correctly, it was being short circuited before when I was calling preventDefault inside of onTap to prevent onFocus from ever firing. Now it's not being short circuited, because it isn't preventing the default behavior, which is the continuation of the event chain. I'm not sure if touchend does anything by itself, except maybe open the little context menu. focus is where the browser selection is created natively.

touchend is firing right away, it's the mousedown/mouseup & subsequent focus event that are being delayed. I see your point about not really understanding why drag & drop would be interfering with non-touch events, and the only evidence I have is that changing delayTouchStart definitely changes the gap between touch & mouse/focus events. That delay allows other consumers to use the touch events, such as gesture drawing.

I think I can manage one more deep dive into this before we run up against reasonable expectations for scope. It looks like the Safari profiler contains pretty good timing information about when the various events are triggered, and hopefully I can decipher that output to get a sense for what's causing the delay.

ethan-james · 2025-01-11T20:48:44Z

So it doesn't look like it was actually related to touch events after all. Using the profiler, I found out that there was a 100ms timer being set in initEvents.ts:

  /** MouseMove event listener. */
  const onMouseMove = _.debounce(
    () => distractionFreeTypingStore.update(false),
    durations.get('distractionFreeTypingThrottle'),
    {
      leading: true,
    },
  )

that was preventing mousemove from ending and moving on to mousedown. You can see in the first profile log where it says Timer 61 installed - 100ms delay and then mousedown doesn't fire until after Timer 61 fired 100ms later.

You can see that without that debounced event listener, there's more like a 30ms delay between mousemove and mousedown. I don't see anything obvious that would indicate that this 30ms delay is intentional. I had delayTouchStart set to 100ms locally (to facilitate drawing gestures on top of thoughts) but setting that to 0 still results in a 30-35ms delay between mousemove & mousedown.

raineorshine · 2025-01-12T01:49:14Z

If I understand the question correctly, it was being short circuited before when I was calling preventDefault inside of onTap to prevent onFocus from ever firing. Now it's not being short circuited, because it isn't preventing the default behavior, which is the continuation of the event chain. I'm not sure if touchend does anything by itself, except maybe open the little context menu. focus is where the browser selection is created natively.

Okay, thanks! The question is not so relevant if onFocus is where the magic happens.

So it doesn't look like it was actually related to touch events after all. Using the profiler, I found out that there was a 100ms timer being set in initEvents.ts:
...
that was preventing mousemove from ending and moving on to mousedown. You can see in the first profile log where it says Timer 61 installed - 100ms delay and then mousedown doesn't fire until after Timer 61 fired 100ms later.

I see the results, but I'm not sure how much I trust them. What it's showing doesn't seem possible to me. _.debounce sets a timeout for the callback and returns immediately. I can't think of a way that could prevent another event from firing.

Aside: I may be confused as to why mousemove is firing at all. Doesn't this task only involve a tap? There is no mousemove or touchmove that fires when just tapping an element.

I'd like to summarize your recommended course of action to make sure I understand it correctly since there is a lot of information in this thread.

Setting delayTouchStart: 100 reduces the focus delay, partially solving the original issue.
We can't reduce the delay further without breaking long press or drag-and-drop.
There is no way to decouple the drag-and-drop behavior from onFocus.
However, setting delayTouchStart: 100 does change the long press and drag-and-drop timing.

Did I get that right?

Another idea is to set delayTouchStart: 0 and find a way to emulate the delay in our code rather than letting react-dnd do it.

ethan-james · 2025-01-15T21:45:44Z

Changing delayTouchStart should not affect long press behavior (as long as we're not re-using the same constant) and if we reduce delayTouchStart to 0 then we won't be able to draw gestures on top of thoughts anymore. Maybe that's OK if it's intentional.

As for mousemove, it seems to always fire on a clickable element. From https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/HandlingEvents/HandlingEvents.html:

raineorshine · 2025-01-15T21:50:15Z

Changing delayTouchStart should not affect long press behavior (as long as we're not re-using the same constant) and if we reduce delayTouchStart to 0 then we won't be able to draw gestures on top of thoughts anymore. Maybe that's OK if it's intentional.

No, we still want to be able to gesture anywhere.

As for mousemove, it seems to always fire on a clickable element. From developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/HandlingEvents/HandlingEvents.html:

Thanks, that's good to know.

ethan-james · 2025-01-15T22:10:06Z

As for decoupling drag-and-drop behavior from onFocus, I think we're limiting the scope of that discussion to

options supported by react-dnd or
replacing/overriding react-dnd behavior

Is that what you meant? It seems to me that we consider touchend to be more important than touchstart because touchend signals that neither a drag-and-drop nor a gesture is occurring. The react-dnd options are here: https://react-dnd.github.io/react-dnd/docs/backends/touch and I'll try to see if any of the options other than delayTouchStart are helpful.

raineorshine · 2025-01-15T22:16:24Z

Yes, theoretically it should be possible to trigger the tap as soon as touchend fires, since we know that a drag-and-drop is not occurring. I'm not sure what react-dnd supports out of the box for this though.

I'm just trying to think of other ideas to get the behavior we want. It's challenging though since we can't manually set the focus.

ethan-james · 2025-01-15T23:32:08Z

I can't find any discussion of this online, but I modified my CodePen to show pretty conclusively that simply setting a timeout in the touchstart handler delays execution of the focus event (after it delays mouse events)

https://codepen.io/ethan-james-the-sasster/pen/ByBYqrL

Let me know whether or not you see the delay!

ethan-james · 2025-01-15T23:39:54Z

Even more amazingly, it seems to delay events up until 400ms, and then not above that threshold. I updated the CodePen again to demonstrate.

ethan-james · 2025-01-15T23:53:03Z

Here is some other poor soul who went through the same thing and dug way deeper into WebKit to figure out what was going on: https://cldfire.dev/blog/when-settimeout-is-blocking/

raineorshine · 2025-01-16T09:15:37Z

Wow! This is one of the craziest browser behaviors I've seen in a long time. It actually makes sense now in terms of a way to make hover events backwards compatible with touch screens. Fascinating.

I can't find any discussion of this online, but I modified my CodePen to show pretty conclusively that simply setting a timeout in the touchstart handler delays execution of the focus event (after it delays mouse events)

https://codepen.io/ethan-james-the-sasster/pen/ByBYqrL

Let me know whether or not you see the delay!

Yes, indeed. 401 is fast, 400 is slow. Crazy.

Here is some other poor soul who went through the same thing and dug way deeper into WebKit to figure out what was going on: https://cldfire.dev/blog/when-settimeout-is-blocking/

And here's a juicy discovery:

This is handy stuff, and ultimately what I was able to use to get out of the debacle I started with. role="button" and various other ARIA roles (full list contained in the isARIAControl method) can be used to easily opt out of this content observation behavior, eliminating the delay involved in waiting on the setTimeout callback to finish executing.

I added role=button to the slow contenteditable in the CodePen and the delay went away! Could you try to apply this in em?

ethan-james · 2025-01-16T17:48:57Z

Looks like it worked!

raineorshine · 2025-01-16T21:42:05Z

Wowwwww. What a payoff!!! 😄

In fact, it's so good that I don't think fastClick is even needed any more! 😮 I tested role='button' with onClick and it's lightning fast ⚡️

Honestly, this is a huge win. It's going to make the app really feel native. Thank you 🙏

Bullet: Use fastClick for clickHandler

23ee553

ethan-james marked this pull request as draft January 2, 2025 22:19

ethan-james added 2 commits January 3, 2025 15:30

Editable: set cursor onTap rather than onFocus

27dddfb

Editable: iOS Safari - place caret at end onTap

e2ab83f

ethan-james marked this pull request as ready for review January 4, 2025 00:11

ethan-james commented Jan 4, 2025

View reviewed changes

useEditMode: explicitly focus contentRef on iOS

5d6a76b

ethan-james commented Jan 4, 2025

View reviewed changes

raineorshine mentioned this pull request Jan 4, 2025

selection.set does not always open keyboard #2753

Open

ethan-james changed the title ~~Bullet: Use fastClick for clickHandler~~ Bullet/Editable: Use fastClick for clickHandler Jan 5, 2025

ethan-james mentioned this pull request Jan 5, 2025

Editable: cancel tap if it moves beyond threshold #2754

Merged

trevinhofmann self-assigned this Jan 8, 2025

trevinhofmann approved these changes Jan 8, 2025

View reviewed changes

src/components/Bullet.tsx Outdated Show resolved Hide resolved

trevinhofmann assigned raineorshine and ethan-james and unassigned trevinhofmann Jan 8, 2025

Update src/components/Bullet.tsx

a81cfd8

Co-authored-by: Trevin Hofmann <[email protected]>

raineorshine requested changes Jan 8, 2025

View reviewed changes

Merge branch 'main' into 1691-bug-fastClick-setCursor

92d9029

Revert changes to onTap, eliminate dnd touch delay

394b481

ethan-james mentioned this pull request Jan 15, 2025

cursorCleared: remove asyncFocus #2772

Merged

ethan-james added 3 commits January 16, 2025 09:37

Merge branch 'main' into 1691-bug-fastClick-setCursor

304effe

Editable: role=button to stop WebKit timeout delay

a09f1cf

Editable: comment explaining role=button

cb192ac

raineorshine approved these changes Jan 16, 2025

View reviewed changes

raineorshine merged commit d83defc into cybersemics:main Jan 16, 2025
3 checks passed

raineorshine mentioned this pull request Jan 16, 2025

Replace fastClick with role='button' #2776

Open

raineorshine mentioned this pull request Feb 5, 2025

[Mobile] Cursor set on wrong thought when siblings expand + collapse on tap #2804

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bullet/Editable: Use fastClick for clickHandler #2752

Bullet/Editable: Use fastClick for clickHandler #2752

ethan-james commented Jan 2, 2025 •

edited

Loading

ethan-james Jan 4, 2025

ethan-james Jan 4, 2025

raineorshine Jan 4, 2025

trevinhofmann left a comment

raineorshine left a comment

ethan-james commented Jan 8, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

ethan-james commented Jan 9, 2025 •

edited

Loading

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 10, 2025 •

edited

Loading

raineorshine commented Jan 11, 2025

ethan-james commented Jan 11, 2025

ethan-james commented Jan 11, 2025

raineorshine commented Jan 12, 2025 •

edited

Loading

ethan-james commented Jan 15, 2025

raineorshine commented Jan 15, 2025

ethan-james commented Jan 15, 2025

raineorshine commented Jan 15, 2025 •

edited

Loading

ethan-james commented Jan 15, 2025

ethan-james commented Jan 15, 2025

ethan-james commented Jan 15, 2025

raineorshine commented Jan 16, 2025

ethan-james commented Jan 16, 2025

raineorshine commented Jan 16, 2025

Bullet/Editable: Use fastClick for clickHandler #2752

Bullet/Editable: Use fastClick for clickHandler #2752

Conversation

ethan-james commented Jan 2, 2025 • edited Loading

ethan-james Jan 4, 2025

Choose a reason for hiding this comment

ethan-james Jan 4, 2025

Choose a reason for hiding this comment

raineorshine Jan 4, 2025

Choose a reason for hiding this comment

trevinhofmann left a comment

Choose a reason for hiding this comment

raineorshine left a comment

Choose a reason for hiding this comment

Steps to Reproduce

Current Behavior

Expected Behavior

ethan-james commented Jan 8, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

ethan-james commented Jan 9, 2025 • edited Loading

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 9, 2025

raineorshine commented Jan 9, 2025

ethan-james commented Jan 10, 2025 • edited Loading

raineorshine commented Jan 11, 2025

ethan-james commented Jan 11, 2025

ethan-james commented Jan 11, 2025

raineorshine commented Jan 12, 2025 • edited Loading

ethan-james commented Jan 15, 2025

raineorshine commented Jan 15, 2025

ethan-james commented Jan 15, 2025

raineorshine commented Jan 15, 2025 • edited Loading

ethan-james commented Jan 15, 2025

ethan-james commented Jan 15, 2025

ethan-james commented Jan 15, 2025

raineorshine commented Jan 16, 2025

ethan-james commented Jan 16, 2025

raineorshine commented Jan 16, 2025

ethan-james commented Jan 2, 2025 •

edited

Loading

ethan-james commented Jan 9, 2025 •

edited

Loading

ethan-james commented Jan 10, 2025 •

edited

Loading

raineorshine commented Jan 12, 2025 •

edited

Loading

raineorshine commented Jan 15, 2025 •

edited

Loading