-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
Drag the window down to open workspace view #579
base: main
Are you sure you want to change the base?
Conversation
This is a cool idea and it works smoothly. I'm into it. Something brought up in slack was making sure that we only do this for bottom displays in a vertical multi-display setup. the percentage works fine for me on a 13" display, but maybe we should get feedback from someone with a rather large display to make sure that's still sane |
How's this going to fit in when Pantheon is gesture aware? |
cebae81
to
afdcc60
Compare
@donadigo I've rebased this on master, but besides the git conflicts, I've had to change the handling of As for the percentage, it feels good to me on a large 3840x1600px screen 👍 |
src/WindowMovementTracker.vala
Outdated
int height; | ||
screen.get_size (null, out height); | ||
|
||
if (y > (float)height * TRIGGER_RATIO) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Does currently not work for multiple monitors if monitors have not the same height or are not aligned. Instead of using the
screen
height I think it should be better to use the height of the current monitor. This should fix the Problem:
Meta.Rectangle wa = window.get_work_area_for_monitor (screen.get_current_monitor ());
if (y - wa.y > wa.height * TRIGGER_RATIO) {
window.position_changed.disconnect (on_position_changed);
open (window, x, y);
}
- Also wouldn't it be reasonable for the
TRIGGER_RATIO
to take the current scaling factor into account, so that it would be larger on HiDPI?
To check if there is no monitor below in a vertical multi monitor setup you could use the code below. bool has_monitor_below (Meta.Window window) {
unowned Meta.Screen screen = window.get_screen ();
var n_monitors = screen.get_n_monitors ();
var current_monitor = screen.get_current_monitor ();
var current_rect = window.get_work_area_for_monitor (current_monitor);
var current_end_x = current_rect.x + current_rect.width - 1;
var current_end_y = current_rect.y + current_rect.height - 1;
for (int i = 0; i < n_monitors; i++) {
if (i == current_monitor) {
continue;
}
var other_rect = window.get_work_area_for_monitor (i);
if (current_end_y < other_rect.y) {
if (current_rect.x <= other_rect.x + other_rect.width - 1 && current_end_x >= other_rect.x) {
return true;
}
}
}
return false;
} |
@andreasfelix do I understand correctly that your code disables the behaviour completely when the monitors are stacked up vertically? If that's the case then it's not what we really want. Ideally, we would like to have always the most bottom edge of the monitor setup to be the trigger instead of disabling the feature completely. |
This is what I was trying to achieve:
The function |
@danrabbit resolved conflicts and merged the multi-monitor fixes from @andreasfelix. Ready for review again. |
@andreasfelix unfortunately I won't be able to solve these issues by myself. I do not have the setup to properly test these cases so there's little here to do for me. |
Converting to draft since there are conflicts and there hasn't been any movement here for a while. Feel free to mark ready for review if this is updated :) |
Fixes #569.
Enables dragging the window down to the bottom screen edge to open workspace view and immediately move the window elsewhere.
Currently based on screen percentage (more than 98% of the height), we may rethink if this should happen.
Testable also with other experimental features in https://github.com/elementary/gala/tree/intelligent-workspaces.