Cursor moves to thought hidden by autofocus

[raineorshine]

Steps to Reproduce

- a
  - b
    - c
- d

1. Set the cursor on c so d disappears
2. Tap 1 line height below c, to the right of where d would be.

Current Behavior

Cursor moves to d.

Expected Behavior

Cursor should move to b.

Tapping on empty space with no visible thoughts to the left or right should always activate cursorUp.

[raineorshine]

Only occurs on mobile, but you can reproduce it on desktop by forcing isTouch = true.

[raineorshine]

@shresthabijay Thank you for the solution! We should still do some forensic work to understand the bigger picture of this. When there is a discrete regression, it's important to identify the commit that broken to inform our fix. I have often fixed a regression only to realize I compromised the original behavior in some way. Usually it's better to identify why something stopped working rather than adding additional code to make it work again.

I put it softly in the description, but I probably should have made it clear that it was a requirement :). Could you find out which commit broke this and why? Thanks!

[shresthabijay]

@raineorshine Understood. I opted to go with the fix as it was fairly easy. I will get back to you after I identify the commit that created the regression.

[shresthabijay]

@raineorshine The regression was introduced from this commit e12699e.

onTap replaced onMouseDown for device with touch screens. Previously both onTapEnd and onMouseDown was being called on touch screen devices. This is because touch events emit artificial mouse events on succession unless touch events are prevented. Since onMouseDown had cursorBack logic, it was working for mobile devices prior to this commit.

onTap doesn't have cursorBack logic. With this commit onTap is being called on onmousedown event for touch screens, thus this regression was introduced.

Also after this commit onTap is being called twice for the same touch event. Once on onTouchEnd and another on onMouseDown. Since we cannot prevent onTouchEvent as it will prevent focus. Instead we need to add cursorBack logic to onTap and pass noop to onMouseDown for touch screen devices.

[raineorshine]

@raineorshine The regression was introduced from this commit e12699e.

Great, thanks!

Reference: #946

onTap replaced onMouseDown for device with touch screens. Previously both onTapEnd and onMouseDown was being called on touch screen devices. This is because touch events emit artificial mouse events on succession unless touch events are prevented. Since onMouseDown had cursorBack logic, it was working for mobile devices prior to this commit.

Makes sense

Instead we need to add cursorBack logic to onTap and pass noop to onMouseDown for touch screen devices.

That sounds reasonable. I see you added the cursorBack logic to onTap. Do you want to add the noop in the PR as well?

[raineorshine]

Regression prevention checklist: #946 (comment)

[raineorshine]

Issue still occurs when in edit mode :(

I’m going to look at this myself since I got involved recently in trying to get everything passing in the regression checklist, so it’s fresh.

@shresthabijay Anything from the regression checklist we can cover in the new puppeteer testing environment would be a big help!

Most of the regressions are on mobile Safari so unfortunately we can't test with puppeteer.

[onurpolattimur]

Is there any specific reason for not hiding the editable element for hidden thoughts?

[raineorshine]

Maybe setting visibility: hidden would work. I hadn't tried that. That would be less logic.

[raineorshine]

Steps to Reproduce

1. Create a thought.
2. Close the keyboard
3. Activate cursorUp (cursor becomes null)
   • by swiping right on top of the thought
4. Tap on the thought

Current Behavior

The cursor moves to the thought and enters edit mode.

Expected Behavior

Should not enter edit mode when the cursor changes.

[onurpolattimur]

Fixed and explained in #1135