mariospr/text-scaler-gnome-shell-extension

Wouse wheel support

Closed this issue · 14 comments

Could you add mouse wheel support on the slider ?
Currently the mouse wheel change the displayed value but it is not applied.

Sorry, I've just seen this almost by chance, not sure why I did not get any notification from github.

I'll take a look to this now, seems like a feature I would appreciate as well! :)

I looked into this yesterday, but I'm unsure I really want to enable that now, since once I did it (in a local branch) I remembered why I wrote it this way in the first place:

When I wrote the extension I could have made it so that the text-scaling-factor would change whenever the slider changed its position (on slider's 'value-changed' signal), but that would be really unconvenient if, instead of mouse-wheeling, you click on the slider and drag it: as soon as the slider moved, the scaling factor would change and everything would go crazy since the cursor would be suddenly pointing to a completely different area on the screen, often causing the slider to move in unpredictable ways.

So, instead of updating the factor on 'value-changed', I decided to do it on 'drag-end', which would allow you to move the slider around as much as you like without updating the factor until you release the mouse button. And because how this slider widget is implemented in the gnome-shell, drag-end would be emitted as well when moving the slider with the arrow keys, so it looked like a good solution.

Unfortunately, mouse-wheeling emits value-changed but not drag-end, meaning that drag-end does not work in that case. Problem is, I really don't want to get back to the unpredictable behaviour when "clicking & dragging" that I would get connecting to value-changed so I think best options are either:

  • Use value-changed to know when to update the scaling factor but not to do it right away. Instead, use some kind of timer so that the factor gets updated only after a certain timeout once the slider gets "quiet" if the drag-end signal is never emitted.
  • Propose changes in the gnome-shell's slider widget, so that the drag-end signal gets emitted whenever the mouse-wheel is used to move the slider one step.

I kind of lean more towards the second option so I'll look into it once I have some time. In the meanwhile, though, I'd be happy to integrate a patch implementing the other approach if it works well enough, but I probably won't be spending time myself on that until I'd checked the other option.

Hi,
What about disconnecting 'value-changed' signal while slider is dragged ?
(disconnect on drag-begin and reconnect on drag-end)

That could actually be a better idea than the ones I proposed, but still would need changes in the gnome-shell, since drag-begin is not implemented at the moment in the slider widget:
https://git.gnome.org/browse/gnome-shell/tree/js/ui/slider.js

But still, something worth considering, might be even a better thing to do than emitting drag-end when the mouse-wheel is used, as there's no real "drag" in the first place. I'll look into it, thanks!

Or maybe use the slider _dragging attribute to ignore the scroll-event if slider is being dragged ?

That's a private attribute, I'd rather use something public.

Ok ... I wonder if drag-begin signal (which is inherited from Widget) is triggered, even if not used by the Slider class ? Did you have try ?
Edit : Oops, it may not be a Widget after all

What about Slider.actor (the StDrawingArea) signals ?
In docs I read only the 'repaint' signal is listed, but we can see in Slider.js more are available like button-press-event and button-release-event ?

Ok ... I wonder if drag-begin signal (which is inherited from Widget) is triggered, even if not used by the Slider class ? Did you have try ?
Edit : Oops, it may not be a Widget after all

As you said, the shell's Slider class is not a GtkWidget, so won't work.

What about Slider.actor (the StDrawingArea) signals ?
In docs I read only the 'repaint' signal is listed, but we can see in Slider.js more are available like button-press-event and button-release-event ?

Those could work, yes. But still I think it would probably be cleaner if the Slider emitted a drag-begin signal, so I'll propose that at some point soon (kind of busy with other things now).

Thanks again!

What about Slider.actor (the StDrawingArea) signals ?
In docs I read only the 'repaint' signal is listed, but we can see in Slider.js more are available like button-press-event and button-release-event ?

Those could work, yes. But still I think it would probably be cleaner if the Slider emitted a drag-begin signal, so I'll propose that at some point soon (kind of busy with other things now).

I've worked on this a bit tonight and I have a fix for the extension that works pretty well by relying on a 'drag-begin' signal being emitted by the Shell's Slider widget. See the commit here: 83030a2

For the gnome-shell part I've filed a bug and proposed a patch that works perfectly (tested locally):
https://bugzilla.gnome.org/show_bug.cgi?id=761208

Let's see what they think...

The patch for gnome shell landed upstream and I just pushed the fix for the extension to master as well.

As a reminder, I'll keep this issue open anyway while GNOME Shell 3.20 is not released, which is when I will proposing the update in extensions.gnome.org, and then I'll close it.

Okay nice !

This has been fixed for a while. Closing