After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 597944 - [alt-tab] Separate windows in thumbnail list
[alt-tab] Separate windows in thumbnail list
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2009-10-09 19:44 UTC by drago01
Modified: 2009-11-04 19:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[alt-tab] Separate windows in thumbnail list (1.57 KB, patch)
2009-10-09 19:44 UTC, drago01
none Details | Review
[alt-tab] Separate windows in thumbnail list (1.62 KB, patch)
2009-10-10 20:56 UTC, drago01
needs-work Details | Review
[AppSwitcher] Add a separator between windows in current and other workspaces. (1.85 KB, patch)
2009-10-14 09:50 UTC, Steve Frécinaux
committed Details | Review

Description drago01 2009-10-09 19:44:26 UTC
Created attachment 145173 [details] [review]
[alt-tab] Separate windows in thumbnail list

Add a separator between the windows on the current workspace and
the windows on different workspaces in the thumbnail list, to indicate
the windows on the current workspace (same as done for the application icons).
Comment 1 drago01 2009-10-10 20:56:13 UTC
Created attachment 145225 [details] [review]
[alt-tab] Separate windows in thumbnail list

Don't show the separator if all windows are on another workspace.
Comment 2 Steve Frécinaux 2009-10-14 09:32:26 UTC
Review of attachment 145225 [details] [review]:

::: js/ui/altTab.js
@@ +688,3 @@
             let box = new Big.Box({ padding: AppIcon.APPICON_BORDER_WIDTH + AppIcon.APPICON_PADDING });
+
+            if (windows[i].get_workspace() != activeWorkspace && !separatorAdded && i != 0) {

What if you have an application with two windows on another workspace but no window on the current one? Reading this code, it looks like there will be a separator between both windows, as for the second window, the if's condition will be true.
Comment 3 Steve Frécinaux 2009-10-14 09:50:20 UTC
Created attachment 145405 [details] [review]
[AppSwitcher] Add a separator between windows in current and other workspaces.

This make it is easier for the user to figure out on which workspace the
windows are. For instance, terminals related to various activities and put on
different workspaces were previously displayed as an uniform list, with no
visible distinction between the ones from the current workspace and the others.
Now they are physically separated by a thin gray line.

This is also consistent with the way applications are displayed in the
AppSwitcher.
Comment 4 Dan Winship 2009-10-15 15:21:12 UTC
Comment on attachment 145405 [details] [review]
[AppSwitcher] Add a separator between windows in current and other workspaces.

The patch is fine, but I don't think it actually helps at all given the near-invisibility of the separator line
Comment 5 Steve Frécinaux 2009-10-15 15:30:01 UTC
Well, the visibility part is discussed in bug 597362. It at least helps when you know what you are looking for...
Comment 6 drago01 2009-10-17 08:43:34 UTC
(In reply to comment #2)
> Review of attachment 145225 [details] [review]:
> 
> ::: js/ui/altTab.js
> @@ +688,3 @@
>              let box = new Big.Box({ padding: AppIcon.APPICON_BORDER_WIDTH +
> AppIcon.APPICON_PADDING });
> +
> +            if (windows[i].get_workspace() != activeWorkspace &&
> !separatorAdded && i != 0) {
> 
> What if you have an application with two windows on another workspace but no
> window on the current one? Reading this code, it looks like there will be a
> separator between both windows, as for the second window, the if's condition
> will be true.

Good catch, yeah missed that case.


(In reply to comment #5)
> Well, the visibility part is discussed in bug 597362. It at least helps when
> you know what you are looking for...

Yeah once 597362 is fixed this will be fixed too, so I wouldn't say this is a blocker.
Comment 7 Steve Frécinaux 2009-11-04 08:36:09 UTC
Shall I push this patch?
Comment 8 drago01 2009-11-04 19:50:50 UTC
(In reply to comment #7)
> Shall I push this patch?

I'd say yes, Dan ?
Comment 9 Steve Frécinaux 2009-11-04 19:55:07 UTC
Attachment 145405 [details] pushed as dc232d4 - [AppSwitcher] Add a separator between windows in current and other workspaces.