Cursor is overriden when upload finishes

  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.

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

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

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

1 Like

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?

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

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

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

1 Like

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

2 Likes