Cursor is overriden when upload finishes

Contribute Feature

AI-generated summary

A user (claas) reported an issue where the cursor is overridden when an image upload finishes, landing behind the uploaded image. codinghorror thought this might be expected behavior due to the text replacement, but claas argued that it’s not ideal for the user experience and proposed saving the cursor position before replacing the text.

codinghorror expressed concerns about the potential issues when typing actively while the image is being uploaded, but claas found that the only issue was a single omitted character when typing quickly.

To fix the issue, claas identified the relevant code in composer-editor.js.es6 and d-editor.js.es6 and suggested preserving the cursor position when replacing the text. A pull request (#4630) was submitted, and after codinghorror and zogstrip reviewed it, it was approved for merge once the tests are fixed.

1
  1. I’m writing a post and upload a photo (which is large or which takes long due to slow internet connection)…
    image.png322×223 4.32 KB
  2. While it uploads, I continue writing…
    image.png318×233 6.07 KB
  3. Once the download finishes, my cursor lands behind the image!
    image.png339×272 8.68 KB

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 curtida
4

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

1 curtida
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:

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 curtidas
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 curtidas
8

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

1 curtida
9

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

2 curtidas