Use TextEditor::pixelRectForScreenRange instead of Marker::getPixelRange #138
Conversation
|
Maybe you should also change the engine in the package.json so users with current versions won't get the next package release. |
This should actually be a non-breaking change, because we had Thank you for taking the time to review this! 🙇 |
|
Hi, @as-cii! Thanks for keeping an eye out. I did some tests, and it works great except for in a case where there are more than one color value (?) on the same line. This screenshot is from
|
|
@thomaslindstrom: for some reason I assumed This should now work properly: please have one last look 👀. Thanks! |
| _cursorPosition = Cursor.getPixelRect() | ||
| _cursorPosition.left = _selectionPosition.end.left - ((_selectionPosition.end.left - _selectionPosition.start.left) / 2) | ||
| _cursorPosition.left = right - ((right - left) / 2) |
There was a problem hiding this comment.
Why not just _cursorPosition.left = right - (width / 2)? :) Isn't width the same as (right - left)? because right = left + width... Maybe i'm wrong again... 😕
There was a problem hiding this comment.
This is definitely a good point! 👍 Thanks!
|
@thomaslindstrom: I think this is good to go now. 🎆 |
| @@ -357,9 +357,10 @@ | |||
| # the middle of the selection range | |||
| # TODO: There can be lines over more than one row | |||
| if _match | |||
| _selectionPosition = _selection.marker.getPixelRange() | |||
| {left, width} = Editor.pixelRectForScreenRange(_selection.getScreenRange()) | |||
| right = left + width | |||
There was a problem hiding this comment.
Please prefix the variables with _.
|
Sorry for taking long. I'm going back home on Monday, so I'll push this out then. |
|
No worries, I have just updated this with your suggestions. 😉 |
Use TextEditor::pixelRectForScreenRange instead of Marker::getPixelRange
|
Alright, this is now included in |
|
❤️ |
This replaces
Marker::getPixelRangewithTextEditor::pixelRectForScreenRange, because the former was not part of the public API and will, therefore, be dropped in the next version of Atom (see atom/atom#8905 for further information).Please, note that starting from the next version we're going to deprecate all those methods in
TextEditorthat refer to pixel coordinates (e.g.::setScrollTop,::pixelPositionForScreenPosition,::pixelRectForScreenRange, etc.). You will still be able to use them, but we discourage it as they might disappear in a future version. These methods will be moved toTextEditorElementand, therefore, we recommend using those ones instead.Unfortunately, I wasn't able to verify against some specs that the change I applied in this PR worked correctly. However, it seems to be fine from a cursory manual test.
Please, let me know if I can improve this somehow. Thanks! 🙇
/cc: @thomaslindstrom