Repository navigation
Improve search tick mark positioning - #11293
Conversation
|
There was an issue like this on CodeMirror: can we use that logic? If there aren't specific requests, can you avoid exposing the setter/getter of |
|
I've already had the appropriate code in there to calculate the scroll height, but I've changed it because CMs code seems to be a tiny bit faster. |
| } else { //(Linux) | ||
| trackOffset = 2; // Custom scrollbar CSS has assymmetrical gap; this approximates it | ||
| } | ||
| trackOffset = scrollbarTrackOffset; |
|
I left some comments. |
3d73ef2 to
be899a2
Compare
|
|
||
| /** | ||
| * @param {number} offset Amount of vertical space above and below the scrollbar, in pixels | ||
| */ |
There was a problem hiding this comment.
Can you change the JSDoc here too, to give it a proper description?
|
Since you have an use case I'm fine with the setter/getter. @marcelgerber @dakaraphi do you know how this will interact with #12442? |
|
@ficristo @marcelgerber just looking at the changes here I don't see that this would interact with the tick mark optimizations for search in #12442 |
|
@ficristo Changed that doc too 👍 |
| posArray.forEach(function (pos) { | ||
| var top = Math.round(pos.line / editor.lineCount() * trackHt) + trackOffset; | ||
| var cursorTop = getY(cm, pos); | ||
| var top = Math.round(cursorTop / editorHt * trackHt) + trackOffset; |
There was a problem hiding this comment.
I missed this before, sorry, but it should be a single var
|
@dakaraphi Thank you to have checked. @marcelgerber I left a small nit, LGTM with or without it. |
716bfe6 to
cd7c836
Compare
cd7c836 to
6da3ed8
Compare
|
@ficristo Thanks for reviewing! |
For #11079.
This does not only fix the
"scrollPastEnd"issue, but also ones where scroll tick marks were inaccurately placed in minified lines, and so on.Also adds new
getScrollbarTrackOffset/setScrollbarTrackOffsetmethods which can be utilized by extensions.Notice though that this new method is more expensive, so we should look out for performance regressions!