Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented workspace switching by scrolling on and the edge of the … #249

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

marmis85
Copy link

…dock when using fixed-sized icons

marmis85 referenced this pull request Nov 13, 2015
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.
@marmis85
Copy link
Author

@micheleg I made some further experiments, I was trying to reliably understand at runtime if there is actually the need to adjust the content of the ScrollView, and this is the best I could come up with XD

This is how it works: I check if the available workspace area (in the relevant direction) is bigger than the actor of the dock, or if the child of the ScrollView is smaller than the ScrollView itself, or if we are on the relevant edge of the dock, then we can (kinda) safely let the event propagate to switch workspaces.

This works because as of the current implemetantion the ScrollView has only one child, a BoxLayout, in which the application icons are added, and in "panel mode" when there aren't enough icons to fill the dock the BoxLayout's size will always be less than the ScrollView's size, while in "resizable icons mode" it won't start scrolling until the available workspace area is completely filled.

The only edge case that is a bit clunky is if by chance there are enough icons to fill the dock (but not overflow) and the BoxLayout's size is EXACTLY the same as the ScrollView's: in that case only the edge of the dock is available, because from my testing the BoxLayout's size will be AT MOST that of the ScrollView.

Like this scrolling at the edge acts "really" as fallback in case the dock is completely filled and is more consistent with the explanation in the settings page.

What do you think about this?

@micheleg
Copy link
Owner

Hi, It will need some time to look into your patch, I have not that much time right now. I'm kind of positive about integrating the previous one. Regarding the new one, I had a similar behaviour before (see ab5367d), but turned out to be not reliable. I haven't tested nor looked into your implementation though. I honestly don't like the current behaviour -- only show app icon active - nor I'm totally convinced by these hybrid ones, but I can't think of a better solution.

@marmis85
Copy link
Author

No problem, take your time, I've been playing with this because when I updated to git master I realized it was kinda important for my personal workflow and configuration, but I've come to understand that it's a though nut to crack: those functionalities just clash and there isn't much that can be done but find a decent compromise.

Please if you will have time take a look at the implementation as I'm not that confident in my skills in the gnome ecosystem!

In the end whichever decision you ultimately make, I can totally live with it and adapt as I have no plan to ditch your wonderful piece of work :)

@micheleg
Copy link
Owner

Hi,

I've merged the first patch into master [d8ebb5a], i.e. now the edge is active in fixed icon mode. I'm skipping the second one (whole dock active when the scrolling is not necessary) because I still think keeping the behaviour consistent is a better option.

@marmis85
Copy link
Author

Hello,
to be honest, I've been testing this all this time (second part of the patch included), and sometimes it happened that lost in thought I scrolled on the dash for no reason, or I scrolled slightly out of the edge of a maximized window, and got unwanted desktop switches, so I agree with you!

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants