Cursor is overriden when upload finishes

feature
1
  1. I’m writing a post and upload a photo (which is large or which takes long due to slow internet connection)…
  2. While it uploads, I continue writing…
  3. Once the download finishes, my cursor lands behind the image!

Expectation: The cursor should be preserved.

2

Isn’t this expected since the text has to be replaced when the upload is complete? That’s not a bug.

3

It’s certainly a side effect of replacing the text, but it’s also not so nice for the user. I think we could save the cursor position before we replace the text and restore it thereafter.

1 Like
4

Still if you are actively typing when that happens weird stuff is gonna happen.

1 Like
5

Just tried it out a couple of times and all I could reproduce is that, if I type very fast, one character is omitted. Is this what you refer to as “weird stuff”?

I would say though that these are two phenomena that add up and getting rid of each of them is desirable, or isn’t it?

6

So the upload placeholder replacement is triggered in composer-editor.js.es6 here (on cancel) and here (on success) and handled by d-editor.js.es6 here or here respectively:

https://github.com/discourse/discourse/blob/7515f4eec20abb5c869434c2ccb2541127628969/app/assets/javascripts/discourse/components/d-editor.js.es6#L526-L533

This would be the place where we could preserve the cursor position, given that we replace the text in a synchronous fashion (*):

  • If the cursor position is before the needle, preserve it.
  • If the cursor position is in the needle, set it to the end of the replacement.
  • If the cursor position is behind the needle, add (replacement.length - needle.length) to the cursor.

(*) = …but _setSelection (which is used by _replaceText to explicitly set the cursor behind the image) works in an asynchronous fashion.

2 Likes
7

Just in case you change your mind and give it a try (works for me):

https://github.com/discourse/discourse/pull/4630

PS: Last PR for some time, as vacation ends for me today.

7 Likes
8

Sure if @zogstrip likes it we can pull it in.

1 Like
9

:heart: it. Just need to fix the tests and I’ll merge it :wink:

2 Likes