-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Change workspace with mouse wheel #752
Conversation
static void mouse_scroll_notify(struct window *window, enum scroll_direction direction) { | ||
sway_log(L_DEBUG, "Mouse wheel scrolled %s", direction == SCROLL_UP ? "up" : "down"); | ||
|
||
const char *workspace_name = direction == SCROLL_UP ? "prev" : "next"; |
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.
Not sure what the correct behavior is here: prev vs. prev_on_output.
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.
Btw. One annoying thing with prev/next is that when changing to a workspace on a different output the mouse pointer follows.
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.
The correct behavior is prev_on_output.
This is more of a RFC pull request. |
@deklov there's horizontal scrolling too (axis == 0 -> vertical, 1 -> horizontal) |
uint32_t time, uint32_t axis, wl_fixed_t value) { | ||
struct window *window = data; | ||
|
||
if (!sway_assert(!axis, "Only support horizontal axis")) { |
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.
Assertions are to detect things that should never happen. Horizontal scrolling doesn't qualify.
LGTM |
if (!sway_assert(false, "Only support horizontal axis")) { | ||
return; | ||
} | ||
} |
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.
@Hummer12007 something like this?
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.
Yeah, but why should you add a default clause? The axis enum in libinput/wayland has only two directions.
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.
I don't have a strong preference for or against a default here, but the assert message needs to be updated.
Thanks! |
No description provided.