Comment 416 for bug 733349

This is the code I asked about:

=== modified file 'launcher/ApplicationLauncherIcon.cpp'
--- launcher/ApplicationLauncherIcon.cpp 2014-03-12 23:46:10 +0000
+++ launcher/ApplicationLauncherIcon.cpp 2014-03-17 20:58:25 +0000
@@ -399,7 +399,16 @@
{
if (arg.source != ActionArg::Source::SWITCHER)
{
-Spread(true, 0, false);
+WindowList windows = GetWindows(WindowFilter::ON_CURRENT_DESKTOP);
+
+if (windows.size() == 1 && minimize_window_on_click)
+{
+wm.Minimize(windows[0]->window_id());
+}
+else
+{
+Spread(true, 0, false);
+}
}
}
}

Looks like it checks how many windows are in the group (windows.size() == 1) and if the setting is enabled (minimize_window_on_click); and the function is limited to windows on the current desktop (WindowFilter::ON_CURRENT_DESKTOP); but I don't see anything about focused/unfocused state.

Most of the time it will be fine as-is, especially for users who don't clutter a single workspace with lots of windows, but there may be occasions when a single window on the current desktop may be under another window or a group of windows and clicking on the icon will minimize it behind them ("Aunt Suzy"). Since there's a minimize animation to see, most users will probably realize what happened and click the icon one more time to get the desired window out and on top.

Adding an additional if statement to cover this case would be a nice improvement. Please excuse my ignorance of the proper names of the functions and variables involved; consider this a blueprint for actual code:

if (windows.size() == 1 && minimize_window_on_click)
{
+if ( windows[0]->window_state() == unfocused )
+{
+wm.Raise(windows[0]->window_id());
+}
+else
+{
 wm.Minimize(windows[0]->window_id());
 }

I know how hard it was to get this feature in even "as-is" with "no more tweaks or enhancements", and I'm happy to see it happen at all, so I won't hold it against anyone if this behavior is never updated; just a suggestion (and reiteration of the same from several comments in this report).