codemirror / dev

Development repository for the CodeMirror editor project
https://codemirror.net/
Other
5.94k stars 377 forks source link

line-height + lineWrapping causes inconsistent cursor positioning on click #1413

Closed ajguyot closed 4 months ago

ajguyot commented 4 months ago

Describe the issue

When using CodeMirror with the lineWrapping extension enabled, there is some inconsistency with where the cursor is positioned when clicking within the trailing whitespace of a wrapped line. If you click horizontally to the letters of the text, the cursor is correctly positioned on the line in which you clicked, residing behind any trailing wrap points. However, if you click within the line's line-height area, slightly above or below the letters themselves, then the cursor is incorrectly positioned at the start of the next line.

While the position is technically identical once you start typing, it's visually jarring to have the cursor be moved to the next line like this.

This behavior can be reproduced even when using with the default line-height, but the area to trigger it is very small. However, if you increase the line-height for cm-lines, then it becomes very easy to trigger and feels particularly disruptive.

Tested in Chrome and Safari, and attached a video demonstrating the issue. Used the video from Safari because the Safari cursor is a lot more visually noticeable, which makes this issue stand out a lot more than it does in Chrome. The reproduction link adds line-height for ease-of-reproduction, but as you can see in the video, the bug can be reproduced even without the added line-height.

Thanks for any help here!

Browser and platform

Chrome, Safari, macOS

Reproduction link

https://codemirror.net/try/?c=aW1wb3J0IHttaW5pbWFsU2V0dXAsIEVkaXRvclZpZXd9IGZyb20gImNvZGVtaXJyb3IiCgpuZXcgRWRpdG9yVmlldyh7CiAgZG9jOiAiLi4uIiwKICBleHRlbnNpb25zOiBbCiAgICBFZGl0b3JWaWV3LmxpbmVXcmFwcGluZwogIF0sCiAgcGFyZW50OiBkb2N1bWVudC5ib2R5Cn0pCgpjb25zdCBzdHlsZSA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoInN0eWxlIikKc3R5bGUudGV4dENvbnRlbnQgPSAiLmNtLWxpbmUgeyBsaW5lLWhlaWdodDogMyAhaW1wb3J0YW50OyB9Igpkb2N1bWVudC5oZWFkLmFwcGVuZENoaWxkKHN0eWxlKQ==

ajguyot commented 4 months ago

Ope, forgot to attach the video, here it is:

https://github.com/user-attachments/assets/2727cfd9-ac2c-47ad-9020-baa0ea7b1a80

marijnh commented 4 months ago

Attached patch should help with this.