After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766883 - mouse scroll wheel to pan workspaces with multiple monitors
mouse scroll wheel to pan workspaces with multiple monitors
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: overview
3.20.x
Other Linux
: Normal minor
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 694835 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-05-25 19:52 UTC by opeltd
Modified: 2016-09-27 00:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
overview: Pass on scroll-events on background group as well (1.49 KB, patch)
2016-05-26 17:17 UTC, Florian Müllner
none Details | Review
workspacesView: Consider workspaces-only-on-primary when scrolling (1.36 KB, patch)
2016-05-26 17:17 UTC, Florian Müllner
none Details | Review
workspacesDisplay: Cancel click when panning (1.11 KB, patch)
2016-06-24 16:57 UTC, Florian Müllner
committed Details | Review
overview: Remove stack actor (3.00 KB, patch)
2016-06-24 16:57 UTC, Florian Müllner
committed Details | Review
overview: Move overview actions and scrolling to background group (3.29 KB, patch)
2016-06-24 16:57 UTC, Florian Müllner
committed Details | Review
workspacesView: Allow activating empty workspaces on any monitor (2.33 KB, patch)
2016-06-24 16:58 UTC, Florian Müllner
committed Details | Review
workspacesView: Consider workspaces-only-on-primary when scrolling (1.20 KB, patch)
2016-06-24 16:58 UTC, Florian Müllner
committed Details | Review
workspacesView: Consider workspaces-only-on-primary when panning (1.42 KB, patch)
2016-06-24 16:58 UTC, Florian Müllner
committed Details | Review
workspacesView: Initialize the adjustment's upper bound (1.28 KB, patch)
2016-06-26 17:37 UTC, Rui Matos
committed Details | Review

Description opeltd 2016-05-25 19:52:22 UTC
I have 2 monitors and make use of at least 3 workspaces at the same time. The problem I am having is when I go to Overview and I use the scroll wheel, it works fine as long as I am either scrolling over the primary monitor, or over an application on the secondary monitor. 

Along with this, the scroll wheel works fine almost anywhere on the primary, but the mouse has to be DIRECTLY over an application on the secondary.

Proper function would be to have the same behavior for all monitors. As right now I try to scroll past 2 or 3 workspaces, but get stuck on one when my secondary monitor is empty.
Comment 1 Florian Müllner 2016-05-26 17:17:34 UTC
Created attachment 328569 [details] [review]
overview: Pass on scroll-events on background group as well

The Overview::scroll-event is meant to expose scroll events that
happen anywhere in the overview, but for now only work on the
primary monitor. Include events on the background group that is
known to span all outputs to fix.
Comment 2 Florian Müllner 2016-05-26 17:17:41 UTC
Created attachment 328570 [details] [review]
workspacesView: Consider workspaces-only-on-primary when scrolling

It is odd to switch workspaces using the scroll wheel when the pointer
is on a monitor without workspaces, so only handle scroll events on
non-primary monitors when workspaces-only-on-primary is disabled.
Comment 3 Rui Matos 2016-05-27 14:30:56 UTC
Review of attachment 328569 [details] [review]:

Do we still need to handle scroll events in this._controls.actor ?
Comment 4 Rui Matos 2016-05-27 14:32:55 UTC
Review of attachment 328570 [details] [review]:

ok
Comment 5 Rui Matos 2016-05-27 14:34:15 UTC
Review of attachment 328569 [details] [review]:

