Conversation
|
@alexdima I've realized that my commit message might be seen as criticising your earlier commit, sorry if it came across that way. I hadn't realized that this tweak was part of a fix for an earlier issue. It doesn't seem that issue #55640 recurs with my reversal of that "centering"; but if you'd prefer, I can look at the approach outlined above that can shift the cursor without moving the text inside it. |
alexdima
left a comment
There was a problem hiding this comment.
I see how this is super annoying. Maybe we can avoid doing the -1 when having a wide cursor. But simply removing the -1 ends up impacting negatively the cursor placement on macOS with default settings:
Before the proposed change:
With the proposed change:
fe14a16 to
062c9a3
Compare
e3de4f6 to
8b1f1fb
Compare
|
I understand; I've updated the PR so the cursor does shift by 1px but it adds left padding so the text is where it's supposed to be. What do you think @alexdima ? I've spotted something else: it looks like the last line of real code which returns |
The change looks alright in principle, but I think the extra padding is making the default cursor "fatter". Here it is running from source on macOS with default settings:
That's a good catch! I've created #164484 to track fixing that. |
* Remove ill-advised tweak * Shift cursor back by 1px but leave text in place * Always use `box-sizing: border-box` Co-authored-by: Alexandru Dima <[email protected]>
Fixes #156788
It looks like the "centering" is the main culprit causing my issue.
Test this by the following steps:
This is most visible on high-DPI screens where a pixel translates to several real pixels.
If we want to keep the "centering", or if we actually want to truly center the line cursor by half its width, we'd need to add left padding to the cursor element so that the shift left is balanced by the content shifting right. It would mean adding left padding (or all paddings) to fastDomNode. Let me know if you'd like that.