Codecov Report

Merging #2551 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #2551   +/-   ##
=======================================
  Coverage   44.93%   44.93%           
=======================================
  Files         352      352           
  Lines       14336    14336           
  Branches     1863     1863           
=======================================
  Hits         6442     6442           
  Misses       7669     7669           
  Partials      225      225

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af03770...5834931. Read the comment docs.

@jaanauati thanks for the PR tbh as a Mac User I noticed that it wasn't quite right after the PR (I forget which) came in but it was slightly improved and I don't scroll much, I'll definitely give this a try locally but since it affects multiple platforms would be good to see what the effect is for @CrossR or @badosu as well

I'll give this a test when I'm home on my Windows Surface and Desktop, so we get both trackpad + trackpad events, since they are both treated differently.

@jaanauati sorry for the delay on trying this out been quite busy lately but will give this a go once I get a moment

Tested on Arch, looks good to me 👍

I tested this with both a mouse and a trackpad on Max OS X High Sierra and it worked nicely.

Just a thought on the side: If we want to make this even smoother and more accurate, could we directly use the fontHeightInPixels as the scrolling threshold and then, instead of ScrollWheelDown/Up keys, send <C-E>/<C-Y> for scrolling only one line? Could this possibly create issues when users have remapped <C-E> or <C-Y> to something else?

Maybe we could even decide based on the _scrollDelta to send one-line(<C-E>/<C-Y>), three-line (<ScrollWheel*>), half-page (<C-D>/<C-U>) or full-page (<C-ScrollWheel*>) keys?

Edit: Or even better: calculate the number of rows and send those via ${rowCount}<C-E> and the same for <C-Y>, respectively

In any case, what you built so far already seems like a great improvement. Let's merge it once @CrossR was able to verify that it works on Windows :)

Tested this on my Surface and with an MX Master and I didn't notice any difference, so if its helping on Mac, thats great!