In fact, since the scroll events handler propagates the event don't we get duplicate scroll signal emissions this way?
Comment 6 Florian Müllner 2016-05-27 18:34:08 UTC
(In reply to Rui Matos from comment #5)
> In fact, since the scroll events handler propagates the event don't we get
> duplicate scroll signal emissions this way?

No, though I have to admit that I can't figure out off-hand what is stopping the event ...
Comment 7 Florian Müllner 2016-06-24 16:56:54 UTC
(In reply to Florian Müllner from comment #6)
> (In reply to Rui Matos from comment #5)
> > In fact, since the scroll events handler propagates the event don't we get
> > duplicate scroll signal emissions this way?
> 
> No, though I have to admit that I can't figure out off-hand what is stopping
> the event ...

I finally had some time to revisit this - updated patches coming.
Comment 8 Florian Müllner 2016-06-24 16:57:30 UTC
Created attachment 330326 [details] [review]
workspacesDisplay: Cancel click when panning

When switching between workspaces via panning, we don't want to
leave the overview when we end up on an empty workspace.

Drive-by fix.
Comment 9 Florian Müllner 2016-06-24 16:57:42 UTC
Created attachment 330327 [details] [review]
overview: Remove stack actor

The stack was used to overlay a message indicator over the overview
group. That indicator is long gone, so there's no longer a need for
an intermediate actor in the hierarchy.

Another drive-by fix.
Comment 10 Florian Müllner 2016-06-24 16:57:55 UTC
Created attachment 330329 [details] [review]
overview: Move overview actions and scrolling to background group

Both the Overview::scroll-event and actions added via addAction()
are meant to work anywhere in the overview, but for now only work
on the primary monitor. Move the handling to the background group
that is known to span all outputs to fix.
Comment 11 Florian Müllner 2016-06-24 16:58:05 UTC
Created attachment 330332 [details] [review]
workspacesView: Allow activating empty workspaces on any monitor

We allow activating a workspace by clicking it when we know that
the user did not try to select a window and missed (namely: the
workspace is empty). However we currently always check for an
empty workspace on the primary monitor, which doesn't make sense
when the click happened on a different monitor.
Comment 12 Florian Müllner 2016-06-24 16:58:14 UTC
Created attachment 330334 [details] [review]
workspacesView: Consider workspaces-only-on-primary when scrolling

It is odd to switch workspaces using the scroll wheel when the pointer
is on a monitor without workspaces, so only handle scroll events on
non-primary monitors when workspaces-only-on-primary is disabled.
Comment 13 Florian Müllner 2016-06-24 16:58:22 UTC
Created attachment 330335 [details] [review]
workspacesView: Consider workspaces-only-on-primary when panning

It is odd to switch workspaces on the primary monitor when panning on
a monitor without workspaces, so reject the gesture on non-primary
monitors when workspaces-only-on-primary is disabled.
Comment 14 Rui Matos 2016-06-26 17:35:23 UTC
Review of attachment 330326 [details] [review]:

This didn't seem to be a problem for me and we already do it on ::gesture-end but it doesn't hurt either
Comment 15 Rui Matos 2016-06-26 17:35:30 UTC
Review of attachment 330327 [details] [review]:

ok
Comment 16 Rui Matos 2016-06-26 17:35:57 UTC
Review of attachment 330329 [details] [review]:

looks good
Comment 17 Rui Matos 2016-06-26 17:36:15 UTC
Review of attachment 330332 [details] [review]:

fine
Comment 18 Rui Matos 2016-06-26 17:36:39 UTC
Review of attachment 330334 [details] [review]:

indeed
Comment 19 Rui Matos 2016-06-26 17:36:46 UTC
Review of attachment 330335 [details] [review]:

++
Comment 20 Rui Matos 2016-06-26 17:37:59 UTC
Created attachment 330407 [details] [review]
workspacesView: Initialize the adjustment's upper bound

Initializing the upper bound to zero means that on panning we'd start
scrolling from the first workspace even if the current workspace when
entering the overview was different since StAdjustment clamps the
value to be inside bounds.

--

noticed while testing your patches
Comment 21 Florian Müllner 2016-06-26 18:25:24 UTC
Review of attachment 330407 [details] [review]:

LGTM
Comment 22 Florian Müllner 2016-06-26 18:42:36 UTC
(In reply to Rui Matos from comment #14)
> Review of attachment 330326 [details] [review] [review]:
> 
> This didn't seem to be a problem for me

I'm definitively seeing this using a touchpad.


> and we already do it on ::gesture-end but it doesn't hurt either

Mmmh, I missed that. We don't do the same on ::gesture-cancel though, which fixes the issue for me as well. Moving the call there is more consistent, so I'll do that.
Comment 23 Rui Matos 2016-06-27 12:52:38 UTC
Comment on attachment 330407 [details] [review]
workspacesView: Initialize the adjustment's upper bound

Attachment 330407 [details] pushed as 500ea13 - workspacesView: Initialize the adjustment's upper bound
Comment 24 Florian Müllner 2016-06-27 14:32:36 UTC
Attachment 330326 [details] pushed as 02bad8e - workspacesDisplay: Cancel click when panning
Attachment 330327 [details] pushed as 5182129 - overview: Remove stack actor
Attachment 330329 [details] pushed as c39ffa1 - overview: Move overview actions and scrolling to background group
Attachment 330332 [details] pushed as e16f63a - workspacesView: Allow activating empty workspaces on any monitor
Attachment 330334 [details] pushed as 2ad2853 - workspacesView: Consider workspaces-only-on-primary when scrolling
Attachment 330335 [details] pushed as 21ddbf0 - workspacesView: Consider workspaces-only-on-primary when panning
Comment 25 Florian Müllner 2016-09-27 00:04:22 UTC
*** Bug 694835 has been marked as a duplicate of this bug. ***