-
Notifications
You must be signed in to change notification settings - Fork 142
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
Minimize an active window when its entry is clicked in the taskbar #234
base: master
Are you sure you want to change the base?
Conversation
rc = wndmgt_get_window_info(entry->wndlist->wndmgt, | ||
entry->wnd_id, &winfo); | ||
if (rc == EOK) { | ||
minimized = (winfo->flags & wndf_minimized) == 0; |
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.
Note that the logic is reversed w.r.t. the name of the variable: minimized will be true iff the window is not minimized
if (!minimized) { | ||
(void) wndmgt_activate_window(entry->wndlist->wndmgt, | ||
dev_id, entry->wnd_id); | ||
} else { |
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.
Now here's the big problem. If you click the button of a window that's not minimized and is not focused, instead of activating the window it will minimize it. Also note that with multiple seats, each window can have zero, one or more foci. A window should only be minimized if it is the focus of the same seat that clicked the window button. For example, you might add a WM operation to test if a particular window is focused by a particular seat (which is specified via input device ID).
(void) wndmgt_activate_window(entry->wndlist->wndmgt, | ||
dev_id, entry->wnd_id); | ||
} else { | ||
(void) wndmgt_deactivate_window(entry->wndlist->wndmgt, |
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 think the operation should simply be called 'minimize', because that's what it does (this pertains to all other places too).
|
||
if (!minimized) { | ||
(void) wndmgt_activate_window(entry->wndlist->wndmgt, | ||
dev_id, entry->wnd_id); |
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 indentation is incorrect. Did you run 'make ccheck'?
@@ -47,6 +47,7 @@ extern errno_t wndmgt_get_window_info(wndmgt_t *, sysarg_t, | |||
wndmgt_window_info_t **); | |||
extern void wndmgt_free_window_info(wndmgt_window_info_t *); | |||
extern errno_t wndmgt_activate_window(wndmgt_t *, sysarg_t, sysarg_t); | |||
extern errno_t wndmgt_deactivate_window(wndmgt_t *, sysarg_t, sysarg_t); |
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.
For this new operation unit tests should be added to /data/helenos/master/uspace/lib/wndmgt/test/wndmgt.c, similar to PCUT_TEST(activate_window_failure) and PCUT_TEST(activate_window_success).
No description provided.