joouha / euporie

Jupyter notebooks in the terminal
https://euporie.readthedocs.io
MIT License
1.54k stars 36 forks source link

Dragging the scrollbar to scroll is extremely finnicky #32

Closed jjeffrey closed 1 year ago

jjeffrey commented 1 year ago

When I use my mouse to left click-and-drag the scrollbar in Euporie Notebook, the scrolling is very irregular. Additionally, when I release the left mouse button, the scroll bar sometimes continues to be dragged with the vertical motion of my mouse; an additional click on the scrollbar seems to remove this effect.

The versions of software I am using, as well as a GIF that demonstrates my issue, are included below.

output

jerch commented 1 year ago

Figured that as well in the browser notebook demo. Imho the mouse move event gets only delivered if you are perfectly on the bar, as soon as you move one pixel to the left or right the event gets not through anymore.

@joouha Would it be possible to keep tracking the mouse moves for scrolling, if mousedown was triggered on the scroll bar? schematically:

(Have not checked how you implemented the mouse event dispatching, thus I simply took the naming from browser convention.)

joouha commented 1 year ago

Thanks for reporting

@jerch Yes you are exactly correct - the current scrollbar implementation only recognizes drag events when the mouse is over the scrollbar itself.

There's no built-in support for this in prompt-toolkit, but I've got a couple of idea how it could be implemented - I'll take a stab this soon!

jjeffrey commented 1 year ago

Even with the mouse maintained over the scrollbar, I've noticed that the scrolling resolution is lower when I scroll by dragging the scrollbar than when I scroll with the mouse scroll wheel, where I get line-by-line resolution.

Based on quick testing, scrolling with the scrollbar generally seems to move in increments of 5, 6, or 7 lines, depending on things (perhaps rounding related?). Additionally, when scrolling down and starting at the top of the cell, e.g. with line 1 at the top of the screen, excluding the top cell border (green in the image below), the screen first scrolls up to include the top cell border and maybe some of the next cell, before beginning to scroll down when you drag further. This causes the first "effective" increment to be lower (3 or 4 in my quick testing).

image

joouha commented 1 year ago
  1. The low scrolling resolution when dragging the scroll-bar is because the mouse position is only reported to the nearest terminal character cell. Especially for long notebooks, this will result in large scroll jumps between scroll-bar positions, because the vertical cell resolution of a terminal is generally not very high.

    I've just seen that some terminals support reporting mouse co-ordinates in pixel rather than terminal cell, so it might be possible to improve things here. This will be a bit complicated to implement though.

  2. The initial scroll-bar jumping you're seeing looks like a bug caused by an off-by-one error to me - this should be fixable.

jerch commented 1 year ago

I've just seen that some terminals support reporting mouse co-ordinates in pixel rather than terminal cell, so it might be possible to improve things here. This will be a bit complicated to implement though.

Yes, there is an old DEC standard for this - locator protocol. But I think thats only implemented by very few terminals (xterm has it). I remember a discussion in terminal-wg about extending the SGR mouse protocol by pixel notions. Not sure if any terminal does that by now. (Maybe I gonna warm up that topic for xterm.js, back then I kinda refused the idea of pixel precision, as it creates tons of event messages.)

Maybe @textshell knows, if there is some progress regarding a pixel precise SGR-like mouse report?

joouha commented 1 year ago

I found out about it here: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Extended-coordinates

WezTerm, Kitty, Foot & Contour also appear to support it

Would be really cool to have it in xterm.js :smiley:

The only problem is I'm not sure there's any way to detect whether a terminal supports it

jerch commented 1 year ago

Oh wow, didnt know that SGR-pixels is a thing in xterm. Thanks for the headsup.

jerch commented 1 year ago

The only problem is I'm not sure there's any way to detect whether a terminal supports it

You can try to detect the SGR-pixels mode state with DECRQM. This would either return:

So a terminal properly implementing DECRQM should notify you with 0, if it does not know 1016. If it knows it, it would send 1 or 2, depending on its state. For a terminal not imlementing DECRQM (no result at all) I think you can safely assume no support at for it. Currently xterm.js lacks both, as most older term libs do. Guess we should fix that in xterm.js...

jerch commented 1 year ago

@joouha SGR-pixels is now in xterm.js master, will see about DECRQM.

textshell commented 1 year ago

Yes, there is an old DEC standard for this - locator protocol. But I think thats only implemented by very few terminals (xterm has it). I remember a discussion in terminal-wg about extending the SGR mouse protocol by pixel notions. Not sure if any terminal does that by now. (Maybe I gonna warm up that topic for xterm.js, back then I kinda refused the idea of pixel precision, as it creates tons of event messages.)

Maybe @textshell knows, if there is some progress regarding a pixel precise SGR-like mouse report?

I would recommend against trying to do anything with the old locator protocol. It's been a bit since i looked at it, but it seemed like something best left historic when i looked. Iirc it's even off in common xterm configurations.

Not sure about SGR-Pixels mode. I personally dislike mixing pixels into terminal controls, because fractional cells would have been a more natural unit in my view. But you generally only get what terminals implement. But then guessing the scaling factor also when terminals a resized is going to be somewhat challenging.

jerch commented 1 year ago

I would recommend against trying to do anything with the old locator protocol. It's been a bit since i looked at it, but it seemed like something best left historic when i looked. Iirc it's even off in common xterm configurations.

I agree, DEC locator is def. not worth to be considered.

Not sure about SGR-Pixels mode. I personally dislike mixing pixels into terminal controls, because fractional cells would have been a more natural unit in my view. But you generally only get what terminals implement. But then guessing the scaling factor also when terminals a resized is going to be somewhat challenging.

True, the fractional part just bugged me while getting this up for xterm.js, On the other hand px (css-pixels) is the smallest metric (integer) I can get from the browser for mouse position, so I just forward that. Cannot account for scaled views, as I would introduce a bigger error by the needed int --> float --> position translation --> int conversion. Luckily that px value is in line with the sixel pixel handling and CSI t reports now, independently from any browser viewport scaling.

Thx for your response. :smiley_cat:

joouha commented 1 year ago

Quick update - I've got both global mouse dragging and pixel-resolution scrolling working. I just need to tidy some things up a bit before pushing. But here's a sneaky preview:

pixel-scrolling.webm

(@willmcgugan - I feel like this SGR-Pixel scrollbar dragging is something that should be in texual. It would make your scroll-bars even more soothing!)

jerch commented 1 year ago

DECRQM also landed in xterm.js. The next major version (v5.0) will contain both, mouse SGR-pixels reports and DECRQM. (image addon for v5 is in the making...)

joouha commented 1 year ago

I've released v2.0.8, which includes fixes for both parts of this issue (you will need to use a terminal which supports SGR-pixel reporting, like wezterm, kitty, xterm, etc.).

@jerch This seems to work great in xterm.js v5.0.0 - thanks for implementing SGR-pixel support!

Now I need to update my browser notebook demo...