Skip to content

Commit

Permalink
myDash: fix scrolling event propagation.
Browse files Browse the repository at this point in the history 
When the fixed-icon options is set, the scroll event should not
propagate to the actor below. However, this was occasionally happening
because the condition I was using was not enough. Just use the settings
to decide what to do with the scroll event instead.
  • Loading branch information
@micheleg
micheleg committed Oct 28, 2015
1 parent dc4bf11 commit ab5367d
Showing 1 changed file with 7 additions and 11 deletions.
18 changes: 7 additions & 11 deletions myDash.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,10 @@ const myDash = new Lang.Class({

_onScrollEvent: function(actor, event) {

// If scroll is not used because the icon is resized, let the scroll event propagate.
if (!this._settings.get_boolean('icon-size-fixed'))
return Clutter.EVENT_PROPAGATE;

// reset timeout to avid conflicts with the mousehover event
if (this._ensureAppIconVisibilityTimeoutId>0) {
Mainloop.source_remove(this._ensureAppIconVisibilityTimeoutId);
Expand All @@ -561,20 +565,12 @@ const myDash = new Lang.Class({
if (event.is_pointer_emulated())
return Clutter.EVENT_STOP;

let adjustment, delta, needscroll;
let adjustment, delta;

if (this._isHorizontal) {
if (this._isHorizontal)
adjustment = this._scrollView.get_hscroll_bar().get_adjustment();
needscroll = actor.get_width() < actor.get_preferred_width(-1)[1];
} else {
else
adjustment = this._scrollView.get_vscroll_bar().get_adjustment();
needscroll = actor.get_height() < actor.get_preferred_height(-1)[1];
}

// If scroll is not needed, either because icon are set to be resized or
// the space available is enough, let the scroll event propagate.
if (!needscroll)
return Clutter.EVENT_PROPAGATE;

let increment = adjustment.step_increment;

Expand Down

9 comments on commit ab5367d

@marmis85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micheleg hello, this commit breaks scrolling on the dock to switch workspaces. The shell freezes for 1-2 seconds before restarting. Trying to scroll again then crashes the whole gnome session.
Scrolling on the "Show Applications" icon works fine though.

@micheleg
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, Which version of the shell are you using?

@marmis85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using gnome shell 3.18.1 on two different arch linux boxes, and both have the same problem.
I also tried disabling all extensions but dash-to-dock and the problem still persist.
I don't know if it can help, I'm using the extension in panel mode with fixed-size icons on the left side on the screen, not enough icons to fill the dock.

/edit
one box is actually updated to 3.18.2

@micheleg
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fixed now after 4792e4a

@marmis85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it doesn't freeze/crash anymore, but it works only on the "Show Applications" icon even if there is free space on the dock

@micheleg
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was actually the reason of this commit. I was trying to keep the scrolling behaviour when the dock scrolling was not required, but

  1. This was causing some bugs with the workspace change when trying to scroll the dock above its limits
  2. It's kind of an inconsistent behaviour to have the scroll gesture performs diferent actions, depending on the dock being full or not, which might not be clear at first.

I choose to just disable the workspace scrolling when the fixed icon size is chosen, limiting this feature to the show apps icon. Do you have any other idea about how to keep the features and consistency?

@marmis85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about limiting the workspace scrolling to a 1 pixel area near the edge of the screen? This way the behavior could be consistent regardless of panel mode and icon size, and it's also a slightly larger (and maybe easier) "hit target"

@marmis85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I had some time to dive into the code and made a tentative implementation and sent you pull request #249
The only case I found that is a bit ugly is when the dock is positioned at the top under the panel, in that case it's a bit hard to hit the right pixel, but one can still scroll on the "Show Applications" icon, use non-fixed icons to be able to scroll anywhere on the dock, or anyway there are other extensions to switch worskspace by scrolling on the panel

@micheleg
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrolling from the outer pixel in horizontal mode is definitely a bit weird. For the top mode, I don't consider it a real issue as the only situation in which it makes sense to me to have the dock at the top is if the panel is not there for whatever reason.

Please sign in to comment.